Uh oh!
There was an error while loading. Please reload this page.
feat(app-shell): the precise sideEffects array and the gate that keeps it honest - #6735
Merged
Merged
Conversation
… that keeps it honest `@object-ui/app-shell` declared no `sideEffects` field, so every bundler had to assume every module in it does something on import and nothing in the package was shakeable. `"sideEffects": false` is not the answer and is closed by measurement: it drops three live SDUI widget registrations to zero chunks on a green build. The package now declares the precise ARRAY -- its entry forms plus the ten modules that register at load time, in both source and published spellings. An incomplete array fails silently inside a CONSUMER's bundle, so the array never ships alone (maintainer ruling, 2026-08-29): - `scripts/check-side-effects-array.mjs` re-derives the enumeration from the module bodies and fails on a missing registrar, a stale name, a registrar no chain of covered modules reaches, or a top-level effect it does not recognise. - `scripts/check-sdui-registration-pins.mjs` weighs the built console for every registration the array promises, with the key set derived from the array itself. `MAX_EAGER_CLOSURE_GZIP_BYTES` is re-baselined downward, 3,345,000 -> 3,300,000 over a measured 3,254,004, in the same commit -- the array took 56,668 gzipped bytes out of the closure, more than the old ceiling's whole headroom. Two entries leave `DECLARED_LAZY_VIEWS_STILL_EAGER`: `RecordFormPage` and `ReportView` are no longer eager, because the co-tenant modules that anchored their chunks became shakeable. The build named both, which is what a recorded win looks like there. Fixes#6683 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
…rement was taken on The constant is not a build input — the console build's turbo `inputs` cover `scripts/vite-*.ts`, not `scripts/check-*.mjs` — so this commit's tree builds identically to the one it names, and the figure was re-measured on it. Part of #6683 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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
|
os-sales
marked this pull request as ready for review
August 29, 2026 04:52
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 29, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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#6683
Implements the 2026-08-29 ruling in its only accepted shape: the precise
sideEffectsARRAY plus the companion gate, as one unit, with theMAX_EAGER_CLOSURE_GZIP_BYTESdownward re-baseline in the same commit.The ruling said "Expected effect re-verified at landing: ~242.6 KB gzip off the console eager closure, headroom ~0.9% → ~8%." Measured on this branch, that does not hold, and the reason is measured rather than argued.
9486ac672, this branch's base)f905b0753)"sideEffects": false(option C, disproven)"sideEffects": falseon today's tree: −242,912 bytes (−237.2 KiB) — reproducing the card's 242.6 KB within measurement noise, and dropping four live registrations (the three named pluscloud-connection:panel).The 242.6 KB figure was
false's number, not the array's. The card says so; the ruling then carried it forward as the array's expected effect. It cannot be the array's: the difference between the two rows is exactly the closure the ten registering modules anchor —metadata-adminalone is 168.6 KiB and stays eager becauseviews/metadata-admin/index.tsregisters previews and inspectors at module top level. Keeping those registrations is the correctness the ruling required; keeping them keeps their import closure eager. ⛔ Nothing here was reshaped to chase the projected number — that would be option C by another name.Headroom, stated both ways so the tightening is not confused with the win:
So headroom goes 0.9%-class → 1.4%-class, not → 8%. It is not "0.9% and any static import trips it" any more, but this card does not end the near-zero-headroom condition. #6681 (~194 KiB of AppContent's other
lazy()declarations,metadata-adminthe bulk of it) is now the lever that would.What shipped
1. The array — 23 entries, derived, not copied
packages/app-shell/package.jsonnames its entry forms (./dist/index.js,./src/index.ts, and./src/styles.css— a stylesheet a bundler must never drop) plus the ten modules that register at load time, each in both its source and its published spelling. Consumers resolveexportstodist/; the in-repo bundler aliases resolve tosrc/, and a bundler reads the same manifest for both.The enumeration was re-derived mechanically by the gate below, never pasted. It returned 10, which matches the card's "measured at 10 today" — stated as an agreement between two independent derivations, not as an input:
The broad scan finds 15 modules with any top-level effect. The five that are not in the array are named here rather than glossed:
src/index.ts(the barrel — in the array as an entry form anyway); threeX.displayName = 'X'writes on the module's own binding (ConsoleNotificationBanners,ImpersonationBanner,LocalizedSidebarTrigger), provably module-local; andsrc/views/RecordDetailView.tsx, whose only effect is a bare import ofrecord-approvals-renderer— a propagation edge, and the barrel bare-imports that module directly anyway. MarkingRecordDetailViewwould have held 47.4 KiB eager for every barrel consumer and bought nothing.2.
scripts/check-side-effects-array.mjs— the ruled companion gateWorkspace-general (it also validates
@object-ui/layout's existing array). One rule: the array names exactly the entry forms plus every module in the entry graph that performs a top-level registration, in both spellings. It fails onbarrel → pure-helper → registrarchain still loses the registration, because the shakeable helper takes the registrar's only edge with it;classifyEffecttreats any top-level call ornewas a registration — deliberately not aregister-name test, because a name test under-reads and an under-read here is exactly the silent drop.Registry.add(...)is caught.Exit codes follow this repo's convention: 0 agree, 1 the array is wrong, 2 no trustworthy enumeration.
3.
scripts/check-sdui-registration-pins.mjs— the artifact halfThe static gate cannot answer the question the hazard turns on: does a real bundler reading that array still emit the registrations.
"sideEffects": falseis statically coherent and drops three of them. This gate weighsapps/console/dist/assets/*.jsfor every registration key the array's modules declare, read throughscripts/component-registrations.mjs— the key set is derived from the array, so there is no second list.RULED_CONTROLSis a floor on that derivation (the three keys the ruling names), not the population; a test pins that the two never coincide.Every zero is loud: no
dist/is exit 2, an empty key set is exit 2, and a negative-control key that must be found in zero chunks is searched on every run, because a matcher that cannot miss reports every key present forever.4.
MAX_EAGER_CLOSURE_GZIP_BYTES: 3,345,000 → 3,300,000Over a measured 3,254,004 on
bd2a7ec50. The array took 56,668 bytes out of the closure — more than the old ceiling's entire 45,102-byte headroom — so leaving the ceiling put would park the aggregate gauge at exactly 1.00x its own sensitivity, one byte of shrink from the exit-2 verdict. Both numbers move in one commit, as that file requires.BASELINEmoves with it (48 chunks of 513). The per-chunk ceilings did not move: those three chunks are byte-identical before and after, and their headrooms are 0.21x / 0.04x / 0.05x.5. Two entries leave
DECLARED_LAZY_VIEWS_STILL_EAGER— a recorded win, not a trimThe first build on the array failed, correctly, in the ledger's
missingdirection:Those two were pinned for chunk co-tenancy (#6680) — held eager by a co-tenant in their own chunk, with no import edge to the view. The package-level array makes those co-tenants (
providers/expressionUser.ts,views/RuntimeDraftBar.tsx) shakeable in their own right, the chunks they anchored stopped being eager, and both views fell out. The lines are deleted because the build named them; counter-probe 1 (every declared view must be found in some chunk) ran green on the same build, which is what separates "became lazy" from "the matcher stopped matching". After:7/8 views AppContent declares lazy are genuinely lazy; 1 eager, all pinned. This does not close#6680 — it removes this instance of it; the chunking-policy question that card raises is untouched.The chunks that moved
The new eager
srcchunk is the ten registering modules being pulled into one place. It is the array's cost, and it is the smaller half of the trade.Controls, shown RED — a pin that cannot fail is not a pin
Both ablations ran from a committed tree (
bd2a7ec50), each under atrap '<restore>' EXIT INT TERMwith absolute paths, each confirming the mutation reached disk by anchoredgrep -ccounts and a blob hash that differs from HEAD's, and each proving the restore by observation —git diff HEADempty andgit hash-objectequal to the HEAD blob — never by an editor's exit code. Exit codes were captured before any pipe.Ablation A —
"sideEffects": false(option C reproduced on today's tree).Chunk counts read off that build's
dist/:"sideEffects": falsemcp:connect-agentcloud:onboarding-nextcloud:ai-model-statuscloud-connection:panelmarketplace:installed-listThe registration pin gate exits 2, naming all three ruled controls: "Either a
sideEffectsarray stopped naming the module that registers the key — which is the silent drop, and must be fixed rather than re-pinned — or the reader has stopped seeing it." The budget gate exits 2 as well: the closure fell to 2995.9 KiB and the aggregate ceiling drifted to 2.55x the regression — the objectui#5924 sensitivity verdict working."sideEffects": falsetakes the package out of this gate's population by construction — that direction belongs toside-effects-declaration-consistency.test.ts's "nosideEffects: falsepackage has a load-time side effect", which is red on that manifest. Two gates, two populations, and the boundary is stated so nobody reads the 0 as approval.Ablation B — the array with ONE control module removed (the shape the dispatch asked for: both spellings of
ConnectAgentWidgetdeleted, 23 entries → 21).and the build that came out of it, read chunk by chunk — surgical, exactly the excluded module and nothing else:
mcp:connect-agentcloud:onboarding-nextcloud:ai-model-statuscloud-connection:panelmarketplace:installed-listRULED_CONTROLSfloor fires before the chunk count is read. That ordering is deliberate and is the louder verdict — the ruling's own instruction is that this must be fixed rather than re-pinned. The exit-1 path ("named, and still absent from the bundle") is the array-honoured-but-bundler-dropped case, covered by fixture incheck-sdui-registration-pins.test.ts.Gate discrimination, established rather than assumed.
check-side-effects-array.test.ts(23 cases) fails the gate on: a missing registrar, a missing published spelling, a missing entry form, a stale name, a name that exists nowhere, an unrecognised top-level effect, a glob, an unresolved specifier, a spelling map that does not round-trip, a registrar reachable only through a shakeable module, and an empty population — each with the partner case that passes. Against the real manifest it asserts both directions in one test: removingConnectAgentWidgetmakesevaluatePackagered while the shipped manifest stays green.Gate verdicts — every exit code captured before any pipe, on
f905b0753after the final commitcheck:eager-closureConsole eager closure is 3177.7 KB gzipped across 48 of 513 chunks (budget: 3222.7 KB, headroom: 44.9 KB).check:sdui-registration-pinsAll 12 registration(s) a sideEffects array promises are present in the built console (513 chunks weighed; the 3 ruled control(s) are in the derived set).check:side-effects-array@object-ui/app-shell: sideEffects names exactly the 10 module(s) that register at load time, plus its entry forms (23 entries, 432 modules walked).vite build[declared-lazy-views] 7/8 views AppContent declares lazy are genuinely lazy; 1 eager, all pinned (objectui#6535).check:docs-route-closureall 13 packages named in registerCatalogBlocks.ts are accounted forcheck:changeset-presenceNo source of a released package changed in this range, so no changeset is owed.(a changeset ships anyway — see below)check:changeset-no-major/-fixedNo changeset declares a major bump./All workspace packages are in the changeset fixed group.check:control-bytescheck-control-bytes: OK (scanned 5588 tracked text file(s); skipped 85 binary).check:entry-guard53 scripts/ file(s) — no entry guard outside the baseline; 48 export bindings, 48 of them inert on importcheck:esm-specifiersSpecifier leg: no un-ledgered package emits an extensionless relative specifier.check:self-importNo package names itself inside its own src/.check:phantom-depsEvery in-scope import is declared by the package that publishes it.check:doc-fences/check:doc-linksevery TypeScript block in 223 document(s) is fenced .../Links are valid across 17 scan roots.pnpm type-check:scriptstsc -p tsconfig.scripts.json, no diagnosticsscripts/__tests__/Test Files 87 passed (87) / Tests 2425 passed (2425)packages/layout/Test Files 21 passed (21) / Tests 261 passed (261)The console build and every closure figure quoted above were produced by the same build on
f905b0753; the report it wrote readseagerGzipBytes 3254004, byte-identical to the figureBASELINErecords forbd2a7ec50— which is the check that the recorded commit and this head build the same tree (check-*.mjsis not in the console build's turboinputs;scripts/vite-*.tsis).Why a changeset ships even though the gate says none is owed
check-changeset-presence.mjsguards<pkg>/src/**. This diff changespackages/app-shell/package.jsonand no source, so the gate correctly reports nothing owed — and asideEffectschange is a published contract change that every consumer's bundler reads. The changeset spells out what it means for a consumer, atminor(objectui'smajorfollows@objectstack's). That the gate cannot see this class of change at all is filed separately, below.Declared narrowings — what was NOT run here, and why that is a measurement
ESLint. Repo-wide
turbo run lintwas not run; ESLint ran on the 8 changed JS/TS files. Three things make that a measurement rather than a gap: (1) the population came from ESLint's own resolution — all 8 returned real result objects, none reported as ignored; (2) the counts are read from--format json: 8 files linted,errorCount0 andwarningCount0 on every one; (3)eslint.config.jsconfigures noprojectService, noprojectand no type-checked ruleset, so no rule's verdict on a file this PR does not touch can depend on this diff. CI runs the full farm regardless.check:readme-exports— NOT MEASURED, not green and not red. It exits 1 in this worktree with 379 findings of the form "its type entry./dist/index.d.tsis not on disk — runpnpm buildfirst", across every package including ones this PR does not touch. That is the unbuilt-tree prerequisite, not a verdict on this diff (which changes no README and noexports/typesfield). CI builds packages before running it.pnpm testfarm,turbo run lint,turbo run type-check, andcheck:readme-exportson a built tree.Findings filed, not fixed here
check-changeset-presence.mjsguards only<pkg>/src/**, so a published-contract change inpackage.jsonis owed no changeset — live instance in #6683 #6736 —check-changeset-presence.mjsguards only<pkg>/src/**, so a change to a package's published contract inpackage.json(sideEffects,exports,files,main,types,peerDependencies) is owed no changeset and can ship unrecorded. This PR is the live instance: the gate says nothing is owed for a field every consumer's bundler reads.Authoring session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Fences respected
packages/app-shell/src/**is untouched — the barrel's exports and its bare side-effect imports are exactly as they were; this PR changes the package's manifest, not its code.apps/console/vite.config.tsis untouched.scripts/vite-declared-lazy-views.tsis edited only in its ledger and its prose, both demanded by its own build-time verdict.Generated by Claude Code