docs(ui): analyze remaining ui and preview bundle size levers - #10632
Open
GiladShoham wants to merge 3 commits into
Open
docs(ui): analyze remaining ui and preview bundle size levers#10632GiladShoham wants to merge 3 commits into
GiladShoham wants to merge 3 commits into
Conversation
PR Summary by QodoDocument UI bundle levers and add preview bundle statistics
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. Analyzer shortcut always fails
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more' |
guysaar223
approved these changes
Aug 18, 2026
GiladShoham
force-pushed
the
test/ui-start-e2e
branch
from
August 19, 2026 05:46
28f6b6e to
c8748e6
Compare
GiladShoham
added a commit
that referenced
this pull request
Aug 19, 2026
…10628/#10629 (#10631) Sanity e2e for `bit start` itself, covering both UI roots. Part 3 of #10596 follow-up work. > Rebased onto `master` now that #10628 and #10629 have merged; targets `master` directly. Until now nothing exercised `bit start` end to end. That is how the scope SSR bundle managed to throw on every request for months (#10628), and the layout change in #10629 has a matching failure mode: name the fallback document wrong and every *client-side* route 404s while SSR-rendered ones keep working. `e2e/harmony/ui-start.e2e.ts` starts a real server per root and asserts, over http: - startup writes nothing matching `/error|exception|unhandled/i` to stderr — a server can listen fine with an aspect that failed to load - the served document has a react root - **every script and stylesheet the document references actually resolves 200** — this is the one that catches assets emitted under a path the server does not expose - a deep client-side route returns a document (the history-api fallback) - `/graphql` answers without errors - workspace only: the document loads the *workspace* entry and not the scope one — both roots are entries of one bundle now, so serving the wrong document would still look like a working page, just booting the other root's app - scope only: the markup is server-rendered and contains the exported component 12 assertions, ~1 min. All `--rebuild`, so they describe this repo's code rather than whichever bit release is installed. ## Two supporting changes **`HttpHelper` can start either root.** It was hardcoded to the bare scope (`scopes.remotePath`, and a ready-message string naming `teambit.scope/scope`). It now takes `{ extraArgs, uiRootAspectId }`, derives the cwd from the root, and builds the ready message per root. Existing callers use the unchanged two-arg form. It also records stderr so tests can assert on a clean startup. **`portHolders()` now filters to listening sockets** (`lsof -ti tcp:PORT -sTCP:LISTEN`). Without it lsof also reports processes holding a *client* socket to the port — including the mocha process itself, since node keeps connections alive after a test fetches from the server. `waitForPortToBeFree` read that as a foreign process squatting the port and refused to continue, failing the `after` hooks. Only a listener can actually hold a port. This was a latent bug in the helper; the new tests hit it because they fetch every referenced asset. --- ## Also: Qodo review findings from #10628 / #10629 Both of those merged before their review findings were addressed, so the actionable ones land here. Each was verified against the code rather than taken on trust. **`bit start` 404s on an existing local UI build (from #10629) — the important one.** `buildIfNoBundle()` treated *any* existing `public/bit` directory as a valid build, but the server now falls back to `<root>.html`, which a build made before #10629 does not contain. Reproduced end to end: with the pre-fix check the whole UI returns **404** on `/` and on deep routes; with the fix it detects the missing document, rebuilds, and serves 200. This would have hit every user upgrading past #10629 with a previously-built local UI. It now checks for the root's document rather than the directory. **Hash written for roots that were never built (from #10629).** `generateHash()` walked a hardcoded root list and threw when one was not registered. Beyond failing in a scope-only runtime, it could record a hash for a root whose document was never emitted — which reads at startup as "a pre-bundle exists" and then 404s, the same failure as above. It now walks the same registered roots `build()` turns into entries, via a new `UiMain.getUiRoots()`. **Service worker bound to a document that is not emitted (from #10629).** Confirmed in the built artifact: `service-worker.js` contained `createHandlerBoundToURL("public/index.html")` while the build emits only `scope.html` / `workspace.html`. With an entry per root there is no single app shell, so `navigateFallback` is removed — the express history-api fallback already serves the right document. Verified the built service worker no longer contains that binding. **Entry name collisions (from #10629).** `Object.fromEntries` would silently keep only the last of two entries sharing a sanitized name, leaving a root with no chunks and no document while still looking built. Now throws instead. **Stats filename could break (from #10628).** `writeBundleStats` interpolated an unsanitized name into a path, so a root name containing `/` would fail with ENOENT into a swallowed debug log. Now sanitized. Not changed: the "ad-hoc chalk in `writeStats`" rule violation. That line matches the surrounding `[Rspack]` log statements in the same file; the style guide it cites covers section titles and symbols in command output, not diagnostic log lines. Happy to switch it if you'd rather be strict. The `preview/bundle-stats.ts` copy of the sanitization fix lands with #10632, which is where that file lives.
GiladShoham
force-pushed
the
docs/bundle-size-analysis
branch
from
August 19, 2026 09:17
53d8234 to
2843c77
Compare
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| ```bash | ||
| BIT_UI_BUNDLE_STATS=1 bit build "teambit.ui-foundation/ui, teambit.preview/preview" \ | ||
| --tasks "BundleUI,PreBundlePreview" --reuse-capsules --unmodified | ||
| node scripts/analyze-bundle.mjs bundle-stats/*.stats.json # or: npm run analyze-bundle |
There was a problem hiding this comment.
1. Analyzer shortcut always fails 🐞 Bug ≡ Correctness
The documented npm run analyze-bundle alternative supplies no stats files, so the analyzer prints usage and exits with status 1. Users following the reproduction instructions cannot analyze the generated bundle stats through the advertised shortcut.
Agent Prompt
## Issue description
Correct the documented npm analyzer command because invoking `npm run analyze-bundle` without arguments exits with an error.
## Issue Context
The npm script only launches `scripts/analyze-bundle.mjs`, which requires at least one positional stats-file path. Document argument forwarding, for example `npm run analyze-bundle -- bundle-stats/*.stats.json`, or change the script to provide suitable defaults.
## Fix Focus Areas
- docs/ui-bundle-size-analysis.md[33-37]
- package.json[31-33]
- scripts/analyze-bundle.mjs[96-100]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 2843c77 |
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.
Adds
docs/ui-bundle-size-analysis.md— where the remaining bundle size sits after #10628 and #10629, what I measured, and what I tried that did not work. Written to be picked up cold in a later session.Env preview duplication is deliberately excluded — the core envs are being removed, which takes it along.
Also in here
BIT_UI_BUNDLE_STATS=1now covers the preview pre-bundle too, so one build producesbrowser,scope-ssrandpreviewstats together. It is a small copy of the UI helper rather than an import:@teambit/ui's index is imported by browser code, and re-exporting a node-only module through it pulledfspolyfills into the UI bundle (caught by the build failing onCan't resolve 'constants').toJsongroups assets and modules into summary rows ("assets by status") that carry a size but no name. That showed up as a single unattributable 3.3 MB / 17.9% bucket, andassets: 0. AllgroupModulesBy*/groupAssetsBy*flags are now off.analyze-bundle.mjscrashed on assets without aname(those same grouped rows).code-view.tsximportedcreateElementfrom thereact-syntax-highlighterpackage root, which defeats its ownprism-lightimport two lines later. Changed to the deep path. Behaviour-neutral, and worth stating plainly: it saves nothing today — see below.Headline findings
The two eagerly-loaded syntax highlighting registries are the biggest single item —
highlight.js(1.34 MB, every language) plusrefractor(0.85 MB, every Prism language), in both the browser and ssr bundles. Neither is imported directly anywhere in this repo. They come in throughreact-syntax-highlighter's package root, which re-exports every build including the full-language ones.The blocker is that the remaining root imports are in published
@teambitcomponents innode_modules(api-reference.renderers.schema-node-member-summary,documenter.ui.code-snippet), whose source is not in this repo. I verified this: fixing the in-repo imports changes the artifact by 0 bytes. Filed separately as #10633.I tried the bundler-level workaround (alias
lowlight→lowlight/lib/core,refractor→refractor/core) and rejected it: bit fails the build because both are transitive and would have to be declared dependencies of@teambit/ui, and it silently degrades any consumer relying on auto-registered languages to plain text. That is a product call, not a build one.lodashis the best effort-to-reward item. It is CJS-only (nomodulefield), so it cannot be tree-shaken, and the repo has 280from 'lodash'imports and zero cherry-picked ones. It is 0.52 MB of the 1.96 MB preview bundle — 28% — for six functions. Fixing it pays out in the browser, ssr and preview bundles at once.Also documented:
graphqlshipping whole into preview (28%),sucrase(0.47 MB) arriving viareact-liveand never lazy-loaded,date-fnsat 302 modules, and the fact that one 6.23 MB chunk is the entire eager payload — which is what makes the cold-cache first paint slower than client-only rendering (measured in #10628).@shikijs/langsis 1.45 MB but already lazy-loaded per language, and is called out as the pattern the rest of the UI should copy.Update after the stack merged
Rebased onto
master. Two follow-ups folded in:preview/bundle-stats.tsgets the filename sanitization that test(ui): bit start sanity e2e, plus fixes for the qodo findings on #10628/#10629 #10631 applied to its UI twin (a Qodo finding from fix(ui): repair scope SSR and shrink the ui pre-bundle 58MB to 24MB #10628). This file only exists on this branch, which is why it was carried over rather than fixed there.navigateFallbackthat still pointed at anindex.htmlthe multi-entry build stopped emitting, so the analysis notes that re-adding an offline shell now has to answer what that means for two roots.Qodo reviewed this PR and found no issues.
Re-validated on the rebased branch: full compile, fresh-capsule build of both bundles with
BIT_UI_BUNDLE_STATS=1, andanalyze-bundle.mjsreproducing the figures the doc quotes — 6.23 MB eager chunk,@shikijs/langs1.45 MB / 9.5%,highlight.js1.34 MB / 8.7%, and in previewgraphql0.52 MB / 27.9% next tolodash0.52 MB / 27.7%.