From dd529c7e2e06a0dc5ea13fb9ee6dfada6a198de8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 7 May 2026 22:47:29 +0000 Subject: [PATCH 1/6] Emit native TC-39 stage 3 decorators and automatic JSX runtime in @buildcanada/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. --- packages/charts/build.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/charts/build.ts b/packages/charts/build.ts index 8b174060f27..9606ca07512 100644 --- a/packages/charts/build.ts +++ b/packages/charts/build.ts @@ -38,15 +38,19 @@ const transpileFile = async (srcPath: string, destPath: string) => { const result = await transform(content, { loader: isTsx ? "tsx" : "ts", format: "esm", - target: "esnext", + target: "es2022", + jsx: "automatic", sourcemap: "external", sourcefile: srcPath, - // Use legacy decorators mode for compatibility with mobx-react + // Emit native TC-39 stage 3 decorators. The codebase mixes mobx + // decorators (@computed, @action) with stage-3-style ones (@bind in + // utils/Util.ts), so the legacy emit path produces output that calls + // stage-3 decorators with legacy arguments at runtime. Stage 3 emit + // is what Storybook uses and matches the source authoring style. tsconfigRaw: { compilerOptions: { - experimentalDecorators: true, - emitDecoratorMetadata: false, - useDefineForClassFields: false, + experimentalDecorators: false, + useDefineForClassFields: true, }, }, }) From 253cd97c4497af4faaa0792064d14659b542ae3f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 16:13:31 +0000 Subject: [PATCH 2/6] Bump @buildcanada/charts to v0.3.9 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. --- packages/charts/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/charts/package.json b/packages/charts/package.json index b651cd1702a..a066eed337d 100644 --- a/packages/charts/package.json +++ b/packages/charts/package.json @@ -1,6 +1,6 @@ { "name": "@buildcanada/charts", - "version": "0.3.8", + "version": "0.3.9", "description": "A configurable data visualization library for creating interactive charts.", "type": "module", "main": "./dist/index.js", From 5626e5158432d0e0a34c4944ab0dac9efe19430b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 16:56:22 +0000 Subject: [PATCH 3/6] Fix CI test and chromatic jobs 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. --- package.json | 2 +- packages/charts/vitest.setup.ts | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index cf3b3f2b0e2..a99a3ccb388 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "node": ">=22.0" }, "scripts": { - "build": "storybook build -o storybook-static", + "build": "storybook build -o storybook-static --stats-json", "build:packages": "bun run build:colours && bun run build:components && bun run build:charts", "build:colours": "cd packages/colours && bun run build", "build:components": "cd packages/components && bun run build", diff --git a/packages/charts/vitest.setup.ts b/packages/charts/vitest.setup.ts index b9e76229961..a24e5c203fa 100644 --- a/packages/charts/vitest.setup.ts +++ b/packages/charts/vitest.setup.ts @@ -1 +1,19 @@ import "@testing-library/jest-dom/vitest" + +// happy-dom 20.x raises an AbortError from `teardownWindow` whenever a test +// finishes with an in-flight fetch (it calls AsyncTaskManager.abortAll, which +// rejects each pending fetch). The rejection has no .catch handler since the +// caller is gone, so it surfaces as an unhandledRejection. +// +// Vitest itself still reports every test as passing and exits 0, but newer +// bun versions (CI runs `bun-version: latest`) treat that unhandledRejection +// as a fatal error and terminate the workspace runner with exit 1. Swallow +// the specific teardown-induced AbortError so CI exit code matches the test +// outcome. +if (typeof process !== "undefined" && process.on) { + process.on("unhandledRejection", (reason: unknown) => { + const err = reason as { name?: string; message?: string } | null + if (err && err.name === "AbortError") return + throw reason + }) +} From c7bcf734835bbab793f39b5a78c5ccb794811894 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 17:04:52 +0000 Subject: [PATCH 4/6] Stub fetch in vitest setup to avoid happy-dom teardown TLS error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- packages/charts/vitest.setup.ts | 37 +++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/charts/vitest.setup.ts b/packages/charts/vitest.setup.ts index a24e5c203fa..950915343df 100644 --- a/packages/charts/vitest.setup.ts +++ b/packages/charts/vitest.setup.ts @@ -1,19 +1,24 @@ import "@testing-library/jest-dom/vitest" +import { vi } from "vitest" -// happy-dom 20.x raises an AbortError from `teardownWindow` whenever a test -// finishes with an in-flight fetch (it calls AsyncTaskManager.abortAll, which -// rejects each pending fetch). The rejection has no .catch handler since the -// caller is gone, so it surfaces as an unhandledRejection. +// `getUserCountryInformation` (utils/Util.ts) calls `fetchWithRetry` against +// COUNTRY_DETECTION_URL when components like GlobalEntitySelector mount. +// Under happy-dom 20.x, that opens a real TLS socket, which is then aborted +// when vitest tears the window down — emitting a synchronous `error` event +// from Node's TLSSocket layer that escapes the Promise chain and surfaces as +// an Uncaught Exception. Vitest aggregates that into the run's exit code, +// so every test passes but the process exits 1 (and bun's workspace runner +// prints `error: script "test" exited with code 1`). // -// Vitest itself still reports every test as passing and exits 0, but newer -// bun versions (CI runs `bun-version: latest`) treat that unhandledRejection -// as a fatal error and terminate the workspace runner with exit 1. Swallow -// the specific teardown-induced AbortError so CI exit code matches the test -// outcome. -if (typeof process !== "undefined" && process.on) { - process.on("unhandledRejection", (reason: unknown) => { - const err = reason as { name?: string; message?: string } | null - if (err && err.name === "AbortError") return - throw reason - }) -} +// Stub fetch with an immediate rejection so no real socket is ever opened. +// `getUserCountryInformation` already swallows fetch failures via its own +// `.catch(() => undefined)`, so behavior is unchanged for tests that don't +// otherwise rely on fetch (none currently do). +vi.stubGlobal( + "fetch", + vi.fn(() => + Promise.reject( + new Error("fetch is disabled in tests (vitest.setup.ts)"), + ), + ), +) From 654be41e49f8630dca055004cfcc4a30791d8acb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 17:08:00 +0000 Subject: [PATCH 5/6] Resolve, don't reject, in fetch stub to avoid CI unhandledRejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- packages/charts/vitest.setup.ts | 44 ++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/packages/charts/vitest.setup.ts b/packages/charts/vitest.setup.ts index 950915343df..ea9b96bb395 100644 --- a/packages/charts/vitest.setup.ts +++ b/packages/charts/vitest.setup.ts @@ -1,24 +1,34 @@ import "@testing-library/jest-dom/vitest" import { vi } from "vitest" -// `getUserCountryInformation` (utils/Util.ts) calls `fetchWithRetry` against -// COUNTRY_DETECTION_URL when components like GlobalEntitySelector mount. -// Under happy-dom 20.x, that opens a real TLS socket, which is then aborted -// when vitest tears the window down — emitting a synchronous `error` event -// from Node's TLSSocket layer that escapes the Promise chain and surfaces as -// an Uncaught Exception. Vitest aggregates that into the run's exit code, -// so every test passes but the process exits 1 (and bun's workspace runner -// prints `error: script "test" exited with code 1`). +// Stub global fetch so tests never open real network sockets. // -// Stub fetch with an immediate rejection so no real socket is ever opened. -// `getUserCountryInformation` already swallows fetch failures via its own -// `.catch(() => undefined)`, so behavior is unchanged for tests that don't -// otherwise rely on fetch (none currently do). +// Why: components like GlobalEntitySelector call getUserCountryInformation +// (utils/Util.ts) on mount, which fetches COUNTRY_DETECTION_URL. Under +// happy-dom 20.x that opens a real Node TLSSocket; vitest's per-file window +// teardown then aborts the socket and the resulting synchronous 'error' +// event escapes the Promise chain and surfaces as a vitest "Uncaught +// Exception". Tests all pass but the run exits 1, which CI's +// `bun-version: latest` propagates as +// `error: script "test" exited with code 1`. +// +// Resolve (rather than reject) with a benign empty-JSON response so the stub +// can't itself create unhandled rejections in callers that don't .catch. +// getUserCountryInformation parses .json() and reads .country, which is +// undefined in {}, matching the production "could-not-detect" branch. vi.stubGlobal( "fetch", - vi.fn(() => - Promise.reject( - new Error("fetch is disabled in tests (vitest.setup.ts)"), - ), - ), + vi.fn(async () => ({ + ok: true, + status: 200, + statusText: "OK", + headers: new Headers({ "Content-Type": "application/json" }), + json: async () => ({}), + text: async () => "", + arrayBuffer: async () => new ArrayBuffer(0), + blob: async () => new Blob([]), + clone() { + return this + }, + })) as unknown as typeof fetch, ) From e968d09c405d36bba8c19b5c159a6437da08fd92 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 19:45:42 +0000 Subject: [PATCH 6/6] Build packages before running tests in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/ci.yml | 3 +++ packages/charts/vitest.setup.ts | 33 --------------------------------- 2 files changed, 3 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa85c55332e..450f2c18600 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,9 @@ jobs: - name: Install dependencies run: bun install --frozen-lockfile + - name: Build packages + run: bun run build:packages + - name: Run tests run: bun run test diff --git a/packages/charts/vitest.setup.ts b/packages/charts/vitest.setup.ts index ea9b96bb395..b9e76229961 100644 --- a/packages/charts/vitest.setup.ts +++ b/packages/charts/vitest.setup.ts @@ -1,34 +1 @@ import "@testing-library/jest-dom/vitest" -import { vi } from "vitest" - -// Stub global fetch so tests never open real network sockets. -// -// Why: components like GlobalEntitySelector call getUserCountryInformation -// (utils/Util.ts) on mount, which fetches COUNTRY_DETECTION_URL. Under -// happy-dom 20.x that opens a real Node TLSSocket; vitest's per-file window -// teardown then aborts the socket and the resulting synchronous 'error' -// event escapes the Promise chain and surfaces as a vitest "Uncaught -// Exception". Tests all pass but the run exits 1, which CI's -// `bun-version: latest` propagates as -// `error: script "test" exited with code 1`. -// -// Resolve (rather than reject) with a benign empty-JSON response so the stub -// can't itself create unhandled rejections in callers that don't .catch. -// getUserCountryInformation parses .json() and reads .country, which is -// undefined in {}, matching the production "could-not-detect" branch. -vi.stubGlobal( - "fetch", - vi.fn(async () => ({ - ok: true, - status: 200, - statusText: "OK", - headers: new Headers({ "Content-Type": "application/json" }), - json: async () => ({}), - text: async () => "", - arrayBuffer: async () => new ArrayBuffer(0), - blob: async () => new Blob([]), - clone() { - return this - }, - })) as unknown as typeof fetch, -)