DallasHoff / sqlocal

SQLocal makes it easy to run SQLite3 in the browser, backed by the origin private file system.
https://sqlocal.dallashoffman.com
MIT License
212 stars 8 forks source link

Feature request: ability to control instantiation of sqlite db #9

Closed clintharris closed 2 days ago

clintharris commented 8 months ago

Wow, amazing work here--really high quality code and docs, and just what I'm looking for (sqlite-wasm + Kysely).

That said, I wish the API supported a way for me to pass in the sqlite DB instead of sqlocal creating it (i.e., the ability to override SQLocalProcessor.init). Would you be open to making a change along those lines, or considering a PR to that effect?

Some reasons this might be a nice feature:

DallasHoff commented 8 months ago

There's difficulty in allowing one to just pass in the database object or a function since neither of these can be transferred from the SQLocal class to the worker through postMessage since functions and class instances cannot be cloned. As an alternative to this, I experimented a while back with allowing for passing a custom worker. This is why SQLocalProcessor is its own class and worker.ts just instantiates that. But if you are making your own version of SQLocalProcessor, you are re-implementing a lot of the library while being bound to the messaging system SQLocal uses, and I'm not sure there's much value in that over building your own full custom solution to use instead of SQLocal. SQLocal is meant to be an abstraction to cover most use-cases. Apps with more complex requirements may not be able to use an abstraction.

However, I do think adding options that can be passed to change the OpfsDb flags and tweak other behaviors is a good idea. I can definitely see someone wanting to use the trace flag, like you said, or use SQLocal to open a database file in read-only mode. This would not be too hard to add. I'll look into it.

clintharris commented 7 months ago

Ah yeah, didn't think of the limitations of passing custom implementations across to the worker...really good point.

What do you think about allowing someone to pass in their own worker as an optional SQLocal constructor param (with the disclaimer that it's an unsupported escape hatch: "by using this, you accept responsibility for maintaining worker message compatibility, forfeit issue support, etc.")?

// myWorker.js
import { SQLocalProcessor } from '@sqlocal/processor';
import sqlite3InitModule from '@sqlite.org/sqlite-wasm';

// Dev gets to pick sqlite-wasm version and customize constructor param (e.g., sqlite logging) 🎉 
const sqlite3 = await sqlite3InitModule(...);
const myDb = new this.sqlite3.oo1.OpfsDb(this.config.databasePath, 'cw');

const processor = new SQLocalProcessor(myDb); // 👈 New: pass in custom db instance

self.onmessage = (message) => processor.postMessage(message);
processor.onmessage = (message) => self.postMessage(message);
// myapp.js
import { SQLocalKysely } from "sqlocal/kysely";

const myWorker = new Worker(new URL('./myWorker', import.meta.url), { type: 'module' });
const { dialect } = new SQLocalKysely("foo.sqlite", myWorker); // 👈 New: pass in custom worker instance
// ...

It seems like the changes to SQLocalProcessor.init() would be fairly minimal? Basically "if custom DB passed in, use that, otherwise use the existing logic for creating it"?

There's just so much other great stuff in sqlocal--would be great to find a way for someone to avoid forking just to customize such a small amount of code.

Do you think something like this would work as an optional escape hatch?

DallasHoff commented 7 months ago

I don't hate that idea, but I'm not sure it's necessary. Here's what I'm thinking right now. Along with the options for flags we mentioned above, there could be a storage option that accepts 'memory' | 'opfs' | 'local' | 'session'. If local or session is used, SQLocal will create an SQLocalProcessor that uses JsStorageDb; OpfsDb for opfs; DB for memory. This gives the freedom to use all the available storage mechanisms while keeping things simple. What do you think?