Update TypeScript compiler options for decorator compatibility#7
Merged
Update TypeScript compiler options for decorator compatibility#7
Conversation
…ildcanada/charts build The published @buildcanada/charts package was unusable in consumer apps (notably Next.js with Turbopack) because the build emitted JS that mixed incompatible decorator and JSX modes: - esbuild was configured with experimentalDecorators: true (TS legacy emit), but utils/Util.ts:bind is authored as a TC-39 stage 3 decorator using ClassMethodDecoratorContext.addInitializer. At runtime that threw 'context.addInitializer is not a function' as soon as any module using @Bind (e.g. grapher/modal/Modal.tsx) was loaded. - esbuild's classic JSX runtime emitted React.createElement / React.Fragment calls but the source files only import named exports from 'react' (e.g. 'import { Component } from "react"'), so consumers saw 'ReferenceError: React is not defined' inside ControlsRow and others. Switch the esbuild transform to: - target es2022 with experimentalDecorators: false + useDefineForClassFields: true, matching what Storybook's vite/babel pipeline uses. MobX 6 decorators (@computed, @action, @action.bound) work correctly under stage 3 emit, and so does the @Bind decorator. - jsx: 'automatic', which imports from react/jsx-runtime and removes the requirement that consuming code keep a React default in scope.
Releases the build pipeline fix (TC-39 stage 3 decorator emit + automatic JSX runtime) so that the package is consumable from Next.js / other apps that don't ship their own decorator/JSX runtime config.
The PR's substantive change (decorator + JSX runtime emit) doesn't
affect CI behavior, but two pre-existing CI bugs were red on this
branch and needed fixing to land the change:
- test: vitest reports 1203/1203 pass and exits 0, but happy-dom 20.x's
teardownWindow aborts in-flight fetches and the resulting AbortError
has no .catch handler. CI's bun-version: latest treats that
unhandledRejection as fatal and exits 1 even though every test
passed. Add a process.on('unhandledRejection') in vitest.setup.ts
that swallows the AbortError class specifically and rethrows
everything else, so real failures still surface.
- chromatic: the workflow runs with onlyChanged: true (TurboSnap),
which requires preview-stats.json from Storybook's build, but the
build script omitted --stats-json. Chromatic was bailing out before
ever comparing snapshots. Add --stats-json to the storybook build
script; verified locally that storybook-static/preview-stats.json
is now emitted.
Diagnosis from the CI 'Uncaught Exception' (run 25568274185, job 75057203243): NetworkError: Failed to execute "fetch()" on "Window" with URL "https://detect-country.example.com/": The operation was aborted. at Fetch.onError happy-dom/lib/fetch/Fetch.js:540:21 at emitErrorEvent node:_http_client:108:11 at TLSSocket.socketErrorListener node:_http_client:575:5 This error originated in "GlobalEntitySelector.test.tsx". When GlobalEntitySelector mounts in tests it calls getUserCountryInformation (utils/Util.ts), which fires a real fetch against COUNTRY_DETECTION_URL. Under happy-dom 20.x that opens a Node TLSSocket. When vitest tears the window down at end-of-file the socket is aborted, and the resulting synchronous TLS 'error' event escapes the Promise chain (which is otherwise correctly .catch'd in the source) and surfaces as a vitest 'Uncaught Exception' — every test still passes but the run exits non-zero. The earlier setup-file unhandledRejection guard didn't catch this because the failure isn't a Promise rejection; it's a sync error event from the socket layer. Stub globalThis.fetch with vi.stubGlobal so no real socket is ever opened. getUserCountryInformation already swallows fetch failures via .catch(() => undefined), so the production behavior under test (couldn't detect country -> undefined) is unchanged. No existing test relies on a non-stubbed fetch (verified by grep).
Last fix replaced fetch with a Promise.reject stub. Locally that ran
clean, but on CI the rejected promise itself created unhandled
rejections in any caller that doesn't .catch — and bun-version: latest
propagates that to a non-zero exit.
Resolve to a benign empty-JSON Response-shaped object instead, so:
- no real TLS socket is opened (happy-dom teardown can't trip a TLS
error event)
- no rejected promise is produced by the stub itself
- consumers of getUserCountryInformation see {} -> .country undefined,
matching the production 'could-not-detect' branch that already
drives a .catch(() => undefined).
The actual CI test failure (which I previously misdiagnosed as fetch / unhandled-rejection / decorator-related) is in build.test.ts for both @buildcanada/components and @buildcanada/charts: Failed to resolve import './dist/index.js' from 'build.test.ts' These are integration tests that import from dist/ and check the build output is correct (.js extensions on imports, expected exports available, expected directory structure). The test files document this themselves: 'Run after `bun run build` to verify the dist/ output.' The CI workflow runs `bun run test` on a clean checkout where dist/ has never been produced, so vite's import-analyzer fails to resolve './dist/index.js'. Locally these passed for me only because I'd already run `bun run build:packages` for the unrelated TradingPost integration earlier in the session. Add a 'Build packages' step before 'Run tests' in the CI workflow that runs `bun run build:packages` (which builds colours, components, and charts in dependency order). This matches what the build.test.ts files actually need and what a developer would do locally. Also revert the speculative fetch stub in vitest.setup.ts. With the real failure cause known, that change isn't needed: the per-file happy-dom teardown AbortError noise we saw in the prior log is benign — vitest reports every test as passing and exits 0, and bun doesn't actually exit 1 because of it. The exit-1 was always the build.test.ts suite failure.
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
2 participants
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.
Context
This change updates the TypeScript build configuration in
packages/charts/build.tsto properly handle decorators and modernize the compilation target.Changes
esnexttoes2022for better compatibility and predictabilityjsx: "automatic"to use the modern JSX transformexperimentalDecorators: false(wastrue)emitDecoratorMetadata: false(no longer needed)useDefineForClassFields: true(wasfalse)The codebase mixes mobx decorators (
@computed,@action) with stage-3-style decorators (e.g.,@bindinutils/Util.ts). The legacy emit path was producing output that called stage-3 decorators with legacy arguments at runtime. Stage 3 emit aligns with how Storybook handles decorators and matches the source authoring style.Testing guidance
@computed,@action) continue to function as expectedChecklist
https://claude.ai/code/session_01H5n1EL6QDosMXGTS8UndR9