fix(ui): repair scope SSR and shrink the ui pre-bundle 58MB to 24MB - #10628
Conversation
PR Summary by QodoFix scope SSR failures and shrink shipped UI SSR bundle
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1. Ad-hoc chalk in writeStats
|
| const filePath = writeBundleStats(stats, name); | ||
| if (filePath) this.logger.console(`${chalk.magenta('[Rspack]')} wrote bundle stats to ${chalk.cyan(filePath)}`); |
There was a problem hiding this comment.
1. Ad-hoc chalk in writestats 📘 Rule violation ⚙ Maintainability
The new writeStats() CLI output uses direct chalk formatting and a hardcoded [Rspack] prefix instead of the repository’s shared CLI output formatting toolkit. This risks inconsistent CLI output styling across commands and bypasses the documented style guide.
Agent Prompt
## Issue description
`UiMain.writeStats()` prints CLI output using ad-hoc `chalk` formatting (e.g. `chalk.magenta('[Rspack]')`) rather than using the shared CLI output formatting toolkit.
## Issue Context
The repo requires CLI output to follow `scopes/harmony/cli/cli-output-style-guide.md` and use the shared formatter utilities from `@teambit/cli` (`scopes/harmony/cli/output-formatter.ts`) to keep output consistent and maintainable.
## Fix Focus Areas
- scopes/ui-foundation/ui/ui.main.runtime.ts[309-315]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const filePath = join(dir, `${name}.stats.json`); | ||
| writeFileSync(filePath, JSON.stringify(json)); |
There was a problem hiding this comment.
2. Stats path can break 🐞 Bug ◔ Observability
writeBundleStats() builds the output path with the unsanitized name, so names containing path separators can create an unintended nested path and make writeFileSync() fail (ENOENT). UiMain.writeStats() swallows that failure (debug-only), so enabling BIT_UI_BUNDLE_STATS may produce no stats files without any visible signal.
Agent Prompt
## Issue description
`writeBundleStats(stats, name)` uses `join(dir, `${name}.stats.json`)` and only creates `dir`. If `name` contains `/` or `\\`, the resulting `filePath` includes intermediate directories that do not exist, causing `writeFileSync()` to throw. The caller (`UiMain.writeStats`) catches and logs only at debug level, making the failure effectively silent when diagnostics are explicitly enabled.
## Issue Context
This is an opt-in diagnostics path (`BIT_UI_BUNDLE_STATS`), so it must be robust to odd names and should reliably write a file when enabled.
## Fix Focus Areas
- scopes/ui-foundation/ui/rspack/bundle-stats.ts[24-39]
- scopes/ui-foundation/ui/ui.main.runtime.ts[305-316]
## Suggested fix
- Sanitize `name` into a filename-safe value (e.g., replace `/` and `\\` with `_`, and prevent `..` segments).
- Ensure the parent directory of `filePath` exists (e.g., `mkdirpSync(dirname(filePath))`) before writing.
- (Optional) If writing fails, consider logging a `warn` (still not failing the build) so the user who opted in understands why no file appeared.
ⓘ 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 557eef4 |
|
Code review by qodo was updated up to the latest commit c1f9286 |
Part 1 of #10596. Cuts the shipped
@teambit/uipre-bundle from 58 MB to 24 MB, and fixes scope SSR, which turned out to have been throwing on every request for months.The workspace/scope single-compilation dedupe (~8 MB) is deliberately left for a follow-up PR — it needs an artifact-layout and
.hashredesign, and mixing it in would make the SSR fix hard to verify.Size: 58 MB → 24 MB
ui-bundle/scope/public/bit/ssr/index.jsui-bundle/scopeui-bundle/workspaceTwo causes, both in
rspack.ssr.config.ts:devtool: 'eval-cheap-module-source-map'wrapped every module ineval('…')with an inlined base64 source map — 2,566 of them, 23.4 MB, 60% of the file. It entered as'eval-cheap-source-map', // TODOand was later retuned in chore(webpack): better development sourcemaps #7147 as a development sourcemap change; it was never a deliberate choice for a shipped artifact. NowshouldUseSourceMap ? 'source-map' : false, matching the browser config's opt-in.optimization.minimizerat all. It now mirrors the browser build'sSwcJsMinimizerRspackPlugin.Scope SSR was dead, and it took three fixes
Only
scope.ui-root.tssetsssr: true. The middleware catches any render error and callsnext(), falling through to the staticindex.html— which looks completely normal in a browser. So the failure was invisible:curl /andcurl '/?rendering=client'returned byte-identical 1,027-byte responses with an empty<div id="root"></div>.Three independent defects were stacked:
cjsmissing from the SSR asset catch-all. The browser config excludes/\.(cjs|js|mjs|jsx|ts|tsx)$/; the SSR config omittedcjs, so every.cjsmodule was emitted as anasset/resourcewhose module value is the file's URL. A component imported from one reached React as a tag name →Minified React error #65= "Invalid tag: /public/ssr/<hash>.cjs". Introduced by the webpack→rspack migration (feat(workspace | preview): migrate from webpack to rspack #10187), which replaced a shared base config with two hand-written ones and addedcjsto only one.use-user-agentpresent at two versions (0.0.199 and 0.0.200). Each copy callscreateContext, soui.ui.runtime.tsxprovidedssrBrowserContexton one instance whileTooltip'suseUserAgentread the other, gotundefined, took the browser fallback and dereferencedwindowon the server. Fixed by adding it toresolveAlias— the list that exists for exactly this class of bug. Aliasing the hook fixes every consumer, so the duplicatedTooltipcopies need no aliasing of their own.windowin auseEffectdependency array —useCurrentUserhad}, [window.location.href]);. The effect body never runs on the server, but a dependency array is evaluated on every render, including the server one.All routes now render server-side and the log shows zero SSR failures:
//ui/button/ui/button/~codeA visible side effect: the document title was the build-time placeholder (
bit-local-88bfe855) and is now the actual scope name.Browser runtime effect
This turns SSR on for the first time in months, so it is a real runtime change. Bare scope on localhost, 7 runs, median, SSR vs
?rendering=client:SSR is ~5× faster to first paint on a repeat visit, and ~200 ms slower on a cold first load where the 6.4 MB of JS dominates and the larger HTML delays stylesheet discovery. Server startup is unchanged (~1.77 s); the first request drops from 0.79 s to 0.49 s while now doing a real render instead of throwing. Note localhost has ~0 latency, which flatters client rendering — over a network SSR's margin widens.
Testing
New e2e
e2e/harmony/ui-ssr.e2e.tsasserts on the served HTML, since a browser cannot distinguish a working SSR render from the fallback.It runs
bit start --rebuild, which matters:getBundleUiPathresolves throughgetAspectDirFromBvm, so without--rebuildthe server serves the pre-bundle from the installed bvm version and the test would assert on whatever bit release happens to be installed rather than on this code.HttpHelpergained an optional extra-args parameter for this.Verified the test actually catches the bug — with the
cjsfix reverted, 2 of the 3 assertions fail. (The third, an assertion that the scope name appears anywhere in the document, passed in both states because--rebuildputs the scope name in the static<title>; it now asserts inside#rootinstead.)Manual verification on both UI roots, with the freshly built artifact swapped into the bvm install and the released
.hashfiles preserved (a.hashmismatch makesbit startsilently rebuild locally instead of serving the artifact). Confirmed the new bundle was genuinely served by diffing content-hashed asset names against the originals. Both roots checked in a real browser: workspace and scope home, component page, code tab, API reference — no console errors. The one error found (componentChangedsubscription missing on a bare scope) reproduces identically on the original bundle and is pre-existing.Bundle analysis tooling
BIT_UI_BUNDLE_STATS=1makes the UI build write an rspack stats file per compilation, andscripts/analyze-bundle.mjs(npm run analyze-bundle) summarizes assets plus the heaviest packages and workspace scopes. No new dependency. This is what found defect 2 above.It also surfaced that 81–86 packages appear at more than one version in a single bundle (
@teambit/design.ui.tooltipat eight). That is only ~0.6 MB (~3% of module bytes), so it is a React-context correctness risk rather than a size lever — worth tracking separately.Not in scope
The six env preview pre-bundles (
@teambit/{react,node,mdx,env,aspect,readme}/artifacts/env-template) are 37.9 MB with only 6.4 MB of unique content — 31.5 MB is byte-identical across the six. That is now the largest single remaining item, bigger than the workspace/scope dedupe, and is not covered by #10596 as written.