Support for webpack scenarios and fixes for bundleAnalysisRepo (--webpack-dir / fetch-on-reuse) - #27627
Conversation
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (411 lines, 13 files), I've queued these reviewers:
How this works
|
…pack-dir / fetch-on-reuse fixes
3c09e44 to
a4e8ae4
Compare
… CommonJS Convert the scenario's webpack.config.cts to webpack.config.cjs so webpack auto-detects it via 'npx webpack' (matching the sibling root config). Because a CommonJS config needs no TS loader and isn't type-checked, terser-webpack-plugin's Node16 .d.ts issue no longer surfaces. As a result, drop skipLibCheck and the webpack.config.cts entry from tsconfig.json (the config is no longer part of the TS program) and remove the now-unneeded eslint override. build:compile's tsc passes clean without skipLibCheck, and 'flub generate bundleAnalysisRepo' collects the bundle stats successfully. Also add a justification to the import-x/no-internal-modules eslint-disable in the scenario's index.ts.
Collapse the tsconfig 'include' array onto a single line so 'biome check' passes; the multi-line form failed the client build's format check.
Alex Villarreal (alexvy86)
left a comment
There was a problem hiding this comment.
Overall idea makes sense to me. A couple of things:
- I don't know enough of the legacy API surface to guarantee that the scenario config covered it all. Could suggest some form of programmatic validation of that as an eventual follow-up (and maybe note that in the documentation of that file).
- A few comments below. Not an expert in webpack so I only called out the bit that stood out to me in the new scenario config; at a glance the config made sense but I could have missed something else there.
collectBundle: replace 'npx webpack' (which can silently download webpack from the registry) with 'pnpm exec webpack --config <scenario config>' run from the package root, and add ensureWebpackInstalled() to fail early with an actionable error when the package does not declare webpack as a devDependency. bundle-size-tests scenario: switch output.library.type from 'jsonp' to 'var' to match the sibling root webpack config. Also clarify the --webpack-dir flag description (compareBundlesOutput subdirectory).
… reading the package.json.
Alex Villarreal (alexvy86)
left a comment
There was a problem hiding this comment.
LGTM :)
Bundle size comparisonBase commit: Pending — |
This change consists of two improvements to the
bundleAnalysisRepotooling inbuild-cli, one of which enables webpack "scenarios."Description
Webpack scenarios are a feature that enables the analysis of webpack configurations and specific sets of imports that are designed to match real use cases. This differs from our typical approach of using a single webpack config for repo-wide analysis. New scenarios are defined under /examples/utils/bundle-size-tests/scenarios.
--webpack-dirflag: Decouples the directory that webpack runs in (and whereanalyzer.json/compareBundlesOutputare collected) from--package-dir(wherebuild:compileruns). This lets a scenario subdirectory that only contains a webpack config be built and collected correctly. Previouslynpm run webpackin a config-less subdir walked up to the parentpackage.jsonand built the wrong config. The flag is added togenerate bundleAnalysisRepo,generate bundleAnalysisReposWithComparison, andcheck bundleAnalysisReposComparison(defaulting to--package-dir, so existing usage is unchanged).Fetch-on-reuse for the inner base-repo clone: When an inner
base-repoclone already exists,ensureInnerRepoAtRevisionnow runsgit fetch origin --no-tags --prunebefore checkout, bringing a reused clone to parity with a fresh one. This fixes the stale-clonefatal: unable to read tree <sha>failure for merge-base / revision SHAs created after the original clone, without forcing a full re-clone.Also adds an
encapsulated-no-treebundle scenario underexamples/utils/bundle-size-testsused to exercise and validate both features end-to-end.Reviewer Guidance
Todo