From 65a2edfef84c7f715546b4b7041f57b7ce3f62b4 Mon Sep 17 00:00:00 2001 From: xrendan Date: Thu, 5 Feb 2026 12:18:42 -0700 Subject: [PATCH 1/2] Fix MobX decorator incompatibility: switch from legacy to TC39 stage 3 decorators The esbuild config in build.ts was emitting legacy TypeScript decorators (__decorateClass), but MobX 6.15 expects TC39 stage 3 decorators, causing "context.addInitializer is not a function" at runtime. - Set experimentalDecorators: false in build.ts esbuild config - Fix 11 source files that mixed @decorator syntax with makeObservable annotation args (not allowed with TC39 decorators) - Use @observable accessor for non-serializable classes - Use all-annotations approach for AxisConfig/ChartDimension to preserve Object.keys() serialization compatibility Co-Authored-By: Claude Opus 4.5 --- packages/charts/build.ts | 6 ++---- .../charts/src/explorer/ExplorerDecisionMatrix.ts | 9 +++------ packages/charts/src/grapher/axis/AxisConfig.ts | 15 ++++++++++----- .../charts/src/grapher/chart/ChartDimension.ts | 14 +++++++++----- packages/charts/src/grapher/footer/Footer.tsx | 6 ++---- .../charts/src/grapher/mapCharts/MapChart.tsx | 9 +++------ .../src/grapher/selection/SelectionArray.ts | 6 ++---- .../src/grapher/slideInDrawer/SlideInDrawer.tsx | 6 ++---- .../src/grapher/stackedCharts/MarimekkoChart.tsx | 9 +++------ .../src/grapher/stackedCharts/StackedBarChart.tsx | 12 ++++-------- .../grapher/stackedCharts/StackedBarSegment.tsx | 6 ++---- .../stackedCharts/StackedDiscreteBarChart.tsx | 9 +++------ 12 files changed, 45 insertions(+), 62 deletions(-) diff --git a/packages/charts/build.ts b/packages/charts/build.ts index 8b174060f27..aaab6609982 100644 --- a/packages/charts/build.ts +++ b/packages/charts/build.ts @@ -41,12 +41,10 @@ const transpileFile = async (srcPath: string, destPath: string) => { target: "esnext", sourcemap: "external", sourcefile: srcPath, - // Use legacy decorators mode for compatibility with mobx-react tsconfigRaw: { compilerOptions: { - experimentalDecorators: true, - emitDecoratorMetadata: false, - useDefineForClassFields: false, + experimentalDecorators: false, + useDefineForClassFields: true, }, }, }) diff --git a/packages/charts/src/explorer/ExplorerDecisionMatrix.ts b/packages/charts/src/explorer/ExplorerDecisionMatrix.ts index 18f27d112a3..cd87e075e56 100644 --- a/packages/charts/src/explorer/ExplorerDecisionMatrix.ts +++ b/packages/charts/src/explorer/ExplorerDecisionMatrix.ts @@ -91,12 +91,9 @@ const makeCheckBoxOption = ( // allow the user to navigate amongst charts. export class DecisionMatrix { table: CoreTable - currentParams: ExplorerChoiceParams = {} + @observable accessor currentParams: ExplorerChoiceParams = {} constructor(delimited: string, hash = "") { - makeObservable(this, { - currentParams: observable, - selectedRow: observable, - }) + makeObservable(this) this.choiceNameToControlTypeMap = makeChoicesMap(delimited) this.table = new CoreTable(parseDelimited(dropColumnTypes(delimited)), [ // todo: remove col def? @@ -413,7 +410,7 @@ export class DecisionMatrix { : this.table.indexOf(this.firstMatch) } - selectedRow: any = {} + @observable accessor selectedRow: any = {} private toControlOption( choiceName: ChoiceName, diff --git a/packages/charts/src/grapher/axis/AxisConfig.ts b/packages/charts/src/grapher/axis/AxisConfig.ts index 8ceedc843fd..28fb80b8f2f 100644 --- a/packages/charts/src/grapher/axis/AxisConfig.ts +++ b/packages/charts/src/grapher/axis/AxisConfig.ts @@ -93,7 +93,12 @@ export class AxisConfig { constructor(props?: AxisConfigInterface, axisManager?: AxisManager) { super() - makeObservable(this) + makeObservable(this, { + fontSize: computed, + constrainedMin: computed, + constrainedMax: computed, + domain: computed, + }) this.updateFromObject(props) this.axisManager = axisManager } @@ -138,12 +143,12 @@ export class AxisConfig return obj } - @computed get fontSize(): number { + get fontSize(): number { return this.axisManager?.fontSize || BASE_FONT_SIZE } // A log scale domain cannot have values <= 0, so we double check here - @computed private get constrainedMin(): number { + private get constrainedMin(): number { if (this.scaleType === ScaleType.log && (this.min ?? 0) <= 0) return Infinity return this.min ?? Infinity @@ -157,13 +162,13 @@ export class AxisConfig return false } - @computed private get constrainedMax(): number { + private get constrainedMax(): number { if (this.scaleType === ScaleType.log && (this.max || 0) <= 0) return -Infinity return this.max ?? -Infinity } - @computed get domain(): [number, number] { + get domain(): [number, number] { return [this.constrainedMin, this.constrainedMax] } diff --git a/packages/charts/src/grapher/chart/ChartDimension.ts b/packages/charts/src/grapher/chart/ChartDimension.ts index b6734565f6b..d50f2a56d4e 100644 --- a/packages/charts/src/grapher/chart/ChartDimension.ts +++ b/packages/charts/src/grapher/chart/ChartDimension.ts @@ -65,14 +65,18 @@ export class ChartDimension ) { super() - makeObservable(this, { + makeObservable(this, { _slug: observable, + table: computed, + slug: computed, + column: computed, + columnSlug: computed, }) this.manager = manager if (obj) this.updateFromObject(obj) } - @computed private get table(): ChartsTable { + private get table(): ChartsTable { return this.manager.table } @@ -102,7 +106,7 @@ export class ChartDimension // Do not persist yet, until we migrate off VariableIds _slug: ColumnSlug | undefined = undefined - @computed get slug(): ColumnSlug { + get slug(): ColumnSlug { if (this._slug) return this._slug return getDimensionColumnSlug(this.variableId, this.targetYear) } @@ -111,11 +115,11 @@ export class ChartDimension this._slug = value } - @computed get column(): CoreColumn { + get column(): CoreColumn { return this.table.get(this.columnSlug) } - @computed get columnSlug(): string { + get columnSlug(): string { return this.slug ?? this.variableId.toString() } } diff --git a/packages/charts/src/grapher/footer/Footer.tsx b/packages/charts/src/grapher/footer/Footer.tsx index 38758b07bbc..5d47dfb4ace 100644 --- a/packages/charts/src/grapher/footer/Footer.tsx +++ b/packages/charts/src/grapher/footer/Footer.tsx @@ -83,9 +83,7 @@ abstract class AbstractFooter< constructor(props: Props) { super(props) - makeObservable(this, { - tooltipTarget: observable.ref, - }) + makeObservable(this) } @computed protected get manager(): FooterManager { @@ -318,7 +316,7 @@ abstract class AbstractFooter< } base = React.createRef() - tooltipTarget: { x: number; y: number } | undefined = undefined + @observable.ref accessor tooltipTarget: { x: number; y: number } | undefined = undefined @action.bound private onMouseMove(e: MouseEvent): void { const cc = this.base.current?.querySelector(".cclogo") diff --git a/packages/charts/src/grapher/mapCharts/MapChart.tsx b/packages/charts/src/grapher/mapCharts/MapChart.tsx index 3c1c4fb46c9..6b0aed20c2c 100644 --- a/packages/charts/src/grapher/mapCharts/MapChart.tsx +++ b/packages/charts/src/grapher/mapCharts/MapChart.tsx @@ -90,10 +90,7 @@ export class MapChart constructor(props: MapChartProps) { super(props) - makeObservable(this, { - hoverBracket: observable, - tooltipState: observable, - }) + makeObservable(this) } /** @@ -101,9 +98,9 @@ export class MapChart * * Hovering a map bracket highlights all countries within that bracket on the map. */ - hoverBracket: MapBracket | undefined = undefined + @observable accessor hoverBracket: MapBracket | undefined = undefined - tooltipState = new TooltipState<{ + @observable accessor tooltipState = new TooltipState<{ featureId: string }>() diff --git a/packages/charts/src/grapher/selection/SelectionArray.ts b/packages/charts/src/grapher/selection/SelectionArray.ts index 8e1b49ec3b2..45a58392599 100644 --- a/packages/charts/src/grapher/selection/SelectionArray.ts +++ b/packages/charts/src/grapher/selection/SelectionArray.ts @@ -3,13 +3,11 @@ import { action, computed, observable, makeObservable } from "mobx" export class SelectionArray { constructor(selectedEntityNames: EntityName[] = []) { - makeObservable(this, { - selectedEntityNames: observable, - }) + makeObservable(this) this.selectedEntityNames = selectedEntityNames.slice() } - selectedEntityNames: EntityName[] + @observable accessor selectedEntityNames: EntityName[] = [] @computed get hasSelection(): boolean { return this.selectedEntityNames.length > 0 diff --git a/packages/charts/src/grapher/slideInDrawer/SlideInDrawer.tsx b/packages/charts/src/grapher/slideInDrawer/SlideInDrawer.tsx index c63fd333cbb..1a924fa58de 100644 --- a/packages/charts/src/grapher/slideInDrawer/SlideInDrawer.tsx +++ b/packages/charts/src/grapher/slideInDrawer/SlideInDrawer.tsx @@ -17,15 +17,13 @@ interface SlideInDrawerProps { @observer export class SlideInDrawer extends React.Component { - visible: boolean = this.props.active // true while the drawer is active and during enter/exit transitions + @observable.ref accessor visible: boolean = this.props.active // true while the drawer is active and during enter/exit transitions drawerRef = React.createRef() constructor(props: SlideInDrawerProps) { super(props) - makeObservable(this, { - visible: observable.ref, - }) + makeObservable(this) } override componentDidMount(): void { diff --git a/packages/charts/src/grapher/stackedCharts/MarimekkoChart.tsx b/packages/charts/src/grapher/stackedCharts/MarimekkoChart.tsx index f25997aa239..8a159d4d2a4 100644 --- a/packages/charts/src/grapher/stackedCharts/MarimekkoChart.tsx +++ b/packages/charts/src/grapher/stackedCharts/MarimekkoChart.tsx @@ -87,19 +87,16 @@ export class MarimekkoChart constructor(props: MarimekkoChartProps) { super(props) - makeObservable(this, { - focusColorBin: observable, - tooltipState: observable, - }) + makeObservable(this) } labelAngleInDegrees = -45 // 0 is horizontal, -90 is vertical from bottom to top, ... // currently hovered legend color - focusColorBin: ColorScaleBin | undefined = undefined + @observable accessor focusColorBin: ColorScaleBin | undefined = undefined // current tooltip target & position - tooltipState = new TooltipState<{ + @observable accessor tooltipState = new TooltipState<{ entityName: string }>() diff --git a/packages/charts/src/grapher/stackedCharts/StackedBarChart.tsx b/packages/charts/src/grapher/stackedCharts/StackedBarChart.tsx index a60cfc7c4a0..eea32aff97a 100644 --- a/packages/charts/src/grapher/stackedCharts/StackedBarChart.tsx +++ b/packages/charts/src/grapher/stackedCharts/StackedBarChart.tsx @@ -77,19 +77,15 @@ export class StackedBarChart constructor(props: StackedBarChartProps) { super(props) - makeObservable(this, { - hoverColor: observable, - hoveredTick: observable, - tooltipState: observable, - }) + makeObservable(this) } // currently hovered legend color - hoverColor: string | undefined = undefined + @observable accessor hoverColor: string | undefined = undefined // currently hovered axis label - hoveredTick: TickmarkPlacement | undefined = undefined + @observable accessor hoveredTick: TickmarkPlacement | undefined = undefined // current hovered individual bar - tooltipState = new TooltipState<{ + @observable accessor tooltipState = new TooltipState<{ bar: StackedPoint series: StackedSeries }>() diff --git a/packages/charts/src/grapher/stackedCharts/StackedBarSegment.tsx b/packages/charts/src/grapher/stackedCharts/StackedBarSegment.tsx index 1bb012c20a2..72d30e55748 100644 --- a/packages/charts/src/grapher/stackedCharts/StackedBarSegment.tsx +++ b/packages/charts/src/grapher/stackedCharts/StackedBarSegment.tsx @@ -29,12 +29,10 @@ export class StackedBarSegment extends React.Component { constructor(props: StackedBarSegmentProps) { super(props) - makeObservable(this, { - mouseOver: observable, - }) + makeObservable(this) } - mouseOver: boolean = false + @observable accessor mouseOver: boolean = false @computed get yPos(): number { const { bar, yAxis } = this.props diff --git a/packages/charts/src/grapher/stackedCharts/StackedDiscreteBarChart.tsx b/packages/charts/src/grapher/stackedCharts/StackedDiscreteBarChart.tsx index 122a1e6a2a3..5f162a3b7c4 100644 --- a/packages/charts/src/grapher/stackedCharts/StackedDiscreteBarChart.tsx +++ b/packages/charts/src/grapher/stackedCharts/StackedDiscreteBarChart.tsx @@ -52,13 +52,10 @@ export class StackedDiscreteBarChart constructor(props: StackedDiscreteBarChartProps) { super(props) - makeObservable(this, { - focusSeriesName: observable, - tooltipState: observable, - }) + makeObservable(this) } - focusSeriesName: SeriesName | undefined = undefined + @observable accessor focusSeriesName: SeriesName | undefined = undefined @computed get chartState(): StackedDiscreteBarChartState { return this.props.chartState @@ -182,7 +179,7 @@ export class StackedDiscreteBarChart return new HorizontalCategoricalColorLegend({ manager: this }) } - tooltipState = new TooltipState<{ + @observable accessor tooltipState = new TooltipState<{ entityName: string seriesName?: string }>() From cc325cfee427d5bee29b1bf1a51f58a4d334f593 Mon Sep 17 00:00:00 2001 From: xrendan Date: Thu, 5 Feb 2026 13:16:04 -0700 Subject: [PATCH 2/2] Fix CI: build packages before tests, generate stats for Chromatic - Add build:packages step before test step in CI so build.test.ts can import from dist/ (which is gitignored and not available in CI) - Add --stats-json to storybook build so Chromatic TurboSnap can find preview-stats.json Co-Authored-By: Claude Opus 4.5 --- .github/workflows/ci.yml | 3 +++ package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) 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/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",