Uh oh!
There was an error while loading. Please reload this page.
test(console): take the app-shell barrel out of three vi.mock factories (#6580) - #7315
Merged
Merged
Conversation
`@object-ui/app-shell` is aliased to `packages/app-shell/src` in `apps/console/vite.config.ts`, so a `vi.mock` factory that calls `importOriginal()` on it transforms the whole barrel graph on demand — 447 lines and thirteen bare side-effect imports. Re-measured on this tree with `performance.now()` around the call: 10019ms, against 237ms for `@object-ui/auth` in the same file. A per-file read of every console test that mocks the package attributes each `importOriginal` to the factory it sits in. The population is three files, not the twelve the path-intersection upper bound suggested: the other nineteen either use no `importOriginal` at all or use it for `../../services/approvalsApi`, `@object-ui/i18n` or `@object-ui/auth`. Each of the three factories overrides most of what its graph reads from the barrel and needs only a handful of real exports, all of which live in submodules that cost ~2.0s together. Timed individually, the barrel still costs 7828ms AFTER every one of those submodules is already loaded — that residue is the side-effect imports and the rest of the package, none of which these tests touch. So the factories now spread those submodules instead of the barrel. Which names each file's graph reads from the barrel was enumerated statically over the whole transitive module graph (~1237 modules per file), not from the paths the tests happen to exercise, and every one of them is an override, a type-only export, or an export of a spread submodule. Three files, same 37 tests, same flags, from the repo root: import phase 13.07s -> 2.64s, transform 17.09s -> 11.27s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
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
|
yinlianghui
marked this pull request as ready for review
September 2, 2026 06:15
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 2, 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
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#6580
1. Measurement first — the population is 3, not 12
The card was explicit that 12 was a path-intersection upper bound, because a file
lands in it if it mocks
@object-ui/app-shelland usesimportOriginalanywhere.Attributing each
importOriginalcall to the factory it actually sits in (paren-balancedscan over every
vi.mockcall, then confirmed by reading the three survivors):Ripgrep over
apps/consolefinds 22 files mocking the package — one more than thecard's GitHub code-search figure of 21, so the index was indeed lagging.
importOriginalbelongs tocomponents/FormPage.predicateScope.test.tsx@object-ui/app-shell,@object-ui/authcomponents/SetupRoute.test.tsx@object-ui/app-shell,@object-ui/auth,packages/auth/src/useAuth,packages/app-shell/src/providers/MetadataProvidercomponents/StudioRoute.test.tsx@object-ui/app-shell,@object-ui/auth,packages/auth/src/useAuthpages/system/ApprovalsInboxPage.cellIdentity.test.tsx../../services/approvalsApipages/system/ApprovalsInboxPage.characterizationPins.test.tsx../../services/approvalsApipages/system/ApprovalsInboxPage.hiddenFieldTrim.test.tsx../../services/approvalsApipages/system/ApprovalsInboxPage.queueHiddenAmount.test.tsx../../services/approvalsApipages/system/ApprovalsInboxPage.rawPayloadGate.test.tsx../../services/approvalsApipages/system/ApprovalsInboxPage.recordLink.test.tsx../../services/approvalsApipages/system/ApprovalsInboxPage.stepProgressVertical.test.tsx../../services/approvalsApipages/system/__tests__/AppManagementPage.i18n.test.tsx@object-ui/i18npages/system/__tests__/AppManagementPage.mutations.test.tsx@object-ui/i18npages/system/__tests__/AppManagementPage.search.test.tsx@object-ui/i18nimportOriginalat allN = 3. The card's own caveat was right: nine of the twelve candidates carry an
importOriginalthat belongs toapprovalsApior@object-ui/i18n, not to the barrel.2. Cost, re-measured on this tree
performance.now()around the call, in a throwaway probe file that was deleted beforethe commit (a
vi.hoistedholder carries the number out of the hoisted factory):That reproduces the card's 10204ms / 240ms independently. Two of the card's structural
numbers have grown since it was filed and are corrected here: the barrel is 447 lines
carrying 13 bare side-effect imports, not 385 and 8.
3. Which lever, and why the other two are not it
(a) alias the test build to built output — not viable.
.github/workflows/ci.ymlruns
pnpm install --frozen-lockfileand thenpnpm test --shard=N/4with no buildstep, and
packages/app-shell/distdoes not exist at test time. The alias table inapps/console/vite.config.tspoints every@object-ui/*entry at source, so this wouldalso be a single inconsistent entry.
(b) trim the barrel's side-effect imports — stopped, per the dispatch ruling. Those
13 imports are registration side effects of the published package; removing or deferring
them changes what a consumer gets from importing the barrel. Making only the test alias
avoid them requires a second entry point, which is lever (a) wearing a different hat.
Not landed here.
(c) drop
importOriginalfor this package in the factories — chosen. Confined to thethree test files, no published surface touched.
What decided it: each submodule these factories actually need was timed individually, and
then the barrel was timed after all of them were already loaded.
chrome/indexconsole/ConsoleShellproviders/ExpressionProviderproviders/expressionUserutils/indexproviders/MetadataProviderSo ~7.8 s of the ~10 s is graph these three files never touch. The factories now spread
the submodules the real exports live in.
4. Why this is not fragile — a static coverage proof
Dropping the whole-barrel spread risks a name silently becoming
undefinedon a path notest exercises. So the covering set was derived statically, over the full transitive
module graph of each test file (~1237 modules once NodeNext
.jsspecifiers resolve totheir TypeScript sources), not from the paths the tests happen to run. Every name any
module in the graph imports from
@object-ui/app-shellmust be an override, a type-onlybarrel export, or an export of a spread submodule:
SetupRoute.test.tsxStudioRoute.test.tsxFormPage.predicateScope.test.tsxThe same scan confirms no module inside any workspace package imports the barrel by its
package name, so the consumer set is exactly those console files.
One subtlety worth flagging for review:
SetupRoute.test.tsxmockspackages/app-shell/src/providers/MetadataProviderseparately, butRootLandingRedirectreads
useMetadatathrough the barrel. The old whole-barrel spread covered that byre-exporting the already-mocked provider. The replacement spells the same stub out in the
app-shell factory rather than importing the provider module, so console's tsc program
gains no new cross-package source edge; the comment there says the two must stay in step.
5. Before / after
Same three files, same flags, same command, from the repo root:
These are shared-box seconds — the runs went through the shared verification lock, which
excludes other locked runs but not unlocked sibling work, so read the ratio (import phase
down ~5x) rather than the absolutes.
6. Verification
All from the repo root of the task worktree, exit codes captured before any pipe.
Test Files 3 passed (3)/Tests 37 passed (37), lockVERDICT command-exit 0pnpm --filter @object-ui/console run type-check-> exit 0, 0error TS. Run afterpnpm --workspace-concurrency=4 --filter "@object-ui/console^..." run buildso thedependency closure was real;
tsc --noEmit --listFilesconfirms all three edited filesare in the program (1 hit each), so "type-check clean" actually covers them.
pnpm exec eslinton the three changed files -> exit 0. One pre-existingno-explicit-anywarning at line 188 ofFormPage.predicateScope.test.tsx, outsideevery hunk in this diff.
pnpm check:control-bytes-> exit 0,scanned 6009 tracked text file(s).node scripts/check-changeset-presence.mjs-> exit 0: "Every one of them has anEMPTY frontmatter — declared as releasing nothing, which is the explicit exemption".
pnpm changeset:check-> exit 0.pnpm lint:coverage-> exit 0 (46/46).pnpm type-check:coverage-> exit 0.Not run locally, left to CI: the full
apps/consoleproject and the repo-wide lint farm.This is a declared narrowing — the diff is three test files plus an empty-frontmatter
changeset, and CI runs the farm exactly once regardless.
7. Not touched
apps/console/vitest.config.ts, rootvitest.config.mtsand rootvitest.setup.*areowned by in-flight PRs #7291 and #7304. No timeout was raised, nothing was added to
heavyDomTests, and nothing was skipped or quarantined.🤖 Generated with Claude Code
https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
Generated by Claude Code