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

more readable _node urls (and cache structure) #1167

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Mar 29, 2024

Since we're free to name the resolutions of /_node/ files (bare module specifiers) as we want, why not give them names that are easy to read in the browser's network tab and make cache busting easier.

Here's what this combination gives when using node_modules versions of duckdb-wasm, d3-array and internmap@1 (when d3-array requires internmap@2, generating two files):

<link rel="modulepreload" href="./_node/[email protected]/77f92c63/internmap.js">
<link rel="modulepreload" href="./_node/[email protected]/a6083063/d3-array.js">
<link rel="modulepreload" href="./_node/@duckdb/[email protected]/5e9b2fc6/duckdb-wasm.js">
<link rel="modulepreload" href="./_node/[email protected]/e0fb710d/internmap.js">
<link rel="modulepreload" href="./_node/[email protected]/8d522eb1/apache-arrow.js">
<link rel="modulepreload" href="./_node/[email protected]/b26fe5c0/tslib.js">
<link rel="modulepreload" href="./_node/[email protected]/ada4f5af/flatbuffers.js">


...

const duckdb = await import("./_node/@duckdb/[email protected]/5e9b2fc6/duckdb-wasm.js");
const MANUAL_BUNDLES = {
  mvp: {
    mainModule: new URL("./_node/@duckdb/[email protected]/b08f36ee/duckdb-mvp.wasm", location).href,
    mainWorker: new URL("./_node/@duckdb/[email protected]/b6de1549/duckdb-browser-mvp.worker.js", location).href
  },
  eh: {
    mainModule: new URL("./_node/@duckdb/[email protected]/75338043/duckdb-eh.wasm", location).href,
    mainWorker: new URL("./_node/@duckdb/[email protected]/d80d9fe7/duckdb-browser-eh.worker.js", location).href
  },
};

This also removes the extractNodeSpecifier function which was has no more purpose.

(Leaving as a draft for now because it might depend on whether we are doing the same for /_npm/, see #1165 (comment).)

@mbostock
Copy link
Member

This seems like purely an aesthetic choice, so I’m not as motivated to change this as I am in #1165 which is fixing a preview server crash and broken imports (e.g., npm:mime/lite). We can already consider everything in _node (and _npm) immutable even though it doesn’t have an explicit content hash in the path because the published contents of npm packages should never change. And you can change the devtools to show the path rather than the file name to avoid seeing _esm.js. That’s not to say this PR isn’t an improvement, perhaps; it’s just I’m going to prioritize working on the other changes. 🙏

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.

2 participants