Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vanilla stdlib; server-side render implicit stylesheets; async Generators; width(target) #294

Merged
merged 24 commits into from
Dec 4, 2023

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Dec 1, 2023

This makes the Observable standard library naturally consumable from vanilla JavaScript: it’s no longer a Library object only intended for use with the Observable Runtime; it’s now just a library with normal exports.

symbol equivalent
now import {now} from "npm:@observablehq/stdlib"
width import {width} from "npm:@observablehq/stdlib"
DatabaseClient import {DatabaseClient} from "npm:@observablehq/stdlib"
FileAttachment import {FileAttachment} from "npm:@observablehq/stdlib"
Generators import {Generators} from "npm:@observablehq/stdlib"
Mutable import {Mutable} from "npm:@observablehq/stdlib"
_ import _ from "npm:lodash"
aq import * as aq from "npm:arquero"
Arrow import * as Arrow from "npm:apache-arrow"
d3 import * as d3 from "npm:d3"
dot import dot from "npm:@observablehq/dot"
duckdb import * as duckdb from "npm:@duckdb/duckdb-wasm"
DuckDBClient import {DuckDBClient} from "npm:@observablehq/duckdb"
htl import * as htl from "npm:htl"
html import {html} from "npm:htl"
svg import {svg} from "npm:htl"
Inputs import * as Inputs from "npm:@observablehq/inputs"
L import * as L from "npm:leaflet"
mermaid import mermaid from "npm:@observablehq/mermaid"
Plot import * as Plot from "npm:@observablehq/plot"
SQLite import SQLite from "npm:@observablehq/sqlite"
SQLiteDatabaseClient import {SQLiteDatabaseClient} from "npm:@observablehq/sqlite"
tex import tex from "npm:@observablehq/tex"
topojson import * as topojson from "npm:topojson-client"

For example, if you import * as L from "npm:leaflet", it is now exactly the same as L from the standard library. And rather than wrapping Leaflet to inject the stylesheet, we server-side render the stylesheet link so that it loads faster! (We do the same now for KaTeX and Observable Inputs.)

Two exceptions are now and width: since these are reactive variables implemented by generators, in vanilla JavaScript they are generator functions and hence you must call now() or width() to get the generator. I’m not entirely sure what to do with these 🤷 but that was the easiest thing to do. Additional exceptions are “special” variables that only make sense in reactive JavaScript: display, invalidation, visibility, and view. The sample datasets also are available in Markdown but aren’t importable; I think that’s fine.

The width function now also accepts an optional target: Element if you want to observe something other than the first main element.

This also kills the following features from the standard library:

  • require
  • resolve
  • md
  • __query
  • DOM.canvas
  • DOM.context2d
  • DOM.download
  • DOM.element
  • DOM.input
  • DOM.range
  • DOM.select
  • DOM.svg
  • DOM.text
  • DOM.uid
  • Files.buffer
  • Files.text
  • Files.url
  • Generators.disposable
  • Generators.filter
  • Generators.map
  • Generators.range
  • Generators.valueAt
  • Generators.worker
  • Promises.delay
  • Promises.tick
  • Promises.when

In addition to the above changes, this also implements server-side rendering of stylesheets that are implicitly needed by recommended libraries. 🚀 Specifically:

library stylesheet
npm:@observablehq/inputs https://cdn.jsdelivr.net/gh/observablehq/inputs/src/style.css
npm:katex https://cdn.jsdelivr.net/npm/katex/dist/katex.min.css
npm:leaflet https://cdn.jsdelivr.net/npm/leaflet/dist/leaflet.css

This approach improves performance over wrapping the library to insert the stylesheet after the library loads.

This PR also simplifies the “CLI resolver” abstraction to a resolveDatabase function.

This PR also re-implements Generators.observe, Generators.input, and Generators.queue as async generators.

Future work:

  • implement typed: "auto" for FileAttachment.csv and FileAttachment.tsv
  • rewrite FileAttachment calls in imported modules; depends on find fetches in locally imported files #286
  • publish @observablehq/dot to npm; archive @observablehq/graphviz
  • publish @observablehq/duckdb to npm
  • publish @observablehq/mermaid to npm
  • publish @observablehq/sqlite to npm
  • publish @observablehq/stdlib v6 with most functionality removed? or branch?
  • publish @observablehq/tex to npm; archive @observablehq/katex
  • publish @observablehq/xlsx to npm

Fixes #24. Fixes #19.

src/javascript/imports.ts Outdated Show resolved Hide resolved
src/client/stdlib/dot.js Outdated Show resolved Hide resolved
@mbostock mbostock force-pushed the mbostock/stdlib branch 3 times, most recently from 510abc5 to 4f18eca Compare December 2, 2023 00:24
@mbostock mbostock changed the title vanilla stdlib vanilla stdlib; server-side render implicit stylesheets Dec 2, 2023
@mbostock mbostock marked this pull request as ready for review December 2, 2023 17:48
@mbostock mbostock requested review from Fil and cinxmo December 2, 2023 17:48
@mbostock
Copy link
Member Author

mbostock commented Dec 2, 2023

Open questions:

1. Implement file.csv({typed: "auto"})?

This involves ~200 lines of code, namely inferSchema and coerceRow. It’s not widely known or used; I’m not sure it’s a great API; what you really want is the ability to specify the types more explicitly, such as what columns you expect to be dates, and what date format and time zone to use. Maybe we add it in a followup PR?

2. Rewrite FileAttachment calls in imported modules?

We should do this, but maybe we can do it in a followup PR since this is already large. The code will need to check if you imported FileAttachment from npm:@observablehq/stdlib.

2.5. Remove rewriting of fetch?

It weirds me out slightly that we change the semantics of fetch, but it also seems weird to encourage people to use FileAttachment when they could just use fetch? 🤔 And presumably people who don’t read the documentation are going to call fetch anyway and then be surprised that it doesn’t work in build? But our rewriting approach only works if you call fetch directly, and so doesn’t work if you use a helper such as d3.json or d3.csv. Maybe this means that rewriting fetch is misguided, and we should use FileAttachment explicitly because it’s different from fetch (and brings the static analysis and rewriting along with it). That way “fetch is just fetch”.

3. Restore deprecated functionality (e.g. ,DOM.context2d) from the standard library?

We can always decide to restore this in a followup PR. I’m of two minds. It’s good to make it easier to port code from notebooks; but it’s also good to sunset methods that we no longer recommend, to reduce what we need to document, to take this moment to shed the old ways, and to reduce competing and unfamiliar approaches that could confuse new users.

4. Publish the new standard library as @observablehq/stdlib, or something else?

The new stdlib is quite small. Most of it is now “recommended libraries” which are available by default in Markdown but in ES modules are imported from npm as separate libraries rather than rolled up in @observablehq/stdlib. So maybe we just fold our own utilities such as Generators.observe into @observablehq/runtime v6 (and remove Library, and archive @observablehq/stdlib)? And notebooks continue to run on v5, but eventually we upgrade them to v6 once we have standard library versioning?

@mbostock mbostock mentioned this pull request Dec 2, 2023
7 tasks
@Fil Fil mentioned this pull request Dec 4, 2023
14 tasks
@Fil
Copy link
Contributor

Fil commented Dec 4, 2023

In a follow-up PR we'll need to document each of the modules in the placeholders (docs/libs/) #307

docs/lib/sqlite.md Outdated Show resolved Hide resolved
@mbostock
Copy link
Member Author

mbostock commented Dec 4, 2023

I wonder if it would be possible to unify the way the symbols are exposed.

We don’t control how third-party libraries and bundled, and we’re already doing The Right Thing(tm) here in my opinion for our own libraries. I can’t really articulate when it’s appropriate to use default exports, but it feels right when the name of the library matches the single thing that it exports (as with dot and mermaid and tex). We can always add redundant named exports in the future. And namespace imports are used when you want to import multiple things at the same time, as with d3.

On the libraries themselves, we’re using the names they’re published as to npm. The only thing I think we should fix there is maybe republishing npm:htl as npm:@observablehq/htl to match our other libraries.

@mbostock mbostock changed the title vanilla stdlib; server-side render implicit stylesheets vanilla stdlib; server-side render implicit stylesheets; async Generators; width(target) Dec 4, 2023
@mbostock mbostock requested review from Fil and cinxmo December 4, 2023 16:45
@mbostock
Copy link
Member Author

mbostock commented Dec 4, 2023

Thank for you the feedback. This is ready for another look. 🙏

@cinxmo
Copy link
Contributor

cinxmo commented Dec 4, 2023

Tested the production build and saw this 👀:
Screenshot 2023-12-04 at 12 03 27 PM

@cinxmo
Copy link
Contributor

cinxmo commented Dec 4, 2023

Re: 2.5. Remove rewriting of fetch?
The current rewriting forces users to change the path to start with ./, ../, or /. Otherwise, the behavior is the same?

@mbostock mbostock marked this pull request as draft December 4, 2023 17:19
@mbostock
Copy link
Member Author

mbostock commented Dec 4, 2023

Good catch @cinxmo. There’s some additional copying over of files in _observablehq that needs to happen during build. I’ll work on this.

@mbostock
Copy link
Member Author

mbostock commented Dec 4, 2023

The current rewriting forces users to change the path to start with ./, ../, or /. Otherwise, the behavior is the same?

We require that for fetch but not for FileAttachment; we probably shouldn’t have introduced that restriction for fetch but I was thinking about it like import — and import is different because it supports import maps (and we wouldn’t support “fetch maps” for fetching bare module specifiers e.g.). If we remove fetch rewriting I wouldn’t have that restriction for FileAttachment since we know that the FileAttachment constructor always takes a relative or absolute path.

@mbostock mbostock marked this pull request as ready for review December 4, 2023 20:36
@mbostock
Copy link
Member Author

mbostock commented Dec 4, 2023

Ready for another look @cinxmo @Fil! Thanks again.

@mbostock mbostock merged commit 7591028 into main Dec 4, 2023
1 check passed
@mbostock mbostock deleted the mbostock/stdlib branch December 4, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated features from the standard library Generators.asyncInput
3 participants