sqlite / sqlite-wasm

SQLite Wasm conveniently wrapped as an ES Module.
520 stars 46 forks source link

Fix type `sqlite3session_attach`, that accepts `0` for `tableName` arg #78

Closed louwers closed 4 months ago

louwers commented 4 months ago
sqlite3.capi.sqlite3session_attach(pSession, 0);

is the easiest way to monitor all tables for changes. But this is not type-correct. This PR fixes that.

sgbeal commented 4 months ago

Note that the sqlite3.capi functions very specifically require the same number of arguments as their C counterparts except when very specifically noted in the docs (sqlite3_create_function() and friends come to mind, as they accept Function args). We intentionally avoided "dumbing down" (for lack of a better term) the C-style API that way, primarily so that the C docs can be used for JS code most of the time (though we make heavy use of default args in the higher-level APIs).

That said, @tomayac is the overseer of the typescript-related stuff, so it's his call as to whether that design decision applies here.

louwers commented 4 months ago

@sgbeal I am not changing the number of arguments. But I did make a typo with the union (fixed now).

I'm not sure if null will also work.

sgbeal commented 4 months ago

@sgbeal I am not changing the number of arguments.

Ah, indeed, my apologies. i completely misunderstood the patch to be providing a default of 0.

sgbeal commented 4 months ago

I'm not sure if null will also work.

Yes, it will. The JS-to-WASM conversion transforms null and undefined to integer 0 for pointer-passing purposes (wasm uses 32-bit ints for addresses).

louwers commented 4 months ago

@sgbeal No worries!

I confirmed null also works. I created a new NullPointer type for clarity. I'll leave it to @tomayac what the best approach is.

tomayac commented 4 months ago

Going with null sounds good to me. We can improve upon this if need be.

steida commented 4 months ago

is the easiest way to monitor all tables for changes.

@louwers May I ask for the use case? Thank you

louwers commented 4 months ago

@steida https://floss.social/@bart/112547953612614556

steida commented 4 months ago

@louwers Interesting. Thinking whether I could use that to speed up reactive queries in https://github.com/evoluhq/evolu