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

Fix SQLite documentation example. #1750

Closed angrytongan closed 1 month ago

angrytongan commented 1 month ago

This is current production https://observablehq.com/framework/lib/sqlite:

Screenshot 2024-10-14 at 05 06 54

rather use the self-hosted chinook.db than the externally hosted one.

Ah, ok. Let me fix up my PR.

mbostock commented 1 month ago

Ah, this must be a Safari bug. I’ll investigate when I’m back at my desktop.

Fil commented 1 month ago

We can reduce this to:

```js echo
import SQLite from "npm:@observablehq/sqlite";
```

```js echo
FileAttachment("chinook.db").sqlite()
```

removing the explicit import SQLite fixes the issue, which seems to confirm that this might be due to https://bugs.webkit.org/show_bug.cgi?id=242740

The issue seems to be caused by the parallel imports of sqlite.js that are made in the two cells.

Here's a different repro which might help investigate:

```js echo
import("npm:@observablehq/sqlite");
```

```js echo
SQLite
```

on Safari, the presence of the first block causes the second code block to be (erroneously) undefined instead of being stdlib’s SQLite object.

Fil commented 1 month ago

To fix the documentation it is enough to set run=false on the cells that explicity load this module:

 [SQLite](https://sqlite.org/) is “a small, fast, self-contained, high-reliability, full-featured, SQL database engine” and “the most used database engine in the world.” Observable provides a ESM-compatible distribution of [sql.js](https://sql.js.org), a WASM-based distribution of SQLite. It is available by default as `SQLite` in Markdown, but you can import it like so:

-```js echo
+```js run=false
 import SQLite from "npm:@observablehq/sqlite";
 ```

 We also provide `SQLiteDatabaseClient`, a [`DatabaseClient`](https://observablehq.com/@observablehq/database-client-specification) implementation.

-```js echo
+```js run=false
 import {SQLiteDatabaseClient} from "npm:@observablehq/sqlite";
 ```

(but that doesn't fix the underlying issue).

mbostock commented 1 month ago

Wow, so top-level await is broken in Safari? That’ll be hard to avoid. I hope they fix it soon.

Fil commented 1 month ago

👎 Not fixed in Safari Technology Preview (Release 205 (Safari 18.0, WebKit 20621.1.2.111.4))

mbostock commented 1 month ago

I’ve implemented a workaround for top-level await being broken in Safari in #1759. However, I’m not sure it’s practical for us to workaround such a fundamental browser bug. Several other recommended libraries also use top-level await, include DuckDBClient and dot. It’d be relatively easy to fix DuckDBClient, but I don’t think we can fix dot without making it async.

So… maybe users will need to avoid importing the same library multiple times (until Safari is fixed)?