[WC-3488]: Charts rework & cleanup

[yordan-st]

Pull request type

Bug fix (non-breaking change which fixes an issue)

---

Description

WC-3488 — cleanup of the unresolved code review (Leo) on the already-merged custom-chart / playground editor rework. All findings shipped unfixed and were still live on main; this PR resolves them.

What & why, per finding:

• [Critical] computed().get() misuse — useCustomChart.ts wrapped a plain object in computed(() => ({...})).get(), allocating a throwaway MobX atom every render with no caching or reactivity benefit. Replaced with a plain object literal; reactivity is handled by the existing observer wrapper on CustomChart.tsx.
• [Critical] containerStyle dead code — the hook returned a containerStyle never consumed (CustomChart.tsx computes its own via constructWrapperStyle). Removed the computation, the return field, and the unused import.
• [Important] hardcoded layoutOptions: {} / configOptions: {} — this silently blanked the playground's Modeler Layout and Modeler Configuration panels. Now passes the adapter's real layout / config. (User-visible → changelog entry added.)
• [Important] store.data as Data[] unsound cast — replaced the bare cast with a single named boundary helper toPlotlyData(). No runtime validation added: the store already guards on write (setDataAt parses, type-checks, warns), and Plotly's Data union is impractical to validate exhaustively — the helper just localizes and documents the unavoidable conversion.
• [Important] @types/jest in dependencies — moved to devDependencies and dropped "types": ["jest"] from tsconfig.build.json so jest types no longer ship as a runtime dep / pollute the production build.
• [Important] deleted 402-line test, no replacement — added a unit spec for EditableChartStore (8 cases: reset, setDataAt valid/out-of-range/invalid-JSON/non-object, setLayout/setConfig null-guard, JSON getters).
• [Minor] undocumented eslint-disable react-hooks/set-state-in-effect — documented why it's safe and the accepted risk.
• [Minor] duplicate prettifyJson — extracted to one shared helper, imported in both editor controllers.
• [Minor] onViewSelectChange not memoized — wrapped in useCallback in both controllers.
• [Minor] key duplicated (React state + MobX box) — collapsed to a single source of truth in useV2EditorController (kept the React state that drives render; the reaction now re-subscribes on the key via its effect deps with fireImmediately).

Deliberately out of scope (deferred to WC-3348):

• The silent empty-JSON catch {} blocks in the editor controllers (Leo's last Minor). WC-3488 marks this as WC-3348's scope; left untouched here.
• CodeEditor.tsx and the editor restore — that was WC-3348 ([WC-3348] Charts: restore highlighted JSON editor in playground #2310).

Notes for reviewers: the OpenSpec change (custom-chart-web/openspec/changes/fix-rework-cleanup/) has the full proposal / test plan / task breakdown. Two test-infra additions (zero product impact): a ^src/ moduleNameMapper in custom-chart-web/jest.config.js (mirrors the tsconfig baseUrl the source already uses) and a stubObjectURL shim (the shared-charts barrel loads Plotly, which touches URL.createObjectURL on import in the test env).

---

What should be covered while testing?

Automated (green locally):

• custom-chart-web — 33 tests · chart-playground-web — 6 tests · shared/charts — 45 tests
• Lint clean on all changed files; shared/charts production build passes after the B5 tsconfig change.

Manual QA — Modeler panels (the user-visible fix):

1. Add a Custom chart widget to a page; give it static/sample data (e.g. [{"type":"bar","x":[1,2],"y":[3,4]}]) and enable the playground slot. Optionally set a Layout value like {"title":"Test"}.
2. Run the app, hard-reload the browser (widget JS is cached — Empty Cache and Hard Reload).
3. Open the chart playground → in the sidebar view selector pick Layout, then Configuration.
4. Expected: the Modeler Layout / Modeler Configuration panels show real JSON (width/height/autosize/font/margin, plus any layout you set) — not {}.

Regression checks:

• Custom chart still renders and click events still fire.
• In the playground, switch between Layout / a trace / Configuration — the editor content follows the selection with no stale/desynced text.
• Edit a value in the editor → chart updates; feed invalid JSON → chart doesn't crash (invalid input is ignored — this catch remains silent by WC-3348 scope decision).

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
chart-playground-web/src/helpers/prettifyJson.ts	Extracted shared helper (was duplicated in both controllers)
chart-playground-web/src/helpers/useComposedEditorController.ts	onViewSelectChange memoized; effect comment added
chart-playground-web/src/helpers/useV2EditorController.ts	Removed keyBox dual state; reaction now uses fireImmediately: true
chart-playground-web/src/helpers/__tests__/prettifyJson.spec.ts	New unit test
chart-playground-web/src/helpers/__tests__/stubObjectURL.ts	New jsdom shim
chart-playground-web/src/helpers/__tests__/useComposedEditorController.spec.ts	New unit test
chart-playground-web/src/helpers/__tests__/useV2EditorController.spec.ts	New unit tests (2 cases)
custom-chart-web/CHANGELOG.md	Unreleased entry for modeler-panels fix
custom-chart-web/jest.config.js	Added moduleNameMapper for ^src/ baseUrl
custom-chart-web/src/controllers/CustomChartControllerHost.ts	Uses toPlotlyData() boundary helper
custom-chart-web/src/hooks/useCustomChart.ts	Removed computed().get() wrapper + dead containerStyle; passes real adapter.layout/config
custom-chart-web/src/hooks/__tests__/useCustomChart.spec.ts	New unit tests (3 cases)
custom-chart-web/src/utils/toPlotlyData.ts	New documented boundary cast helper
custom-chart-web/src/utils/__tests__/toPlotlyData.spec.ts	New unit tests
shared/charts/package.json	@types/jest moved from dependencies → devDependencies
shared/charts/tsconfig.build.json	"types": ["jest"] removed from production build config
shared/charts/src/model/stores/__tests__/EditableChart.store.spec.ts	New unit tests (8 cases)

Skipped (out of scope): pnpm-lock.yaml, openspec/ docs

---

Findings

🔶 Medium — chart-playground-web missing CHANGELOG entry

File: packages/pluggableWidgets/chart-playground-web/CHANGELOG.md
Problem: The V2 editor controller had a user-visible bug fix: replacing the keyBox dual-state pattern with reaction({ fireImmediately: true }) means the editor panel now correctly shows content immediately when switching views. Previously, on view selection change, the initial render was driven by stale React state until the reaction fired on the next external change. This is a behavioral fix that belongs in the changelog.
Fix:

## [Unreleased]

### Fixed

- We fixed an issue where the chart playground editor panel could show stale content after switching views.

---

⚠️ Low — console.warn spy not in a cleanup guard

File: packages/shared/charts/src/model/stores/__tests__/EditableChart.store.spec.ts line 66–72
Note: warn.mockRestore() is called inline after the assertions. If either expect fails, mockRestore() is skipped and the spy leaks into subsequent tests. Wrap in try/finally or move cleanup to afterEach:

const warn = jest.spyOn(console, "warn").mockImplementation(() => undefined);
try {
    store.setDataAt(0, "{ not json ");
    expect(store.data).toBe(before);
    expect(warn).toHaveBeenCalledTimes(1);
} finally {
    warn.mockRestore();
    dispose();
}

---

⚠️ Low — useComposedEditorController spec covers only memoization

File: packages/pluggableWidgets/chart-playground-web/src/helpers/__tests__/useComposedEditorController.spec.ts
Note: The V2 spec has a "tracks the selected view without desyncing value and editor code" case; the V1 spec does not. A view-switch test for V1 would confirm modelerCode returns real layoutOptions/configOptions values (the user-visible fix channeled through useCustomChart) and that value follows the selected key. Not blocking, but the asymmetry leaves a gap.

---

Positives

• Removing computed(() => ({...})).get() from useCustomChart is the correct fix — a throwaway computed atom allocates a MobX node with no caching benefit when the component is already an observer.
• @types/jest moving from dependencies → devDependencies plus dropping "types": ["jest"] from tsconfig.build.json is a well-scoped two-part fix; the jest globals no longer pollute @mendix/shared-charts consumers.
• reaction(..., { fireImmediately: true }) + dep on [store, key] cleanly replaces the keyBox dual-state pattern. Each key switch now creates a new reaction that immediately syncs the editor — no stale renders.
• toPlotlyData() localizes the unavoidable as Data[] cast with a doc comment explaining exactly why it can't be narrowed — future readers won't be tempted to "fix" it.
• The 8-case EditableChartStore spec covers every guard path in setDataAt (valid, out-of-range, invalid JSON, non-object) and the null guards in setLayout/setConfig — solid regression net.
• moduleNameMapper in custom-chart-web/jest.config.js mirrors the tsconfig baseUrl the source already uses without requiring source changes.