Rename errgo.ts to result.ts and move assets/ to ESM - #42
Merged
Conversation
The vendored `errgo.ts` name said nothing about what the module does.
Every export in it is about the `Result<T, E>` tuple, and its siblings in
assets/js/ are named as nouns-by-concern (theme, dates, routes, sentry),
so `result.ts` fits both. Upstream still calls the package `errgo`; only
the local filename differs, and CLAUDE.md records that.
While updating the `test:unit` script for the rename it was hardcoded to
a single file, so it now globs `js/**/*.test.ts`. The glob is scoped to
js/ rather than relying on bare `node --test` discovery, which today only
skips the Playwright suite because those files happen to end in .spec.ts.
That script also carried --disable-warning=MODULE_TYPELESS_PACKAGE_JSON,
suppressing a warning Node emits once per test file because assets/ had
no "type" field. Rather than keep suppressing it, assets/ is now
"type": "module" and the five CommonJS build scripts are explicitly .cjs.
This removes the reparse overhead, makes the module system of each file
explicit, and defuses the trap where Node's own warning text tells you to
add "type": "module" without mentioning it breaks all five scripts.
Two things the move required:
* watch-ssr-pages resolved the generator via path.join at runtime, so
the .cjs rename had to be reflected there or the dev watcher would
have silently stopped regenerating the page registry.
* e2e/global-setup.ts used __dirname, which does not exist in ESM.
Playwright loads it as ESM under "type": "module", so global setup
threw ReferenceError and would have failed the entire e2e suite.
It now uses import.meta.dirname.
Verified: mix precommit, 21/21 Playwright e2e, full mix assets.deploy
(exercising upload-sourcemaps and compress-assets), both dev watchers
regenerating, and priv/ssr.js still loading as CommonJS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The vendored
errgo.tsname said nothing about what the module does. Every export in it is about theResult<T, E>tuple, and its siblings in assets/js/ are named as nouns-by-concern (theme, dates, routes, sentry), soresult.tsfits both. Upstream still calls the packageerrgo; only the local filename differs, and CLAUDE.md records that.While updating the
test:unitscript for the rename it was hardcoded to a single file, so it now globsjs/**/*.test.ts. The glob is scoped to js/ rather than relying on barenode --testdiscovery, which today only skips the Playwright suite because those files happen to end in .spec.ts.That script also carried --disable-warning=MODULE_TYPELESS_PACKAGE_JSON, suppressing a warning Node emits once per test file because assets/ had no "type" field. Rather than keep suppressing it, assets/ is now "type": "module" and the five CommonJS build scripts are explicitly .cjs. This removes the reparse overhead, makes the module system of each file explicit, and defuses the trap where Node's own warning text tells you to add "type": "module" without mentioning it breaks all five scripts.
Two things the move required:
Verified: mix precommit, 21/21 Playwright e2e, full mix assets.deploy (exercising upload-sourcemaps and compress-assets), both dev watchers regenerating, and priv/ssr.js still loading as CommonJS.