perf(console): make the /docs portal genuinely lazy - #5485
Merged
Conversation
App.tsx statically imported DocPage / DocsSlug / DocsLayout while AppContent.tsx lazy-imported the same three for the app-scoped /apps/:packageId/docs tree, so all of them sat in the eager graph and the import() moved nothing -- three INEFFECTIVE_DYNAMIC_IMPORT warnings on every vite build. App.tsx now reaches all four docs pages through lazy() behind Suspense, matching AppContent's existing pattern. DocsIndex joins them because, left static, it alone would keep DocShell / use-book-data / book-nav eager. Measured (both builds exit 0): warnings 46 -> 44, eager closure 3,881,609 -> 3,870,058 gzipped bytes across 58 -> 52 chunks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
…(objectui#5467) App.docsPortalLazy.test.tsx measures WHEN each docs page enters the module graph, not how App.tsx spells its imports: a vi.mock factory runs on first import, so a static import sets the flag while App.tsx is evaluated, before any test body runs. Two counter-probes keep "never loaded" from passing vacuously -- the still statically imported SharedRecordPage is a live positive control, and visiting /docs must flip the layout's flag and render it through the Suspense boundary. internalFormShell.test.tsx's @object-ui/app-shell mock gains LoadingScreen: App.tsx's route elements are built when App renders, so the Suspense fallback's export is read even by a test that never visits /docs. Without it both of its tests died on the vitest mock proxy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
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 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 #5467
AppContent.tsxlazy-importsDocsLayout/DocsSlug/DocPagefor the app-scoped/apps/:packageId/docstree (ADR-0048).App.tsximported the same three statically for the platform portal at/docs(ADR-0046 section 6), so all three sat in the eager graph regardless and theimport()moved nothing — threeINEFFECTIVE_DYNAMIC_IMPORTwarnings on everyvite build.Which side was fixed, and why
Both route trees are live, so neither set of imports is dead code:
App.tsx— the platform docs portal at/docs(ADR-0046 section 6):DocsLayoutwithDocsIndex,DocsSlug,DocPage.AppContent.tsx— the app-scoped package docs at/apps/:packageId/docs(ADR-0048): the sameDocsLayout/DocsSlug/DocPage, withAppDocsIndexat the index slot.Deleting the lazy imports in
AppContent.tsxwould make the intent honest and change nothing: the pages are already eager, so that route is the one paying, not the one deciding. The laziness was written on purpose andAppContentapplies it uniformly to all thirteen of its pages — nothing on a normal console page load visits docs, andDocPageis the only console-owned module that reaches@object-ui/plugin-markdown. SoApp.tsxis the side that was wrong, and it now reaches all four docs pages throughlazy()behindSuspense, in exactly the shapeAppContent.tsxalready uses.DocsIndexjoins them even though it carried no warning —AppContentrendersAppDocsIndexat that slot, so nothing importedDocsIndexdynamically. Left static it alone would keepDocShell,use-book-dataandbook-naveager, and the portal would only half-leave the closure.Measured
Gauge:
apps/console/dist/eager-closure.json, added by #5324 / PR #5466. Both legs exited 0 — the warning count was never read from a build that died. Final numbers taken from the build onf9bbd4144, the branch HEAD.INEFFECTIVE_DYNAMIC_IMPORTwarningsWarning composition, which is the counter-probe on the zero: the three console-page warnings go 3 to 0, while the 43
packages/fieldswarnings of #5325 stay at 43 in both legs. A grep that had stopped matching, or a build that had died, would have taken those 43 with it.Six chunks leave the eager closure:
srcplugin-markdownCreateViewDialoguse-book-dataDocShellcomponentRegistrysrcis not a saving: rolldown folds it into the entry chunk instead of dropping it, which is why the entry chunk grows from 25,910 to 154,378 gzipped bytes while the closure as a whole shrinks. The entry stays far under that budget's 350 KB line, andscripts/check-eager-closure-budget.mjspasses with headroom up from 78.4 KB to 87.8 KB. Worth knowing before reading the Bundle Analysis comment, where the entry-chunk number jumps six-fold and the closure number is the one that matters.The saving is 0.30%, not 4% — and the reason is #5325
vendor-markdowndoes not move: 164,708 gzipped bytes, still eager. Three eager chunks import it statically and only one of them was this portal:plugin-chatbot, which reaches it directly and is itself eager;ui-components, becausepackages/fields'MarkdownContent—React.lazyin source — is folded into that eagerly imported chunk by theadvancedChunksgroup claiming everypackages/fieldsmodule.That second one is exactly the mechanism #5325 measured, arriving from a different direction. This card's hypothesis — that
apps/console/srcis claimed by no group and so a genuinely lazy page can get its own chunk — held: the console's own modules did cleave. What did not follow is the markdown vendor payload, because the console pages were never its only eager owner. Non-zero, but an order of magnitude below what the file list suggests.Reverse verification
Ablation:
git checkout origin/main -- apps/console/src/App.tsx, both legs re-run, direction predicted before running.App.docsPortalLazy.test.tsxexpect(loaded.DocsLayout).toBeUndefined(), positive control still greenexpected true to be undefinedat line 151; theSharedRecordPagecontrol on line 149 passed firstvite buildRestored with
git checkout HEAD -- apps/console/src/App.tsx;git status --porcelainempty andgit hash-objectof the working file equals the committed blob68d115b3, so the restore is byte-identical rather than merely clean-looking.A warning appears that was not there before, and it is not a regression
The build now reports one warning it did not report before:
That file is byte-identical to
mainin this branch (git diff origin/main...HEADtouches it not at all), and its laziness was already dead there: it statically importsregisterAppComponentfrom the same barrel on line 19 whilelazy()-importing the barrel on line 22, andApp.tsxonmainalready importsBuilderLandingstatically from it. The ablation confirms the direction — with the fix removed the warning goes away again, so what changed is reportability, not the defect: once the docs pages leave,srcfolds into the entry chunk and rolldown can see that thisimport()cannot move anything. Filed separately rather than folded in; the correct shape there is a decision (drop thelazy(), or stop importingBuilderLandingstatically inApp.tsx), not a mechanical edit.Tests
apps/console/src/__tests__/App.docsPortalLazy.test.tsxpins laziness as a runtime property, not as a spelling. Avi.mockfactory runs the first time its module is imported, so the flags it sets record when each page entered the graph: a static import inApp.tsxsets the flag whileApp.tsxis evaluated, before any test body runs. A regex over the source would pin one spelling of the mistake; this pins the mistake.Two counter-probes, because "never loaded" is what a flag that can never be set also looks like:
SharedRecordPageis a live positive control (still statically imported, so its flag must already be set), and visiting/docsmust flip the layout's flag and render it through the Suspense boundary.DocsSlugandDocPagemust still be unloaded afterwards, since/docsmatches neither.internalFormShell.test.tsx's@object-ui/app-shellmock gainsLoadingScreen. This is required, not optional:App.tsx's route elements are constructed whenApprenders, so the Suspense fallback's export is read even by a test that never visits/docs, and without it both of its tests died on the vitest mock proxy. That failure was observed before it was fixed.Surface note: the card's declared file surface was
apps/console/src/{App,AppContent}.tsxplussrc/pages/**. The two test files above sit insrc/__tests__/— the new one is a new path that cannot collide, and the mock repair is mandated by this change rather than chosen.Verification run on
f9bbd4144pnpm exec vitest run apps/console/pnpm --filter @object-ui/console type-checkpnpm --filter @object-ui/console lintcheck:eager-closurecheck:control-bytescheck:self-import,check:esm-specifiers,check:phantom-depscheck-changeset-presence,check-changeset-no-majorChangeset:
.changeset/console-lazy-docs-portal-5467.md(@object-ui/console: patch).Generated by Claude Code