Missing typing for sqlite3Worker1Promiser

[benjamin-wright]

trafficstars

In order to follow the recommended "wrapped worker" approach from the readme I had to add:

declare module '@sqlite.org/sqlite-wasm' {
  export function sqlite3Worker1Promiser(...args: any): any
}

immediately below the import statement, because sqlite3Worker1Promiser is not included in the index.d.ts

[tomayac]

I'm not much of a TypeScript person, but the any suggests there's more work to be done. Started https://github.com/sqlite/sqlite-wasm/pull/54. Feel free to pile on. FYI @jbaiter.

[filipe-freire]

Hey there! Firstly, thanks for shipping this package in the first place! :)

I understand that having this typed as "any" is far from ideal, and even saw the ongoing PR to add the type definitions to it. Not an easy job!

However, it would make a potential developer less confused when using the package with Typescript if the type was shipped by default instead of having to declare it manually. Especially since the recommended approach uses "sqlite3Worker1Promiser".

Maybe we could add a JSDoc noting that the type is under development and keep it as something along the lines of:

(...args: unknown[]) => Promise<unknown>

I know there's nothing more permanent than a temporary solution, so I understand if we don't follow with this approach too 😅

[tomayac]

@filipe-freire I'm not opposed, but am unsure where you'd add this, the caveat from https://github.com/sqlite/sqlite-wasm/issues/53#issuecomment-1827405950 still applies 🫣. To the thin wrapper code part of this repo (if so, please file a quick PR), or the underlying SQLite code (if so, that's something to raise with @sgbeal)?

[sgbeal]

To the thin wrapper code part of this repo (if so, please file a quick PR), or the underlying SQLite code (if so, that's something to raise with @sgbeal)?

In the upstream project we use only vanilla JS and avoid all tooling-specific extensions (because none of us use them, so we can't be relied upon to maintain such pieces properly long-term).

[filipe-freire]

Never mind my comment above, after analyzing the PR more thoroughly the work there is already way beyond what I suggested. Apologies for that 😅 The work would be done in this wrapper though.

I unfortunately don't have the insight needed to complete that PR. However, an option is to merge it as it stands right now, which would still provide a better DX to someone just starting out.

The only thing needed would be to change the Docs to reflect the types added:

import {
    sqlite3Worker1Promiser,
    type Promiser,
    type PromiserResponseSuccess
} from '@sqlite.org/sqlite-wasm';

const log = console.log;
const error = console.error;

const initializeSQLite = async () => {
    try {
        log('Loading and initializing SQLite3 module...');

        const promiser = (await new Promise((resolve) => {
            const _promiser = sqlite3Worker1Promiser({
                onready: () => resolve(_promiser)
            });
        })) satisfies Promiser;

        log('Done initializing. Running demo...');

        const configResponse = (await promiser(
            'config-get', {}
        )) as PromiserResponseSuccess<'config-get'>;
        log('Running SQLite3 version', configResponse.result.version.libVersion);

        const openResponse = (await promiser('open', {
            filename: 'file:mydb.sqlite3?vfs=opfs'
        })) as PromiserResponseSuccess<'open' >;
        const {
            dbId
        } = openResponse;
        log(
            'OPFS is available, created persisted database at',
            openResponse.result.filename.replace(/^file:(.*?)\?vfs=opfs$/, '$1')
        );
        // Your SQLite code here.
    } catch (err) {
        if (!(err instanceof Error)) {
            err = new Error(err.result.message);
        }
        error(err.name, err.message);
    }
};

initializeSQLite();

Type casting is not an ideal solution, yet it might be a good enough compromise while this work is ongoing. I'll defer the decision to you! ✌🏻

[tomayac]

I don't know how merge-ready #54 is. Do people here generally approve of a not-perfect-but-good-enough merger? I think for the documentation, we should keep it vanilla. Those wanting to use the types will know how to include them, whereas people unfamiliar with TypeScript might be wondering what the to them weird type imports are.

[filipe-freire]

How about simply adding a Typescript version for its users out there? Having them figure it out by themselves is not the best solution here imo...

As for the approval of "not-perfect-but-good-enough" PR's, I'll defer that to y'all 😁

[mkjawadi]

Any update on this? I cannot follow the recommended "wrapped worker" approach in my Vite React + TypeScript project due to sqlite3Worker1Promiser issue. Is there any demo of this approach with TypeScript? Thanks in advance.