fix(app-shell): pin nodenext and read Monaco's named Editor export - #6338
Merged
Conversation
`packages/app-shell/tsconfig.json` now carries the `module` / `moduleResolution: nodenext` pin that `@object-ui/react`, `@object-ui/fields` and five other packages already carry. This package builds with a bare `tsc` and `tsc` never rewrites specifiers, so what the source writes is what `dist` ships; under `nodenext` a missing relative extension is TS2835 and a bare directory import is TS2834, which makes the property the compiler's business instead of review's. The pin was used as a measuring instrument here long before it could be used as enforcement. Re-measured on this branch at f8c70f4: unpinned 0 errors, pinned 2 — down from 1097 before the `@object-ui/components` typings were fixed and 23 before `@object-ui/plugin-chatbot`'s were. Both survivors are the lazy `@monaco-editor/react` import. That import now reads the named `Editor` export. `@monaco-editor/react@4.7.0` is CommonJS with no `exports` map, so under `nodenext` its default is the module namespace rather than the component and `React.lazy` rejects it (TS2345). Verified rather than assumed from the name: the package's own typings alias one declaration to both spellings (`export { _default as Editor, ..., _default as default }`), and in the installed 4.7.0 `default === Editor` holds in its CommonJS build and its ESM build alike, so the component that renders is unchanged. The two test stubs for the module bound only `default`, so they are rebound to both names as the real module does. A new test then covers the one case nothing else did, which is not the case that was assumed: the compiler already rejects both wrong components at this call site (TS2345 for the namespace, TS2353 for `DiffEditor`), but an editor that resolves to NOTHING produces exactly the textarea fallback the neighbouring suites assert — measured, that suite stays green with `Editor` deleted from its stub. The new test renders the editor and asserts it painted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CZgmFFzqA9cX8tBMhvpFe
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 25, 2026
os-warren
marked this pull request as ready for review
August 25, 2026 14:22
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.
Fixes #5440
Lands the
module/moduleResolution: nodenextpin inpackages/app-shell/tsconfig.json— the last of the consumer pins, and the largest — and resolves the two@monaco-editor/reactinterop errors that were the only thing standing between the pin and a green enforcement.(Generics below are written with a space after the opening angle bracket, following the card's own convention: GitHub's body sanitizer strips an angle bracket followed by a letter as an HTML tag.)
The table, re-derived rather than inherited
Every number in the card was measured on 2026-08-20 at
32ef595f4and was explicitly not re-measured at dispatch. Re-derived here atf8c70f4f3, closure built first (pnpm --filter '@object-ui/app-shell^...' build, exit 0), viapnpm --filter @object-ui/app-shell exec tsc --noEmit:packages/app-shell/tsconfig.jsonnodenextpin, before this changenodenextpin, after this changeThe control row still holds —
app-shelltype-checks clean unpinned, so every error is produced by the pin, which is what made it usable as a measuring instrument before it was usable as enforcement.The 21 TS7006 are gone. Triage asked for this specifically: they were attributed to #5439, and #5439 landed. None survived, so there is no new residue to report and nothing was widened. The card's 23 is now 2, and the line/column anchors re-anchored unchanged at
JsonSourceEditor.tsx(30,31)andpreviews/SourcePageEditor.tsx(30,31).The monaco call: named export, and why
m.Editoris verifiably the componentTaken as triage presumed — the named-export shape. The card's worry was that a lazy import can type-check and still render the wrong thing, which a green type-check would not catch, so the name was checked in the installed
@monaco-editor/react@4.7.0rather than assumed:dist/index.d.tsisexport { ..., _default as Editor, ..., _default as default, useMonaco }, and_defaultisreact.MemoExoticComponent< typeof Editor >.Editoranddefaultare not two symbols that happen to agree — they are the same declaration.dist/index.js, whatnodenextresolves throughmain, since the package ships noexportsmap):m.default === m.Editoristrue, with$$typeof=Symbol(react.memo). In the ESM build (dist/index.mjs, what the bundler path uses today): the emitted export line isexport{we as DiffEditor,de as Editor,Ft as default,...}withFt=de, and importing it givesm.default === m.Editortrue.So this is the same object the default was resolving to, under both resolutions — the swap cannot change which component renders. That is established by identity, not by a browser run: I did not boot the app or drive Monaco in a browser, and the assertion here is the narrower one that identity permits.
What the ablation contradicted
I predicted that pointing the factory at
DiffEditorwould type-check green — the "typed but wrong" case that motivated adding a render-level test. That prediction was wrong, and the measurement is reported rather than the template:tsc --noEmitmod.default(the namespace)mod.DiffEditor'tabSize' does not exist in type 'IDiffEditorConstructionOptions'The compiler already rejects both wrong components at this call site, because the two components' prop types differ and this call site passes editor-shaped options. So the new test is not what catches a wrong component.
Measuring what it does catch turned up a real hole: an editor that resolves to nothing is invisible to the existing suites, because they assert the textarea fallback and a broken lazy import produces exactly that fallback. Measured — remove
EditorfromJsonSourceEditor.fallback.test.tsx's stub, leaving the factory reading an export that is not there, and that suite still passes (exit 0): the DOM-poll backstop flips to the textarea before the broken import is ever rendered. Green there means "the fallback works", never "the editor works".JsonSourceEditor.monaco-export.test.tsxis the one test that renders the editor and asserts it painted. Its docstring states this measured reasoning, not my original assumption.Each ablation leg mutated source only (vitest and
tscboth read these sources directly — nodistis involved forapp-shell's own tree), proved the mutation landed on disk by grep count of the injected and removed text, and restored under a trap with absolute paths, proving restoration by blob hash against theHEADblob plus an emptygit diff HEAD— not by an exit code.File surface
Two files beyond the three the claim declared, plus one new test, all inside
packages/app-shell/**:packages/app-shell/tsconfig.json— the pin.JsonSourceEditor.tsx,previews/SourcePageEditor.tsx— the named export.JsonSourceEditor.fallback.test.tsx,useMonacoFallback.fastfail.test.tsx— added surface. Both stubs bound the component todefaultonly, so the change would have left them handingReact.lazyan undefined export. They are rebound to both names, as the real module does.JsonSourceEditor.monaco-export.test.tsx— new, per the hole measured above..changeset/app-shell-nodenext-pin-5440.md—patch, percheck-changeset-presence.mjs's own verdict.No pin asserts the set of packages carrying the nodenext lines, so this change has no second half. Checked rather than assumed: every mention of
nodenext/node16in tracked files outsidenode_modulesis either a package's own tsconfig (8 of them, now 9), avite.config.ts, a changelog, or prose. The gate that reads these configs —check-node-esm-load.mjs— reads onlynoEmit, which this change does not touch, soapp-shellstays in the specifier leg exactly as before.Gates
All run at
ae4546995, exit codes captured by redirect before any pipe, each quoted from the gate's own verdict line.pnpm --filter @object-ui/app-shell type-check(acceptance)tsc --noEmit && tsc -p tsconfig.test.jsonpnpm exec vitest run—views/metadata-admin/+views/studio-design/Test Files 239 passed (239),Tests 2225 passed / 1 skippedcheck:vi-mock-specifiersOK (3744 tracked source file(s) ... 437 carry a mock ...)check:esm-specifiersSpecifier leg: no un-ledgered package emits an extensionless relative specifier.check-node-esm-load.test.ts(asserts every tsconfig in the repo parses)Test Files 1 passed,Tests 44 passedcheck:control-bytesOK (scanned 5217 tracked text file(s); skipped 85 binary)type-check:coverage45/46 via type-check ... 41/41 packages compile their testscheck:lint-coveragelint coverage: 46/46 packages linted, 0 with outstanding errorscheck:published-distNo published package's build output carries tooling material.check:eager-closureConsole eager closure is 3223.1 KB gzipped across 52 of 508 chunks (budget: 3266.6 KB, headroom: 43.5 KB)check:changeset-presence/-no-major/-fixeddeclares 1 changeset(s)·No changeset declares a major bump.·All workspace packages are in the changeset fixed group.pnpm --filter @object-ui/console buildcheck:eager-closurefirst came back exit 2 withNo eager-closure report at apps/console/dist/eager-closure.json ... This is a broken gauge, not a passing budget— a missing prerequisite, not a finding. It is reported above only after building the console, which is the run that makes it a measurement.Declared narrowing: repo-wide
pnpm lint(turbo run lint, 46 packages) was narrowed topnpm --filter @object-ui/app-shell lint— exit 0,0 errors, 2678 warnings(warnings pre-existing). The narrowing is a measurement rather than a gap, on three counts: the population comes from eslint's own config resolution rather than my guess;--format jsonreports 962 files inspected, all 5 of my touched-or-new files among them; andeslint.config.jsenables no type-aware linting (noprojectService, noparserOptions.project), so no rule reads types across a package boundary and a diff confined topackages/app-shell/**cannot move any untouched package's verdict. CI runs the full farm regardless.CI had not converged when this was opened — reporting at draft-PR time is the dispatch contract, and red gates come back as a patch round on this same claim.
Generated by Claude Code