Replace old bundle-viz with two new chart examples#7390
Merged
Conversation
Contributor
WalkthroughThis PR migrates bundle-size visualization from a standalone app to integrated examples within the documentation site. It refactors bundle-data generation to support parameterized build options, produces a new JSON visualization file, and adds Treemap and Sunburst components that consume the generated data with fallback support. ChangesBundle Size Visualization Examples and Data Generation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle ReportBundle size has no change ✅ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
www/src/docs/exampleComponents/TreeMap/BundleSizeTreemap.tsx (2)
5-31: ⚡ Quick winImport the shared bundle-visualization types here.
These types duplicate
www/src/docs/exampleComponents/_bundleSize/types.ts, so the treemap loader can drift from the generator contract without a compiler catch. Import the shared exports instead of redeclaring them locally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@www/src/docs/exampleComponents/TreeMap/BundleSizeTreemap.tsx` around lines 5 - 31, The duplicated type declarations (BundleVisualizationStage, BundleVisualizationNode, BundleVisualizationEntry, BundleVisualizationData) should be removed and replaced with imports from the shared bundle-visualization types module used by the generator; update BundleSizeTreemap.tsx to import the shared types (e.g., BundleVisualizationStage, BundleVisualizationNode, BundleVisualizationEntry, BundleVisualizationData) instead of redeclaring them so the treemap loader and generator share the same contract and the TypeScript compiler will catch mismatches.
54-477: 🏗️ Heavy liftExtract the shared bundle-size helpers before these two examples diverge.
Most of this loader/transform/coloring code is mirrored in
www/src/docs/exampleComponents/SunburstChart/BundleSizeSunburst.tsx. Moving the common pieces into_bundleSizewould keep both examples aligned and remove a lot of double-maintenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@www/src/docs/exampleComponents/TreeMap/BundleSizeTreemap.tsx` around lines 54 - 477, Duplicate bundle-size loader/transform/coloring helpers are present; extract them into a shared module named _bundleSize and have both examples import the common code. Create a new module (_bundleSize) that exports the shared constants/types and functions (e.g. generatedBundleVisualizationAssetPath, BundleVisualization types, getWebsiteBaseUrl, loadGeneratedBundleVisualizationEntry, useBundleVisualizationEntry, collectLeafNodes, getPackageName, inferLibraryFamily, getDisplayPathSegments, insertLeaf, createBuildNode, buildTreemapDisplayData, getLibraryColorSeed, getNodeFill, finalizeDisplayNode, getHumanReadableSize, clamp, hashString), move their implementations there, update BundleSizeTreemap.tsx and BundleSizeSunburst.tsx to import the needed symbols, and remove the duplicated implementations from both files so they rely on the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-bundle-data.ts`:
- Around line 117-120: The thrown Error in the default branch of
parseBundleVisualizationTargets() lists only the "*-example" flags while the
function accepts both real and example aliases (--treemap, --sunburst,
--example, --treemap-example, --sunburst-example); update the error message
thrown for unknown argument (variable argument) to enumerate all supported flags
(include --example, --treemap, --treemap-example, --sunburst,
--sunburst-example) so the CLI help text matches the actual accepted aliases and
points users to valid options.
In `@scripts/treeshaking.ts`:
- Around line 149-163: The Rollup bundle may leak if bundle.generate throws
because bundle.close() is only called on the success path; update the
buildBundle (used by treeshake/getModuleGraph/bundle-size reporting) to always
close the Rollup bundle by creating the bundle variable before calling rollup,
then wrap the generate/close sequence in try/finally (or try/catch/finally) and
in the finally block await bundle.close() only if bundle is defined, rethrowing
any error so failures still propagate; ensure you reference the same bundle
variable used with bundle.generate and bundle.close so resources are cleaned on
error.
In `@www/package.json`:
- Around line 7-9: The build:client script currently always invokes
generate-bundle-data which hard-fails if ../es6/index.js is missing; update the
workflow so the root build is produced before generate-bundle-data or make
generate-bundle-data tolerant of a missing ../es6/index.js. Concretely, either
change the "build:client" NPM script to run the root build/preflight (e.g., run
the root package's build or a script that ensures ../es6/index.js exists) before
invoking "generate-bundle-data", or modify ../scripts/generate-bundle-data.ts to
detect a missing ../es6/index.js and either generate a fallback or exit
gracefully with a clearer message; reference the "build:client" script,
"generate-bundle-data" script, ../scripts/generate-bundle-data.ts, and the
../es6/index.js artifact to locate the code to change.
In `@www/src/docs/exampleComponents/TreeMap/index.ts`:
- Around line 16-23: The Treemap example registry entry (treeMapExamples ->
BundleSizeTreemap) is added but not referenced by the API examples list; update
the consumer in the Treemap examples file so the new example is reachable: open
the Treemap consumer (the module that currently hard-codes SimpleTreemap and
TreemapWithPaddingAndGaps), import or reference the BundleSizeTreemap entry from
treeMapExamples and add it to the exported examples array (the same list that
contains SimpleTreemap and TreemapWithPaddingAndGaps) so BundleSizeTreemap
appears on the Treemap examples page.
---
Nitpick comments:
In `@www/src/docs/exampleComponents/TreeMap/BundleSizeTreemap.tsx`:
- Around line 5-31: The duplicated type declarations (BundleVisualizationStage,
BundleVisualizationNode, BundleVisualizationEntry, BundleVisualizationData)
should be removed and replaced with imports from the shared bundle-visualization
types module used by the generator; update BundleSizeTreemap.tsx to import the
shared types (e.g., BundleVisualizationStage, BundleVisualizationNode,
BundleVisualizationEntry, BundleVisualizationData) instead of redeclaring them
so the treemap loader and generator share the same contract and the TypeScript
compiler will catch mismatches.
- Around line 54-477: Duplicate bundle-size loader/transform/coloring helpers
are present; extract them into a shared module named _bundleSize and have both
examples import the common code. Create a new module (_bundleSize) that exports
the shared constants/types and functions (e.g.
generatedBundleVisualizationAssetPath, BundleVisualization types,
getWebsiteBaseUrl, loadGeneratedBundleVisualizationEntry,
useBundleVisualizationEntry, collectLeafNodes, getPackageName,
inferLibraryFamily, getDisplayPathSegments, insertLeaf, createBuildNode,
buildTreemapDisplayData, getLibraryColorSeed, getNodeFill, finalizeDisplayNode,
getHumanReadableSize, clamp, hashString), move their implementations there,
update BundleSizeTreemap.tsx and BundleSizeSunburst.tsx to import the needed
symbols, and remove the duplicated implementations from both files so they rely
on the single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46d396c3-d4de-4237-8363-76d7afa3bb4d
⛔ Files ignored due to path filters (16)
bundle-viz/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsontest-vr/__snapshots__/tests/AreaChart/stacked.spec-vr.tsx-snapshots/multiple-data-arrays-exclusive-dataKey-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/AreaChart/stacked.spec-vr.tsx-snapshots/multiple-data-arrays-repeated-dataKey-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/AreaChart/stacked.spec-vr.tsx-snapshots/multiple-data-arrays-repeated-dataKey-without-XAxis-dataKey-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/AreaChart/stacked.spec-vr.tsx-snapshots/single-data-array-without-XAxis-dataKey-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/Bar.6192.spec-vr.tsx-snapshots/ComposedChart-with-stacked-Area-and-Bar-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/SunburstChartExamples.spec-vr.tsx-snapshots/BundleSizeSunburst-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/SunburstChartExamples.spec-vr.tsx-snapshots/BundleSizeSunburst-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/SunburstChartExamples.spec-vr.tsx-snapshots/BundleSizeSunburst-1-webkit-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/TreemapApiExamples.spec-vr.tsx-snapshots/BundleSizeTreemap-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/TreemapApiExamples.spec-vr.tsx-snapshots/BundleSizeTreemap-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/TreemapApiExamples.spec-vr.tsx-snapshots/BundleSizeTreemap-1-webkit-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/dark-mode.spec-vr.tsx-snapshots/dark-mode-examples-list-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/dark-mode.spec-vr.tsx-snapshots/dark-mode-examples-list-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/dark-mode.spec-vr.tsx-snapshots/dark-mode-examples-list-1-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (26)
.gitignorebundle-viz/README.mdbundle-viz/index.htmlbundle-viz/package.jsonbundle-viz/src/App.tsxbundle-viz/src/bundle-data.jsonbundle-viz/src/main.tsxbundle-viz/tsconfig.jsonbundle-viz/vite.config.tsomnidoc/verifyExamples.spec.tspackage.jsonscripts/generate-bundle-data.test.tsscripts/generate-bundle-data.tsscripts/treeshaking.tstest-vr/tests/www/SunburstChartExamples.spec-vr.tsxtest-vr/tests/www/TreemapApiExamples.spec-vr.tsxvitest.config.mtswww/package.jsonwww/src/docs/exampleComponents/SunburstChart/BundleSizeSunburst.tsxwww/src/docs/exampleComponents/SunburstChart/SunburstChartNavExample.tsxwww/src/docs/exampleComponents/SunburstChart/index.tswww/src/docs/exampleComponents/TreeMap/BundleSizeTreemap.tsxwww/src/docs/exampleComponents/TreeMap/index.tswww/src/docs/exampleComponents/_bundleSize/types.tswww/src/docs/exampleComponents/index.tswww/src/views/ExampleView.css
💤 Files with no reviewable changes (8)
- bundle-viz/vite.config.ts
- bundle-viz/README.md
- bundle-viz/index.html
- bundle-viz/package.json
- bundle-viz/src/bundle-data.json
- bundle-viz/src/App.tsx
- bundle-viz/tsconfig.json
- bundle-viz/src/main.tsx
Comment on lines
+117
to
+120
| default: | ||
| throw new Error( | ||
| `Unknown argument "${argument}". Supported options: --example, --treemap-example, --sunburst-example.`, | ||
| ); |
Contributor
There was a problem hiding this comment.
Keep the error text in sync with the accepted flags.
parseBundleVisualizationTargets() accepts --treemap and --sunburst, but the thrown message only advertises the *-example forms. That makes the CLI look broken when someone passes a real supported alias.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/generate-bundle-data.ts` around lines 117 - 120, The thrown Error in
the default branch of parseBundleVisualizationTargets() lists only the
"*-example" flags while the function accepts both real and example aliases
(--treemap, --sunburst, --example, --treemap-example, --sunburst-example);
update the error message thrown for unknown argument (variable argument) to
enumerate all supported flags (include --example, --treemap, --treemap-example,
--sunburst, --sunburst-example) so the CLI help text matches the actual accepted
aliases and points users to valid options.
Comment on lines
149
to
+163
| const bundle = await rollup({ | ||
| input: tmpFile, | ||
| external: getExternals(), | ||
| external: getExternals(options), | ||
| treeshake: { | ||
| moduleSideEffects: false, | ||
| }, | ||
| logLevel: 'silent', | ||
| plugins: [ | ||
| { | ||
| name: 'recharts-alias', | ||
| resolveId(source) { | ||
| if (source === 'recharts') { | ||
| return srcEntry; | ||
| } | ||
| if (source.startsWith('file://')) { | ||
| return fileURLToPath(source); | ||
| } | ||
| return null; | ||
| }, | ||
| }, | ||
| nodeResolve({ | ||
| extensions: ['.mjs', '.js', '.jsx', '.json', '.node', '.ts', '.tsx'], | ||
| moduleDirectories: ['node_modules'], | ||
| }), | ||
| babel({ | ||
| babelHelpers: 'bundled', | ||
| extensions: ['.js', '.jsx', '.ts', '.tsx'], | ||
| presets: [ | ||
| ['@babel/preset-env', { targets: { node: 'current' } }], | ||
| ['@babel/preset-react', { runtime: 'automatic' }], | ||
| '@babel/preset-typescript', | ||
| ], | ||
| }), | ||
| resolveDirectoryIndexPlugin, | ||
| ], | ||
| plugins: getRollupPlugins(options, graphByModuleId), | ||
| }); | ||
| const output = await bundle.generate({ format: 'esm' }); | ||
| await bundle.close(); | ||
| return output.output; | ||
| return { | ||
| output: output.output, | ||
| graphByModuleId, | ||
| }; |
Contributor
There was a problem hiding this comment.
Always close the Rollup bundle on failures.
If bundle.generate(...) throws, bundle.close() is skipped and the shared helper leaks Rollup resources on the error path. Since buildBundle(...) now backs treeshake, getModuleGraph, and bundle-size reporting, one failed build can leave later calls in a dirty state.
Possible fix
- const bundle = await rollup({
- input: tmpFile,
- external: getExternals(options),
- treeshake: {
- moduleSideEffects: false,
- },
- logLevel: 'silent',
- plugins: getRollupPlugins(options, graphByModuleId),
- });
- const output = await bundle.generate({ format: 'esm' });
- await bundle.close();
- return {
- output: output.output,
- graphByModuleId,
- };
+ const bundle = await rollup({
+ input: tmpFile,
+ external: getExternals(options),
+ treeshake: {
+ moduleSideEffects: false,
+ },
+ logLevel: 'silent',
+ plugins: getRollupPlugins(options, graphByModuleId),
+ });
+ try {
+ const output = await bundle.generate({ format: 'esm' });
+ return {
+ output: output.output,
+ graphByModuleId,
+ };
+ } finally {
+ await bundle.close();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const bundle = await rollup({ | |
| input: tmpFile, | |
| external: getExternals(), | |
| external: getExternals(options), | |
| treeshake: { | |
| moduleSideEffects: false, | |
| }, | |
| logLevel: 'silent', | |
| plugins: [ | |
| { | |
| name: 'recharts-alias', | |
| resolveId(source) { | |
| if (source === 'recharts') { | |
| return srcEntry; | |
| } | |
| if (source.startsWith('file://')) { | |
| return fileURLToPath(source); | |
| } | |
| return null; | |
| }, | |
| }, | |
| nodeResolve({ | |
| extensions: ['.mjs', '.js', '.jsx', '.json', '.node', '.ts', '.tsx'], | |
| moduleDirectories: ['node_modules'], | |
| }), | |
| babel({ | |
| babelHelpers: 'bundled', | |
| extensions: ['.js', '.jsx', '.ts', '.tsx'], | |
| presets: [ | |
| ['@babel/preset-env', { targets: { node: 'current' } }], | |
| ['@babel/preset-react', { runtime: 'automatic' }], | |
| '@babel/preset-typescript', | |
| ], | |
| }), | |
| resolveDirectoryIndexPlugin, | |
| ], | |
| plugins: getRollupPlugins(options, graphByModuleId), | |
| }); | |
| const output = await bundle.generate({ format: 'esm' }); | |
| await bundle.close(); | |
| return output.output; | |
| return { | |
| output: output.output, | |
| graphByModuleId, | |
| }; | |
| const bundle = await rollup({ | |
| input: tmpFile, | |
| external: getExternals(options), | |
| treeshake: { | |
| moduleSideEffects: false, | |
| }, | |
| logLevel: 'silent', | |
| plugins: getRollupPlugins(options, graphByModuleId), | |
| }); | |
| try { | |
| const output = await bundle.generate({ format: 'esm' }); | |
| return { | |
| output: output.output, | |
| graphByModuleId, | |
| }; | |
| } finally { | |
| await bundle.close(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/treeshaking.ts` around lines 149 - 163, The Rollup bundle may leak if
bundle.generate throws because bundle.close() is only called on the success
path; update the buildBundle (used by treeshake/getModuleGraph/bundle-size
reporting) to always close the Rollup bundle by creating the bundle variable
before calling rollup, then wrap the generate/close sequence in try/finally (or
try/catch/finally) and in the finally block await bundle.close() only if bundle
is defined, rethrowing any error so failures still propagate; ensure you
reference the same bundle variable used with bundle.generate and bundle.close so
resources are cleaned on error.
Comment on lines
+7
to
+9
| "generate-bundle-data": "tsx ../scripts/generate-bundle-data.ts", | ||
| "build": "npm run build:client && npm run prerender && npm run postprocess-sitemap && npm run validate-sitemap", | ||
| "build:client": "npm run generate-bundle-data && vite build", |
Contributor
There was a problem hiding this comment.
build:client now has an unsatisfied root-build prerequisite.
This script always runs generate-bundle-data, but scripts/generate-bundle-data.ts hard-fails unless ../es6/index.js already exists. On a clean checkout, npm run build from www/ will now fail before Vite starts unless the root package was built separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@www/package.json` around lines 7 - 9, The build:client script currently
always invokes generate-bundle-data which hard-fails if ../es6/index.js is
missing; update the workflow so the root build is produced before
generate-bundle-data or make generate-bundle-data tolerant of a missing
../es6/index.js. Concretely, either change the "build:client" NPM script to run
the root build/preflight (e.g., run the root package's build or a script that
ensures ../es6/index.js exists) before invoking "generate-bundle-data", or
modify ../scripts/generate-bundle-data.ts to detect a missing ../es6/index.js
and either generate a fallback or exit gracefully with a clearer message;
reference the "build:client" script, "generate-bundle-data" script,
../scripts/generate-bundle-data.ts, and the ../es6/index.js artifact to locate
the code to change.
Comment on lines
16
to
+23
| export const treeMapExamples = { | ||
| BundleSizeTreemap: { | ||
| Component: BundleSizeTreemap, | ||
| sourceCode: bundleSizeTreemapSource, | ||
| name: 'Bundle Size Treemap', | ||
| description: | ||
| 'This chart shows actual bundle size of tree-shaken Recharts app. Click to individual rectangles to see nested folders and files.', | ||
| }, |
Contributor
There was a problem hiding this comment.
BundleSizeTreemap still won’t show up on the Treemap examples page.
This adds the registry entry, but www/src/docs/apiExamples/Treemap/index.tsx:4-7 still hard-codes only SimpleTreemap and TreemapWithPaddingAndGaps, so the new example remains unreachable in docs. Please wire it into the consuming API examples list in the same PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@www/src/docs/exampleComponents/TreeMap/index.ts` around lines 16 - 23, The
Treemap example registry entry (treeMapExamples -> BundleSizeTreemap) is added
but not referenced by the API examples list; update the consumer in the Treemap
examples file so the new example is reachable: open the Treemap consumer (the
module that currently hard-codes SimpleTreemap and TreemapWithPaddingAndGaps),
import or reference the BundleSizeTreemap entry from treeMapExamples and add it
to the exported examples array (the same list that contains SimpleTreemap and
TreemapWithPaddingAndGaps) so BundleSizeTreemap appears on the Treemap examples
page.
|
Tip All tests passed and all changes approved!🟢 UI Tests: 198 tests unchanged |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7390 +/- ##
==========================================
+ Coverage 89.05% 89.07% +0.01%
==========================================
Files 548 552 +4
Lines 40577 41454 +877
Branches 5581 5719 +138
==========================================
+ Hits 36136 36924 +788
- Misses 4432 4521 +89
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
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
1 participant
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.
New bundle visualization examples, no code changes
Summary by CodeRabbit
New Features
Tests
Chores