observablehq / framework

A static site generator for data apps, dashboards, reports, and more. Observable Framework combines JavaScript on the front-end for interactive graphics with any language on the back-end for data analysis.
https://observablehq.com/framework/
ISC License
2.56k stars 122 forks source link

avoid top-level await; import npm:sql.js #1759

Closed mbostock closed 2 weeks ago

mbostock commented 1 month ago

Ref. #1750. Avoids top-level await in our SQLite implementation. Also makes it so that importing npm:sql.js “just works” (by rewriting the code slightly from what is distributed via npm!). Removes SQLite from standard library (because there’s no way to support it without using top-level await) in favor of having people import sql.js directly or use SQLiteDatabaseClient.

Marking as draft because the hacky rewriting of sql.js should be cleaned up before merging.

Also we might also want to fix DuckDBClient? And dot? But I don’t think dot is fixable without making it async…

mbostock commented 1 month ago

Just want to clean up the sql.js shim if I can.

mbostock commented 1 month ago

The other thing I’m ruminating on is that this isn’t strictly a backwards-compatible change. Previously this would be equivalent to the SQLite that is available by default in Markdown:

import SQLite from "npm:@observablehq/sqlite"; 

Now, however, the explicit import gives you a promise. That’s kinda the same as long as you reference SQLite from another fenced code block. But technically it’s not backwards-compatible. And it would be more noticeable in a local JavaScript module (where you don’t get implicit await).

So… We’re kinda stuck with top-level await in the SQLite case if we want to maintain backwards compatibility. But maybe it’s okay to change the semantics slightly, especially if it’s rare and given that top-level await is broken in Safari? Maybe I should break out the DuckDB-Wasm changes since those don’t have the same concern.

Fil commented 1 month ago

OK… I wouldn't worry too much about the slight change in API because… the default SQLite symbol is not documented in https://observablehq.com/framework/lib/sqlite (I mean, it's referenced, but nothing says how to use it); only SQLiteDatabaseClient is shown. We also need to fix the documentation :)

Fil commented 2 weeks ago

lgtm