Fix ESM conversion correctness ahead of launch - #496
Conversation
Feed esbuild a synthetic ESM entry for CommonJS modules instead of regex-splicing named exports onto the bundled output. esbuild's __toESM interop now resolves the default export (unwrapping exports.default for __esModule modules), named exports are real ESM exports that survive minification, inline sourcemaps are no longer truncated, and reexports of other packages become export * statements that resolve at runtime.
Bundling a subpath's bare self-reference produced a private copy of the package core, so preact/hooks and preact ran as two module instances. Keep bare self-references external, pin them to the version being built, and skip the registry lookup for exact dependency versions.
The emitted @jspm/core@2/nodelibs/browser/* URLs never resolved through @jspm/core's exports map (the wildcard doubled the browser segment), so every polyfill import 404ed. Emit the bare nodelibs subpath instead and let the exports map pick the browser variant. Cover the full set of builtins jspm provides so bare specifiers like querystring, vm, and tty polyfill instead of resolving to unrelated same-named npm packages, and reject any remaining builtin (including unknown node: specifiers, which previously passed through as unresolvable URLs) with a clear error. Deletes the hardNodeBuiltins list, which was fully shadowed by the polyfill map and unreachable.
Drop the ?bundle, ?standalone, ?no-bundle, ?keep-names, and ?ignore-annotations params (400 with a clear diagnostic), the denonext target, and the &query-in-path syntax. The bundle-mode params were no-ops (the bundler never consulted them), no-bundle produced broken output for CommonJS sources, and each param multiplied cache keys. Builds now always use smart bundling; ?external covers dependency control. Corpus cases and docs updated to match.
Dependency and polyfill URLs now include the target param and use the canonical sorted param order, so browsers no longer pay a 301 redirect on every dependency import. Polyfill URLs pin the resolved @jspm/core version for deterministic builds. Adds a short-lived in-process packument cache and parallelizes specifier rewriting, which previously fetched full packuments serially per import.
Dependency versions like npm:string-width@^4.2.0, workspace: ranges, and git/URL specifiers previously produced broken URLs such as /string-width-cjs@npm:string-width@^4.2.0. npm: aliases now resolve to the aliased package and protocol specifiers fall back to the latest published version. Package names are also no longer force-lowercased: the registry is case-sensitive and legacy uppercase packages (JSONStream) often coexist with unrelated lowercase ones, so the old behavior silently served the wrong package. Lookups try the name as given and fall back to lowercase, and requests redirect to the registry's canonical casing.
New browser smoke cases: preact root + hooks sharing one module instance, minified CommonJS named imports, Babel default-export unwrapping, and cross-package reexports. New seed corpus cases for the same classes plus npm: alias dependencies and legacy uppercase package names — the blind spots that let the interop bugs through.
Build artifacts, CSS modules, and worker wrappers are only served at exact-version canonical URLs, so serve them with immutable year-long caching instead of the 60s/300s TTL that forced browsers to refetch every module each minute. Redirects, metadata, and errors stay short-lived so resolution changes roll out quickly. ?meta integrity is now computed from the same bytes the module URL serves: it prefers the edge-cached artifact and stores freshly built artifacts under the module URL, so the SRI hash handed to users matches what a subsequent module request in the same colo returns.
Correctness: - Reject ?meta combined with ?worker/?css/?module; the meta path was building the plain artifact and caching it (immutable) under the module URL those params actually route differently, poisoning the edge cache. The integrity cache also now skips development/test modes. - Bare self-references that resolve to the entry being built are bundled again (externalizing them made a module import its own URL mid-evaluation), and blocked exports-map subpaths fail the build with a clear diagnostic again. Extensionless spellings of the entry serve the in-memory entry code. - The require shim keeps the full namespace for __esModule modules instead of double-unwrapping exports.default, so required Babel-built dependencies keep their named properties. - Package-name lookups no longer hit Object.prototype members: the polyfill table is a Map and dependency/alias/override lookups use own-property access (a dependency named 'constructor' previously crashed the build). - resolveDependencyVersion returns semver's cleaned form (v1.2.3 -> 1.2.3), never throws (a registry failure degrades to the raw range), and unpolyfilled or unknown node builtins map to @jspm/core's empty stub instead of failing the whole build. - Builds whose dependency URLs could not all be pinned are served with the short cache policy instead of immutable, so degraded artifacts heal. Unknown query params are stripped during URL normalization so they cannot multiply immutable cache entries or force extra builds. - The registry doc's name is only trusted as a case variant of the requested package, and the files service validates targets instead of passing unknown strings to esbuild. Efficiency: - buildEsmModule extracts the tarball once and reads everything from the extracted directory (previously 4-8 tarball downloads per build). - The packument cache uses abbreviated registry documents, evicts oldest-first instead of clearing wholesale, and only ever evicts its own entry on failure. - Module, raw-file, and transform responses stream instead of buffering. Consistency: - normalizeSearchParams is exported from unpkg-worker and used by the esm worker and dependency-URL rewriting, so canonical param ordering has one implementation. - Removed the stale path-query corpus case, restored the corpora to prettier formatting, dropped empty scenario arrays in the generator, and updated the www docs for the removed params.
- eval and arguments cannot be import bindings; drop them from CommonJS named exports instead of generating a syntactically invalid interop entry that failed the whole build. - Restore .cjs (and .json) probing for extensionless entry and require resolution, lost in the single-extraction refactor. - The lowercase package-name fallback only fires on a definitive 404; a transient registry error no longer silently resolves an unrelated same-named lowercase package. Dependency version resolution drops the fallback entirely since published package.json files already carry registry-canonical names. - Canonicalize flag param values (?min=1 -> ?min=) and comma-list params (conditions/exports/external order and duplicates) during URL normalization so equivalent spellings share one immutable cache entry instead of forcing separate year-long builds. - Test hygiene: the module-level packument cache is cleared after the rewrite suite and the mocked edge-cache store is cleared per test, so mocked registry data cannot leak across test files.
|
Ran a max-effort local adversarial review over this PR (10 finder angles + verification + a gap sweep). 21 verified findings, all fixed in three follow-up commits:
Full suite: 312 tests, 0 failures across all packages. Post-merge: deploy both workers + fly origin (and purge the CF cache — old cached artifacts emit dep URLs with now-removed params), then re-run |
- Make the default target implicit in canonical URLs. Browsers key the module map by request URL, so /preact@10.26.4 redirecting to a ?target=es2022 variant while rewritten dependency imports used the variant directly loaded the same module twice — preact hooks got a different preact instance than the page's own import and crashed. Default-target URLs now serve directly (also removing the redirect hop entirely); only non-default targets appear in URLs. - Shim the Buffer and process globals in browser builds via esbuild inject (and define global as globalThis). CJS packages that reference the Node globals without requiring them (JSONStream) previously threw ReferenceError in browsers. The shim imports are marked side-effect free so bundles that never touch them stay unchanged, and the local 'process' function preact defines is left alone. - Forward the worker's own origin to /transform so inline-script transforms emit imports against the serving deployment instead of the production origin — dev and CI runs are now hermetic. - Make the is-hotkey smoke assertion platform-independent (shift+s; 'mod' resolves to meta on macOS but ctrl on Linux CI) and point the two redirect-query-preservation corpus cases at range versions, since their exact-version URLs no longer redirect. Verified against the CI-equivalent local stack: seed compat suite 37/37, browser smoke 35/35.
|
CI is green as of Canonical URLs no longer carry Also in this commit: browser builds shim the Local verification against the CI-equivalent stack: seed compat suite 37/37, browser smoke 35/35, unit suites 314/314. |
Pre-launch audit of esm.unpkg.com found several verified correctness bugs in the ESM conversion pipeline plus esm.sh legacy baggage worth shedding while the service is still in beta. This PR fixes all of them. Every fix was verified end-to-end against a local build of the service, and the full repo suite passes.
Correctness fixes
f5cf2345): CJS entries are built through a synthetic ESM interop entry instead of regex-splicing named exports onto esbuild output. Fixes: wrong default export for Babel-compiled packages (exports.defaultnow unwraps via__toESM),?minsilently dropping all CJS named exports, inline sourcemaps being truncated on CJS builds, and cross-package CJS reexports losing named exports (import { Buffer } from "safe-buffer"now works viaexport * from).85ac2793): bare self-references stay external and version-pinned instead of being bundled into subpath builds.preact/hookspreviously inlined a private copy of preact core, breaking hooks whenever the app also importedpreact.b849d181): the emitted@jspm/core@2/nodelibs/browser/*URLs never resolved through @jspm/core's exports map, so every builtin polyfill import 404ed in production. Polyfill URLs now use the exports-map subpath and pin the resolved version; the map covers the full jspm nodelibs set (previouslyquerystring,vm,tty,constantsetc. resolved to unrelated same-named npm packages); unpolyfillable builtins and unknownnode:specifiers return a clear 422 instead of reaching the browser as unresolvable specifiers.293db50e):npm:aliases resolve to the aliased package (previously emitted broken URLs like/string-width-cjs@npm:string-width@^4.2.0), workspace/git/URL specifiers fall back to the latest published version, and package names are no longer force-lowercased — the registry is case-sensitive and/JSONStreampreviously served the unrelatedjsonstreampackage.API surface cuts (beta breaking changes)
822bc6c6removes?bundle,?standalone,?no-bundle,?keep-names,?ignore-annotations(now 400 with a clear diagnostic), thedenonexttarget, and the&query-in-path syntax. The bundle-mode params were unimplemented no-ops and?no-bundleproduced broken output for CJS.?metano longer reports abuild.bundlefield and theX-UNPKG-Bundle-Modeheader is gone.Performance
961cc1c3: dependency and polyfill URLs are emitted in canonical form (target included, params sorted), eliminating a 301 redirect on every dependency import. Adds an in-process packument cache and parallelizes specifier rewriting (previously serial multi-MB packument fetches per import).Coverage
e7d0ad34: new browser-smoke and corpus cases for the exact blind spots that let these bugs through: root+subpath shared instances,?minnamed imports, default-export unwrapping, cross-package reexports, npm: aliases, and uppercase package names.After merge
pnpm test:esm-compatand the browser smoke against the fresh deployment before announcing.