-
Notifications
You must be signed in to change notification settings - Fork 965
fix(ui): repair scope SSR and shrink the ui pre-bundle 58MB to 24MB #10628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import { expect } from 'chai'; | ||
| import { IS_WINDOWS } from '@teambit/legacy.constants'; | ||
| import { Helper } from '@teambit/legacy.e2e-helper'; | ||
| import { HttpHelper } from '../http-helper'; | ||
|
|
||
| const PORT = 3025; | ||
|
|
||
| /** | ||
| * The scope UI root is the only one built with `ssr: true`, and its ssr middleware swallows a render | ||
| * failure by falling through to the client-rendered `index.html`. That fallback looks identical to a | ||
| * working page in a browser, so a broken ssr bundle is invisible without asserting on the *served | ||
| * html* - which is what this file does. It is how "Invalid tag" (react #65), caused by `.cjs` modules | ||
| * being emitted as assets in the ssr build, went unnoticed for months. | ||
| */ | ||
| (IS_WINDOWS ? describe.skip : describe)('scope UI server-side rendering', function () { | ||
| this.timeout(0); | ||
| let helper: Helper; | ||
| let httpHelper: HttpHelper; | ||
| let html: string; | ||
|
|
||
| before(async () => { | ||
| helper = new Helper(); | ||
| // `--rebuild` so the ssr bundle is compiled from this repo's rspack config. without it the | ||
| // server serves the pre-built bundle shipped by the installed bit version, and the assertions | ||
| // below would describe that release rather than the code under test. | ||
| httpHelper = new HttpHelper(helper, PORT, ['--rebuild']); | ||
| helper.scopeHelper.setWorkspaceWithRemoteScope(); | ||
| helper.fixtures.populateComponents(1, false); | ||
| helper.command.tagAllWithoutBuild(); | ||
| helper.command.export(); | ||
| await httpHelper.start(); | ||
| const response = await fetch(`http://localhost:${PORT}/`); | ||
| html = await response.text(); | ||
| }); | ||
|
|
||
| after(async () => { | ||
| await httpHelper.killHttp(); | ||
| helper.scopeHelper.destroy(); | ||
| }); | ||
|
|
||
| it('should render the app on the server, not fall back to an empty client-rendered root', () => { | ||
| // the client-only fallback is exactly `<div id="root"></div>`; anything ssr rendered puts markup | ||
| // inside it. asserting on the emptiness is what distinguishes the two - a 200 with valid html is | ||
| // returned either way. | ||
| expect(html).to.not.have.string('<div id="root"></div>'); | ||
| const rendered = html.match(/<div id="root"[^>]*>([\s\S]*)<\/div>/); | ||
| expect(rendered, 'no #root element in the served html').to.not.equal(null); | ||
| expect((rendered as RegExpMatchArray)[1].trim()).to.not.have.lengthOf(0); | ||
| }); | ||
|
|
||
| it('should not emit a module as an asset url where a component is expected', () => { | ||
| // the "Invalid tag" symptom: an emitted asset path reaching react as a tag name. | ||
| expect(html).to.not.match(/<\/?"?\/public\/ssr\//); | ||
| }); | ||
|
|
||
| it('should render the scope name into the markup, not just the document title', () => { | ||
| // deliberately scoped to the contents of `#root`: the static `index.html` already carries the | ||
| // scope name in its `<title>`, so asserting on the whole document would pass even when the ssr | ||
| // render failed and the client fallback was served. | ||
| const rendered = html.match(/<div id="root"[^>]*>([\s\S]*)<\/div>/); | ||
| expect(rendered, 'no #root element in the served html').to.not.equal(null); | ||
| expect((rendered as RegExpMatchArray)[1]).to.have.string(helper.scopes.remote); | ||
| }); | ||
| }); |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { join } from 'path'; | ||
| import { mkdirpSync, writeFileSync } from 'fs-extra'; | ||
|
|
||
| /** | ||
| * Set to a directory to write a stats file per compilation, or to `1`/`true` to write them under | ||
| * `<cwd>/bundle-stats`. Off by default: collecting the module graph is not free, and the stats file | ||
| * is far larger than the bundle it describes. | ||
| */ | ||
| export const BUNDLE_STATS_ENV_VAR = 'BIT_UI_BUNDLE_STATS'; | ||
|
|
||
| export function bundleStatsDir(): string | undefined { | ||
| const value = process.env[BUNDLE_STATS_ENV_VAR]; | ||
| if (!value) return undefined; | ||
| if (value === '1' || value === 'true') return join(process.cwd(), 'bundle-stats'); | ||
| return value; | ||
| } | ||
|
|
||
| /** | ||
| * Write an rspack stats file for a finished compilation, for `scripts/analyze-bundle.mjs` to read. | ||
| * | ||
| * Deliberately written outside the output directory: everything under an output dir is matched by | ||
| * the build task's artifact glob, so a stats file placed there would ship inside the package. | ||
| */ | ||
| export function writeBundleStats(stats: any, name: string): string | undefined { | ||
| const dir = bundleStatsDir(); | ||
| if (!dir || !stats) return undefined; | ||
| const json = stats.toJson({ | ||
| all: false, | ||
| assets: true, | ||
| chunks: true, | ||
| chunkModules: true, | ||
| modules: true, | ||
| reasons: false, | ||
| source: false, | ||
| }); | ||
| mkdirpSync(dir); | ||
| const filePath = join(dir, `${name}.stats.json`); | ||
| writeFileSync(filePath, JSON.stringify(json)); | ||
| return filePath; | ||
| } | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ import { UIServer } from './ui-server'; | |
| import { UIAspect, UIRuntime } from './ui.aspect'; | ||
| import createRspackBrowserConfig from './rspack/rspack.browser.config'; | ||
| import createRspackSsrConfig from './rspack/rspack.ssr.config'; | ||
| import { writeBundleStats } from './rspack/bundle-stats'; | ||
| import type { StartPlugin, StartPluginOptions } from './start-plugin'; | ||
| import { BundleUiTask, BUNDLE_UI_HASH_FILENAME } from './bundle-ui.task'; | ||
|
|
||
|
|
@@ -270,6 +271,7 @@ export class UiMain { | |
| this.clearConsole(); | ||
| throw new Error(results?.toString()); | ||
| } | ||
| this.writeStats(results, `${uiRoot.name}-browser`); | ||
|
|
||
| if (ssr) { | ||
| const ssrConfig = createRspackSsrConfig(outputPath, [mainEntry], publicDir); | ||
|
|
@@ -286,6 +288,7 @@ export class UiMain { | |
| this.clearConsole(); | ||
| throw new Error(ssrResults?.toString()); | ||
| } | ||
| this.writeStats(ssrResults, `${uiRoot.name}-ssr`); | ||
| } | ||
|
|
||
| return results; | ||
|
|
@@ -299,6 +302,19 @@ export class UiMain { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Opt-in, via `BIT_UI_BUNDLE_STATS` - see `rspack/bundle-stats.ts`. Never fails a build: this is | ||
| * diagnostics, and a bundle that compiled is still a bundle worth keeping. | ||
| */ | ||
| private writeStats(stats: any, name: string) { | ||
| try { | ||
| const filePath = writeBundleStats(stats, name); | ||
| if (filePath) this.logger.console(`${chalk.magenta('[Rspack]')} wrote bundle stats to ${chalk.cyan(filePath)}`); | ||
|
Comment on lines
+311
to
+312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Ad-hoc chalk in writestats 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
|
||
| } catch (err: any) { | ||
| this.logger.debug(`failed writing bundle stats for ${name}: ${err?.message || err}`); | ||
| } | ||
| } | ||
|
|
||
| registerStartPlugin(startPlugin: StartPlugin) { | ||
| this.startPluginSlot.register(startPlugin); | ||
| return this; | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| #!/usr/bin/env node | ||
| /* eslint no-console: 0 */ | ||
| /** | ||
| * Summarize an rspack stats file written by `BIT_UI_BUNDLE_STATS`. | ||
| * | ||
| * BIT_UI_BUNDLE_STATS=1 bit build "teambit.ui-foundation/ui" --tasks BundleUI --reuse-capsules --unmodified | ||
| * node scripts/analyze-bundle.mjs bundle-stats/scope-ssr.stats.json | ||
| * | ||
| * Prints the assets, then the heaviest npm packages and workspace scopes, so it is obvious which | ||
| * dependency is responsible for a bundle's size rather than only how large the bundle is. | ||
| */ | ||
| import { readFileSync, existsSync } from 'fs'; | ||
| import { basename } from 'path'; | ||
|
|
||
| const TOP = Number(process.env.TOP || 25); | ||
|
|
||
| function mb(bytes) { | ||
| return `${(bytes / 1024 / 1024).toFixed(2)} MB`; | ||
| } | ||
|
|
||
| /** | ||
| * Attribute a module to something a human can act on. Anything under node_modules is charged to its | ||
| * package (scoped names kept whole); everything else is charged to its workspace directory, which | ||
| * for this repo means the aspect or component it came from. | ||
| */ | ||
| function bucketOf(name) { | ||
| if (!name) return '(unknown)'; | ||
| const clean = name.replace(/^.*?!/, '').replace(/\?.*$/, ''); | ||
| const segments = clean.split(/[\\/]/); | ||
| const lastNodeModules = segments.lastIndexOf('node_modules'); | ||
| if (lastNodeModules !== -1) { | ||
| const first = segments[lastNodeModules + 1]; | ||
| if (!first) return '(node_modules)'; | ||
| const pkg = first.startsWith('@') ? `${first}/${segments[lastNodeModules + 2] ?? ''}` : first; | ||
| return `node_modules/${pkg}`; | ||
| } | ||
| for (const root of ['scopes', 'components', 'e2e']) { | ||
| const at = segments.indexOf(root); | ||
| if (at !== -1) return segments.slice(at, at + 3).join('/'); | ||
| } | ||
| return clean.startsWith('webpack') || clean.startsWith('external') ? '(runtime)' : '(other)'; | ||
| } | ||
|
|
||
| /** rspack nests concatenated/child modules; only leaves carry a size worth counting once. */ | ||
| function walkModules(modules, visit, seen = new Set()) { | ||
| for (const mod of modules ?? []) { | ||
| const id = mod.identifier ?? mod.name; | ||
| if (id && seen.has(id)) continue; | ||
| if (id) seen.add(id); | ||
| if (mod.modules?.length) walkModules(mod.modules, visit, seen); | ||
| else visit(mod); | ||
| } | ||
| } | ||
|
|
||
| function analyze(file) { | ||
| const stats = JSON.parse(readFileSync(file, 'utf8')); | ||
|
|
||
| console.log(`\n=== ${basename(file)} ===\n`); | ||
|
|
||
| const assets = (stats.assets ?? []).filter((a) => !a.name.endsWith('.map')).sort((a, b) => b.size - a.size); | ||
| const assetTotal = assets.reduce((sum, a) => sum + a.size, 0); | ||
| console.log(`assets: ${assets.length}, total ${mb(assetTotal)}\n`); | ||
| for (const asset of assets.slice(0, TOP)) { | ||
| console.log(` ${mb(asset.size).padStart(10)} ${asset.name}`); | ||
| } | ||
| if (assets.length > TOP) console.log(` ... and ${assets.length - TOP} more`); | ||
|
|
||
| const buckets = new Map(); | ||
| let moduleTotal = 0; | ||
| walkModules(stats.modules, (mod) => { | ||
| const size = mod.size ?? 0; | ||
| moduleTotal += size; | ||
| const bucket = bucketOf(mod.name ?? mod.identifier); | ||
| buckets.set(bucket, (buckets.get(bucket) ?? 0) + size); | ||
| }); | ||
|
|
||
| if (!buckets.size) { | ||
| console.log('\nno module information in this stats file.'); | ||
| return; | ||
| } | ||
|
|
||
| const ranked = [...buckets].sort((a, b) => b[1] - a[1]); | ||
| console.log(`\nmodules: ${mb(moduleTotal)} across ${buckets.size} packages/scopes (parsed, pre-minification)\n`); | ||
| for (const [bucket, size] of ranked.slice(0, TOP)) { | ||
| const share = ((size / moduleTotal) * 100).toFixed(1).padStart(5); | ||
| console.log(` ${mb(size).padStart(10)} ${share}% ${bucket}`); | ||
| } | ||
| if (ranked.length > TOP) { | ||
| const rest = ranked.slice(TOP).reduce((sum, [, size]) => sum + size, 0); | ||
| console.log(` ${mb(rest).padStart(10)} ... and ${ranked.length - TOP} more`); | ||
| } | ||
| } | ||
|
|
||
| const files = process.argv.slice(2); | ||
| if (!files.length) { | ||
| console.error('usage: node scripts/analyze-bundle.mjs <stats.json> [...]'); | ||
| process.exit(1); | ||
| } | ||
| for (const file of files) { | ||
| if (!existsSync(file)) { | ||
| console.error(`no such stats file: ${file}`); | ||
| process.exit(1); | ||
| } | ||
| analyze(file); | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Stats path can break
🐞 Bug◔ ObservabilityAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools