-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
generate typescript definitions #42
Conversation
@@ -16,7 +16,7 @@ | |||
}, | |||
"scripts": { | |||
"install": "node-pre-gyp install --fallback-to-build", | |||
"pretest": "node test/support/createdb.js", | |||
"xpretest": "node test/support/createdb.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is disabled (by renaming it from "pretest") because it uses the old API.
|
||
if isinstance(node.type, pycparser.c_ast.Enum): | ||
self.result += f'auto {name}_enum = Napi::Object::New(env);\n' | ||
self.cpp_result += f'exports.Set(Napi::String::New(env, "{name}"), duckdb_node::PointerHolder<{name}>::Init(env, "{name}")->Value());\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed "result" to "cpp_result", since there is now also a "types_result". I haven't changed the generated C++ at all.
self.cpp_result += f'exports.Set(Napi::String::New(env, "{name}"), duckdb_node::PointerHolder<{name}>::Init(env, "{name}")->Value());\n' | ||
self.types_result += f'export class {name} {{}}\n' | ||
self.c_type_to_ts_type[name] = name | ||
self.c_type_to_ts_type[f'{name}*'] = name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each struct type is exposed to JS as a class that is a variant of PointerHolder. This is true for both direct references to the struct type as well as pointers to that type. I'm representing these as separate classes; the only thing that a TS client can do is "new" them and then pass them around.
# Do these void* types need any corresponding Napi code? | ||
self.types_result += f'export class {name} {{}}\n' | ||
self.c_type_to_ts_type[name] = name | ||
self.c_type_to_ts_type[f'{name}*'] = name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part handles typedefs like typedef void *duckdb_bind_info;
. It's needed because some APIs take or return duckdb_bind_info
s.
@@ -59,43 +101,60 @@ def visit_FuncDecl(self, node): | |||
|
|||
if node.args: | |||
for p in node.args.params: | |||
args.append(typename(p.type)) | |||
args.append((p.name, typename(p.type))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the name of each arg to the args
array, so I can generate the TypeScript function signatures with the correct argument names.
return # ?? | ||
|
||
if 'delete_callback' in name: | ||
print(f'function not handled: {name}') | ||
self.types_result += f'export type {name} = (data: pointer) => void;\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed because duckdb_delete_callback_t
is used in a couple places.
|
||
n_args = len(args) | ||
args.append(name) | ||
|
||
fwrap_args = list(map(lambda arg: arg[1], args)) + [name] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than modify args
, which I need for the TS generation, I'm creating a copy for the C++ generation. The C++ needs the arg types (not the names), plus the name of the function.
@@ -52,7 +52,7 @@ async function test() { | |||
|
|||
// we want an incremental AND streaming query result | |||
const pending_result = new duckdb_native.duckdb_pending_result; | |||
await duckdb_native.duckdb_pending_prepared_streaming(prepared_statement, pending_result); // TODO can this fail? | |||
duckdb_native.duckdb_pending_prepared_streaming(prepared_statement, pending_result); // TODO can this fail? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This await
was unnecessary, because this function is not async.
2c2a17e
to
d4ac3fb
Compare
@@ -4,15 +4,15 @@ const duckdb_native = require('.'); | |||
console.log("DuckDB version:", duckdb_native.duckdb_library_version()); | |||
|
|||
function convert_validity(vector, n) { | |||
const res = new Uint8Array(n).fill(true); | |||
const res = Array.from({ length: n }).fill(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this function should return a normal JS array of booleans. This is a more canonical way to do that.
const validity_buf = duckdb_native.copy_buffer(duckdb_native.duckdb_vector_get_validity(vector), | ||
Math.ceil(n / 64) * 8); // this will be null if all rows are valid | ||
if (validity_buf == null) { | ||
return res; // TODO maybe return a singleton so we dont have to allocate? | ||
} | ||
const typed_validity_buf = new BigUint64Array(validity_buf.buffer); | ||
for (let row_idx = 0; row_idx < n; row_idx++) { | ||
res[row_idx] = (typed_validity_buf[Math.floor(row_idx / 64)] & (1n << BigInt(row_idx % 64))) > 0; | ||
res[row_idx] = (typed_validity_buf[Math.floor(row_idx / 64)] & (BigInt(1) << BigInt(row_idx % 64))) > 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all JS targets can handle the newer bigint literal syntax.
ff0426f
to
9a9ba59
Compare
#include "duckdb.h" | ||
#include "function_wrappers.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change happened automatically when I ran clang format.
No description provided.