From a4f6cb5d31a80538ac5fa920d38a7fa43618ff03 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 4 Aug 2026 21:50:28 +0200 Subject: [PATCH 01/14] chore(react-headless-components-preview): add bundle isolation verification --- ...-8a1f0c62-4d3e-47b5-9c0a-1f2e6b7d5a94.json | 7 + .../library/eslint.config.js | 12 +- .../library/project.json | 17 + .../scripts/verify-bundle-isolation/cli.js | 302 ++++++++++++++++++ 4 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 change/@fluentui-react-headless-components-preview-8a1f0c62-4d3e-47b5-9c0a-1f2e6b7d5a94.json create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js diff --git a/change/@fluentui-react-headless-components-preview-8a1f0c62-4d3e-47b5-9c0a-1f2e6b7d5a94.json b/change/@fluentui-react-headless-components-preview-8a1f0c62-4d3e-47b5-9c0a-1f2e6b7d5a94.json new file mode 100644 index 00000000000000..ca8f9a07e94c4d --- /dev/null +++ b/change/@fluentui-react-headless-components-preview-8a1f0c62-4d3e-47b5-9c0a-1f2e6b7d5a94.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "chore: verify headless entry points do not bundle tabster, Griffel or react-icons", + "packageName": "@fluentui/react-headless-components-preview", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/packages/react-components/react-headless-components-preview/library/eslint.config.js b/packages/react-components/react-headless-components-preview/library/eslint.config.js index ec2e7cb1fc479f..6e76685858d5fd 100644 --- a/packages/react-components/react-headless-components-preview/library/eslint.config.js +++ b/packages/react-components/react-headless-components-preview/library/eslint.config.js @@ -2,4 +2,14 @@ const fluentPlugin = require('@fluentui/eslint-plugin'); -module.exports = [...fluentPlugin.configs['flat/react']]; +module.exports = [ + ...fluentPlugin.configs['flat/react'], + { + // Build-time verification tooling - not shipped, runs on Node, reports via stdout. + files: ['scripts/**/*.js'], + rules: { + 'no-console': 'off', + 'import/no-extraneous-dependencies': 'off', + }, + }, +]; diff --git a/packages/react-components/react-headless-components-preview/library/project.json b/packages/react-components/react-headless-components-preview/library/project.json index ecab81877a1e1f..03c21ae6298f97 100644 --- a/packages/react-components/react-headless-components-preview/library/project.json +++ b/packages/react-components/react-headless-components-preview/library/project.json @@ -10,6 +10,23 @@ "options": { "exportSubpaths": true } + }, + "verify-bundle-isolation": { + "cache": true, + "dependsOn": ["build", "^build"], + "command": "node scripts/verify-bundle-isolation/cli.js", + "options": { + "cwd": "{projectRoot}" + }, + "inputs": [ + "{projectRoot}/scripts/verify-bundle-isolation/cli.js", + "{projectRoot}/package.json", + { "externalDependencies": ["esbuild"] } + ], + "metadata": { + "technologies": ["esbuild"], + "description": "Assert entry points do not bundle tabster, Griffel or react-icons" + } } } } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js new file mode 100644 index 00000000000000..17553a3438abd5 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js @@ -0,0 +1,302 @@ +// @ts-check +/** + * Asserts that no entry point of this package bundles a runtime the headless API is + * meant to stay free of. + * + * Runs against the built `lib/` output (never `src/`) because tree shaking depends on + * `/*#__PURE__*\/` annotations that swc only adds at build time. + * + * Usage: node scripts/verify-bundle-isolation/cli.js + */ +const { existsSync, readFileSync } = require('node:fs'); +const { dirname, isAbsolute, join, resolve, sep } = require('node:path'); + +const esbuild = require('esbuild'); + +/** Packages that must never survive tree shaking. `@scope/*` matches the whole scope. */ +const FORBIDDEN = ['tabster', '@griffel/*', '@fluentui/react-icons']; + +/** + * Entry point -> forbidden packages that are tolerated for now. + * + * This baseline may only shrink. Removing a leak without deleting its entry here + * fails the check, so a fix cannot silently regress later. + * + * @type {Record} + */ +const KNOWN_VIOLATIONS = { + // https://github.com/microsoft/fluentui/pull/36503 + './tag-picker': ['@fluentui/react-icons', '@griffel/core', '@griffel/react'], + // https://github.com/microsoft/fluentui/pull/36504 + './teaching-popover': ['@fluentui/react-icons', '@griffel/core', '@griffel/react'], +}; + +/** Host-provided modules - including them would drown the signal. */ +const EXTERNALS = ['react', 'react-dom', 'react/jsx-runtime', 'react/compiler-runtime']; + +const ENTRY_SOURCEFILE = 'bundle-isolation-entry.js'; + +const packageRoot = resolve(__dirname, '../..'); +const workspaceRoot = findWorkspaceRoot(packageRoot); + +main().catch(error => { + console.error(error); + process.exit(1); +}); + +async function main() { + const packageJson = readJson(join(packageRoot, 'package.json')); + const entryPoints = Object.keys(packageJson.exports ?? {}).filter( + subpath => subpath.startsWith('.') && !subpath.endsWith('package.json') && !subpath.includes('*'), + ); + + if (entryPoints.length === 0) { + console.error(`No entry points found in ${packageJson.name} "exports" - nothing to verify.`); + process.exit(1); + } + + const results = await Promise.all(entryPoints.sort().map(subpath => verifyEntryPoint(subpath, packageJson.name))); + + const failures = collectFailures(results); + + if (failures.length === 0) { + console.log(`OK ${packageJson.name}: ${entryPoints.length} entry points free of ${FORBIDDEN.join(', ')}`); + return; + } + + console.error(`Bundle isolation check failed for ${packageJson.name}:\n`); + failures.forEach(failure => console.error(` ${failure}\n`)); + process.exit(1); +} + +/** + * @param {string} subpath + * @param {string} packageName + */ +async function verifyEntryPoint(subpath, packageName) { + const specifier = packageName + subpath.slice(1); + /** @type {{subpath: string, found: string[], rootCauses: string[], chains: Record, sourceResolved: string[], error?: string}} */ + const result = { subpath, found: [], rootCauses: [], chains: {}, sourceResolved: [] }; + + let metafile; + try { + ({ metafile } = await esbuild.build({ + stdin: { + contents: `export * from '${specifier}';\n`, + resolveDir: workspaceRoot, + sourcefile: ENTRY_SOURCEFILE, + loader: 'js', + }, + bundle: true, + write: false, + metafile: true, + format: 'esm', + platform: 'browser', + external: EXTERNALS, + absWorkingDir: workspaceRoot, + logLevel: 'silent', + // `tsconfig.base.json` maps every `@fluentui/*` specifier to `library/src/index.ts`, and esbuild + // honours those paths - which would verify sources instead of the published output. An inline + // empty config disables tsconfig discovery so resolution goes through `exports` only. + tsconfigRaw: {}, + conditions: ['import'], + })); + } catch (error) { + result.error = error instanceof Error ? error.message : String(error); + return result; + } + + const output = Object.values(metafile.outputs)[0]; + const kept = new Set(Object.keys(output.inputs)); + const entry = output.entryPoint ?? ENTRY_SOURCEFILE; + const ownerOf = createForbiddenOwnerResolver(); + + result.sourceResolved = [...kept].filter(modulePath => /[/\\]library[/\\]src[/\\]/.test(modulePath)); + + // The output's module list is authoritative for *what* leaked. + result.found = [...new Set([...kept].map(ownerOf).filter(Boolean))].sort(); + + // Chains are best effort: prefer a path made only of retained modules, but esbuild records + // import edges from the pre-shaking graph, so a retained module's only recorded importer may + // itself have been eliminated. Fall back to the full graph so every leak still gets a chain. + const keptChains = traceForbiddenPackages(metafile, entry, ownerOf, modulePath => kept.has(modulePath)); + const allChains = result.found.every(name => name in keptChains) + ? keptChains + : traceForbiddenPackages(metafile, entry, ownerOf, () => true); + result.chains = Object.fromEntries(result.found.map(name => [name, keptChains[name] ?? allChains[name] ?? []])); + + // A package reached *through* another forbidden package is a symptom, not a cause. + result.rootCauses = result.found.filter(name => + result.chains[name].slice(0, -1).every(step => { + const owner = ownerOf(step); + return owner === null || owner === name; + }), + ); + + return result; +} + +/** + * Breadth-first walk of the module graph, recording the shortest import chain from the + * entry to the first module of every forbidden package it reaches. + * + * One traversal (rather than a search per package) guarantees each reported chain is + * genuinely reachable and is the shortest one. + * + * @param {import('esbuild').Metafile} metafile + * @param {string} entry + * @param {(modulePath: string) => string | null} ownerOf + * @param {(modulePath: string) => boolean} allowEdge + * @returns {Record} + */ +function traceForbiddenPackages(metafile, entry, ownerOf, allowEdge) { + const previous = new Map([[entry, null]]); + const queue = [entry]; + /** @type {Record} */ + const chains = {}; + + while (queue.length > 0) { + const current = /** @type {string} */ (queue.shift()); + const owner = current === entry ? null : ownerOf(current); + + if (owner && !(owner in chains)) { + const chain = []; + for (let node = current; node; node = previous.get(node) ?? null) { + chain.unshift(node); + } + chains[owner] = chain.slice(1); // drop the synthetic entry module + } + + for (const imported of metafile.inputs[current]?.imports ?? []) { + if (!allowEdge(imported.path) || previous.has(imported.path)) { + continue; + } + previous.set(imported.path, current); + queue.push(imported.path); + } + } + + return chains; +} + +/** + * Maps a module path to the forbidden package owning it, or `null`. + * + * Ownership is resolved by walking up to the nearest `package.json`, which handles both + * `node_modules` dependencies and workspace packages (esbuild resolves symlinked workspace + * packages to their real path, so there is no `node_modules` segment to match on). + */ +function createForbiddenOwnerResolver() { + const exact = new Set(FORBIDDEN.filter(pattern => !pattern.endsWith('/*'))); + const scopes = FORBIDDEN.filter(pattern => pattern.endsWith('/*')).map(pattern => pattern.slice(0, -1)); + /** @type {Map} */ + const cache = new Map(); + + return function ownerOf(modulePath) { + let dir = dirname(isAbsolute(modulePath) ? modulePath : join(workspaceRoot, modulePath)); + const visited = []; + + while (dir && dir !== dirname(dir)) { + if (cache.has(dir)) { + const cached = cache.get(dir) ?? null; + visited.forEach(seen => cache.set(seen, cached)); + return cached; + } + visited.push(dir); + + const manifest = join(dir, 'package.json'); + if (existsSync(manifest)) { + const name = readJson(manifest).name; + // Nested manifests without a name (e.g. `{ "type": "module" }` markers) are not package roots. + if (name) { + const owner = exact.has(name) || scopes.some(scope => name.startsWith(scope)) ? name : null; + visited.forEach(seen => cache.set(seen, owner)); + return owner; + } + } + + dir = dirname(dir); + } + + visited.forEach(seen => cache.set(seen, null)); + return null; + }; +} + +/** @param {Array extends Promise ? T : never>} results */ +function collectFailures(results) { + const failures = []; + + for (const result of results) { + if (result.error) { + failures.push(`${result.subpath} could not be bundled - is the package built?\n ${result.error}`); + continue; + } + + if (result.sourceResolved.length > 0) { + failures.push( + `${result.subpath} resolved to package sources instead of built output, so the result is meaningless.\n` + + ` e.g. ${result.sourceResolved[0]}`, + ); + continue; + } + + const allowed = KNOWN_VIOLATIONS[result.subpath] ?? []; + const regressions = result.found.filter(name => !allowed.includes(name)); + const fixed = allowed.filter(name => !result.found.includes(name)); + + if (regressions.length > 0) { + const causes = regressions.filter(name => result.rootCauses.includes(name)); + const symptoms = regressions.filter(name => !causes.includes(name)); + const details = (causes.length > 0 ? causes : regressions) + .map(name => { + const chain = result.chains[name].map( + (step, index) => `${' '.repeat(index + 3)}|- ${relativeToWorkspace(step)}`, + ); + return ` ${name}\n${chain.join('\n')}`; + }) + .join('\n'); + const trailer = symptoms.length > 0 ? `\n ...which also pulls in ${symptoms.join(', ')}` : ''; + failures.push(`${result.subpath} pulls in forbidden runtime:\n${details}${trailer}`); + } + + if (fixed.length > 0) { + failures.push( + `${result.subpath} no longer pulls in ${fixed.join(', ')} - ` + + `remove it from KNOWN_VIOLATIONS in ${__filename.replace(workspaceRoot + sep, '')} to lock the fix in.`, + ); + } + } + + const verified = new Set(results.map(result => result.subpath)); + for (const [subpath, packages] of Object.entries(KNOWN_VIOLATIONS)) { + if (!verified.has(subpath)) { + failures.push(`KNOWN_VIOLATIONS lists "${subpath}" (${packages.join(', ')}) which is not an entry point.`); + } + } + + return failures; +} + +/** @param {string} startDir */ +function findWorkspaceRoot(startDir) { + let dir = startDir; + while (dir !== dirname(dir)) { + if (existsSync(join(dir, 'nx.json'))) { + return dir; + } + dir = dirname(dir); + } + throw new Error(`Could not locate the workspace root above ${startDir}`); +} + +/** @param {string} filePath */ +function readJson(filePath) { + return JSON.parse(readFileSync(filePath, 'utf-8')); +} + +/** @param {string} modulePath */ +function relativeToWorkspace(modulePath) { + const absolute = isAbsolute(modulePath) ? modulePath : resolve(workspaceRoot, modulePath); + return absolute.startsWith(workspaceRoot + sep) ? absolute.slice(workspaceRoot.length + 1) : modulePath; +} From a35dd1e0d19930946d11686dfca29b8d25d7116c Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 4 Aug 2026 22:13:25 +0200 Subject: [PATCH 02/14] refactor(react-headless-components-preview): configure bundle isolation --- .../library/bundle-isolation.config.json | 9 + .../library/project.json | 5 +- .../scripts/verify-bundle-isolation/README.md | 74 +++++++ .../scripts/verify-bundle-isolation/cli.js | 200 +++++++++++------- .../verify-bundle-isolation/schema.json | 49 +++++ 5 files changed, 264 insertions(+), 73 deletions(-) create mode 100644 packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json new file mode 100644 index 00000000000000..b2e422f148a784 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -0,0 +1,9 @@ +{ + "$schema": "./scripts/verify-bundle-isolation/schema.json", + "fixturesRoot": "./bundle-size", + "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], + "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], + "knownViolations": { + "AllComponents.fixture.js": ["@fluentui/react-icons", "@griffel/core", "@griffel/react"] + } +} diff --git a/packages/react-components/react-headless-components-preview/library/project.json b/packages/react-components/react-headless-components-preview/library/project.json index 03c21ae6298f97..085163037e6f0c 100644 --- a/packages/react-components/react-headless-components-preview/library/project.json +++ b/packages/react-components/react-headless-components-preview/library/project.json @@ -20,8 +20,11 @@ }, "inputs": [ "{projectRoot}/scripts/verify-bundle-isolation/cli.js", + "{projectRoot}/bundle-isolation.config.json", + "{projectRoot}/scripts/verify-bundle-isolation/schema.json", + "{projectRoot}/bundle-size/**/*", "{projectRoot}/package.json", - { "externalDependencies": ["esbuild"] } + { "externalDependencies": ["ajv", "esbuild"] } ], "metadata": { "technologies": ["esbuild"], diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md new file mode 100644 index 00000000000000..030684a069a602 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md @@ -0,0 +1,74 @@ +# Bundle isolation verification + +Verifies that headless package bundle-size fixtures do not retain browser runtimes that the headless API must avoid. + +The check bundles each `*.fixture.js` file with esbuild and inspects the retained module graph. It fails when a configured forbidden package survives tree shaking, when bundling resolves to package source instead of built output, or when a known violation becomes stale. + +## Usage + +Run from the package root after building the package and its dependencies: + +```sh +node scripts/verify-bundle-isolation/cli.js +``` + +Use a different package-root-relative configuration file with: + +```sh +node scripts/verify-bundle-isolation/cli.js --config ./bundle-isolation.config.json +``` + +The Nx target builds dependencies and runs the check with the correct working directory: + +```sh +yarn nx run react-headless-components-preview:verify-bundle-isolation +``` + +## Configuration + +The default configuration is `bundle-isolation.config.json` in the package root. Its schema is [`schema.json`](./schema.json). + +```json +{ + "$schema": "./scripts/verify-bundle-isolation/schema.json", + "fixturesRoot": "./bundle-size", + "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], + "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], + "knownViolations": { + "AllComponents.fixture.js": ["@fluentui/react-icons"] + } +} +``` + +All configured paths are resolved relative to the package root: + +- `fixturesRoot` is the directory containing bundle-size fixtures. +- `externals` lists host-provided modules excluded from the bundle. +- `forbiddenPackages` lists exact package names or scoped globs such as `@griffel/*`. +- `knownViolations` maps fixture paths, relative to `fixturesRoot`, to temporarily tolerated forbidden packages. + +## Fixtures + +Fixtures follow the existing Monosize convention in `bundle-size/*.fixture.js`. A fixture imports the public API under test and uses the import observably so tree shaking cannot discard it. + +```js +import * as HeadlessButton from '@fluentui/react-headless-components-preview/button'; + +console.log(HeadlessButton); + +export default { + name: 'HeadlessButton', +}; +``` + +Using the same fixtures keeps bundle isolation and bundle-size measurements aligned. + +## Known violations + +`knownViolations` is a shrink-only baseline: + +- A newly retained forbidden package fails the check. +- A package that no longer survives bundling also fails the check until its baseline entry is removed. +- A baseline entry for a missing fixture fails the check. + +This prevents fixed leaks from being silently reintroduced. diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js index 17553a3438abd5..72f943ed06e95a 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js @@ -1,6 +1,6 @@ // @ts-check /** - * Asserts that no entry point of this package bundles a runtime the headless API is + * Asserts that no bundle-size fixture in this package bundles a runtime the headless API is * meant to stay free of. * * Runs against the built `lib/` output (never `src/`) because tree shaking depends on @@ -8,59 +8,47 @@ * * Usage: node scripts/verify-bundle-isolation/cli.js */ -const { existsSync, readFileSync } = require('node:fs'); +const { existsSync, readdirSync, readFileSync } = require('node:fs'); const { dirname, isAbsolute, join, resolve, sep } = require('node:path'); +const { parseArgs } = require('node:util'); +const Ajv = /** @type {typeof import('ajv').default} */ (/** @type {unknown} */ (require('ajv'))); const esbuild = require('esbuild'); -/** Packages that must never survive tree shaking. `@scope/*` matches the whole scope. */ -const FORBIDDEN = ['tabster', '@griffel/*', '@fluentui/react-icons']; +/** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], knownViolations: Record}} Config */ +/** @typedef {{configPath: string, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ -/** - * Entry point -> forbidden packages that are tolerated for now. - * - * This baseline may only shrink. Removing a leak without deleting its entry here - * fails the check, so a fix cannot silently regress later. - * - * @type {Record} - */ -const KNOWN_VIOLATIONS = { - // https://github.com/microsoft/fluentui/pull/36503 - './tag-picker': ['@fluentui/react-icons', '@griffel/core', '@griffel/react'], - // https://github.com/microsoft/fluentui/pull/36504 - './teaching-popover': ['@fluentui/react-icons', '@griffel/core', '@griffel/react'], -}; - -/** Host-provided modules - including them would drown the signal. */ -const EXTERNALS = ['react', 'react-dom', 'react/jsx-runtime', 'react/compiler-runtime']; +const schemaPath = join(__dirname, 'schema.json'); -const ENTRY_SOURCEFILE = 'bundle-isolation-entry.js'; - -const packageRoot = resolve(__dirname, '../..'); -const workspaceRoot = findWorkspaceRoot(packageRoot); - -main().catch(error => { +main(processArgs()).catch(error => { console.error(error); process.exit(1); }); -async function main() { +/** @param {{configPath: string}} options */ +async function main(options) { + const packageRoot = dirname(options.configPath); + const workspaceRoot = findWorkspaceRoot(packageRoot); + const config = loadConfig({ ...options, workspaceRoot }); + const fixturesRoot = resolve(packageRoot, config.fixturesRoot); const packageJson = readJson(join(packageRoot, 'package.json')); - const entryPoints = Object.keys(packageJson.exports ?? {}).filter( - subpath => subpath.startsWith('.') && !subpath.endsWith('package.json') && !subpath.includes('*'), - ); + const fixtures = findFixtures(fixturesRoot); - if (entryPoints.length === 0) { - console.error(`No entry points found in ${packageJson.name} "exports" - nothing to verify.`); + if (fixtures.length === 0) { + console.error(`No bundle-size fixtures found in ${packageJson.name} - nothing to verify.`); process.exit(1); } - const results = await Promise.all(entryPoints.sort().map(subpath => verifyEntryPoint(subpath, packageJson.name))); + /** @type {RuntimeOptions} */ + const runtimeOptions = { ...options, config, fixturesRoot, packageRoot, workspaceRoot }; + const results = await Promise.all(fixtures.map(fixture => verifyFixture(fixture, runtimeOptions))); - const failures = collectFailures(results); + const failures = collectFailures(results, runtimeOptions); if (failures.length === 0) { - console.log(`OK ${packageJson.name}: ${entryPoints.length} entry points free of ${FORBIDDEN.join(', ')}`); + console.log( + `OK ${packageJson.name}: ${fixtures.length} bundle-size fixtures free of ${config.forbiddenPackages.join(', ')}`, + ); return; } @@ -69,31 +57,39 @@ async function main() { process.exit(1); } +function processArgs() { + const { values } = parseArgs({ + options: { + config: { + type: 'string', + default: 'bundle-isolation.config.json', + }, + }, + allowPositionals: false, + }); + + return { configPath: resolve(process.cwd(), values.config) }; +} + /** - * @param {string} subpath - * @param {string} packageName + * @param {string} fixture + * @param {RuntimeOptions} options */ -async function verifyEntryPoint(subpath, packageName) { - const specifier = packageName + subpath.slice(1); - /** @type {{subpath: string, found: string[], rootCauses: string[], chains: Record, sourceResolved: string[], error?: string}} */ - const result = { subpath, found: [], rootCauses: [], chains: {}, sourceResolved: [] }; +async function verifyFixture(fixture, options) { + /** @type {{fixture: string, found: string[], rootCauses: string[], chains: Record, sourceResolved: string[], error?: string}} */ + const result = { fixture, found: [], rootCauses: [], chains: {}, sourceResolved: [] }; let metafile; try { ({ metafile } = await esbuild.build({ - stdin: { - contents: `export * from '${specifier}';\n`, - resolveDir: workspaceRoot, - sourcefile: ENTRY_SOURCEFILE, - loader: 'js', - }, + entryPoints: [join(options.fixturesRoot, fixture)], bundle: true, write: false, metafile: true, format: 'esm', platform: 'browser', - external: EXTERNALS, - absWorkingDir: workspaceRoot, + external: options.config.externals, + absWorkingDir: options.workspaceRoot, logLevel: 'silent', // `tsconfig.base.json` maps every `@fluentui/*` specifier to `library/src/index.ts`, and esbuild // honours those paths - which would verify sources instead of the published output. An inline @@ -108,8 +104,13 @@ async function verifyEntryPoint(subpath, packageName) { const output = Object.values(metafile.outputs)[0]; const kept = new Set(Object.keys(output.inputs)); - const entry = output.entryPoint ?? ENTRY_SOURCEFILE; - const ownerOf = createForbiddenOwnerResolver(); + const entry = output.entryPoint; + const ownerOf = createForbiddenOwnerResolver(options); + + if (!entry) { + result.error = 'esbuild did not report an entry point for the fixture output'; + return result; + } result.sourceResolved = [...kept].filter(modulePath => /[/\\]library[/\\]src[/\\]/.test(modulePath)); @@ -150,6 +151,7 @@ async function verifyEntryPoint(subpath, packageName) { * @returns {Record} */ function traceForbiddenPackages(metafile, entry, ownerOf, allowEdge) { + /** @type {Map} */ const previous = new Map([[entry, null]]); const queue = [entry]; /** @type {Record} */ @@ -161,7 +163,7 @@ function traceForbiddenPackages(metafile, entry, ownerOf, allowEdge) { if (owner && !(owner in chains)) { const chain = []; - for (let node = current; node; node = previous.get(node) ?? null) { + for (let node = /** @type {string | null} */ (current); node; node = previous.get(node) ?? null) { chain.unshift(node); } chains[owner] = chain.slice(1); // drop the synthetic entry module @@ -186,14 +188,18 @@ function traceForbiddenPackages(metafile, entry, ownerOf, allowEdge) { * `node_modules` dependencies and workspace packages (esbuild resolves symlinked workspace * packages to their real path, so there is no `node_modules` segment to match on). */ -function createForbiddenOwnerResolver() { - const exact = new Set(FORBIDDEN.filter(pattern => !pattern.endsWith('/*'))); - const scopes = FORBIDDEN.filter(pattern => pattern.endsWith('/*')).map(pattern => pattern.slice(0, -1)); +/** @param {RuntimeOptions} options */ +function createForbiddenOwnerResolver(options) { + const exact = new Set(options.config.forbiddenPackages.filter(pattern => !pattern.endsWith('/*'))); + const scopes = options.config.forbiddenPackages + .filter(pattern => pattern.endsWith('/*')) + .map(pattern => pattern.slice(0, -1)); /** @type {Map} */ const cache = new Map(); + /** @param {string} modulePath */ return function ownerOf(modulePath) { - let dir = dirname(isAbsolute(modulePath) ? modulePath : join(workspaceRoot, modulePath)); + let dir = dirname(isAbsolute(modulePath) ? modulePath : join(options.workspaceRoot, modulePath)); const visited = []; while (dir && dir !== dirname(dir)) { @@ -223,25 +229,28 @@ function createForbiddenOwnerResolver() { }; } -/** @param {Array extends Promise ? T : never>} results */ -function collectFailures(results) { +/** + * @param {Array extends Promise ? T : never>} results + * @param {RuntimeOptions} options + */ +function collectFailures(results, options) { const failures = []; for (const result of results) { if (result.error) { - failures.push(`${result.subpath} could not be bundled - is the package built?\n ${result.error}`); + failures.push(`${result.fixture} could not be bundled - is the package built?\n ${result.error}`); continue; } if (result.sourceResolved.length > 0) { failures.push( - `${result.subpath} resolved to package sources instead of built output, so the result is meaningless.\n` + + `${result.fixture} resolved to package sources instead of built output, so the result is meaningless.\n` + ` e.g. ${result.sourceResolved[0]}`, ); continue; } - const allowed = KNOWN_VIOLATIONS[result.subpath] ?? []; + const allowed = options.config.knownViolations[result.fixture] ?? []; const regressions = result.found.filter(name => !allowed.includes(name)); const fixed = allowed.filter(name => !result.found.includes(name)); @@ -251,33 +260,77 @@ function collectFailures(results) { const details = (causes.length > 0 ? causes : regressions) .map(name => { const chain = result.chains[name].map( - (step, index) => `${' '.repeat(index + 3)}|- ${relativeToWorkspace(step)}`, + (step, index) => `${' '.repeat(index + 3)}|- ${relativeToWorkspace(step, options.workspaceRoot)}`, ); return ` ${name}\n${chain.join('\n')}`; }) .join('\n'); const trailer = symptoms.length > 0 ? `\n ...which also pulls in ${symptoms.join(', ')}` : ''; - failures.push(`${result.subpath} pulls in forbidden runtime:\n${details}${trailer}`); + failures.push(`${result.fixture} pulls in forbidden runtime:\n${details}${trailer}`); } if (fixed.length > 0) { failures.push( - `${result.subpath} no longer pulls in ${fixed.join(', ')} - ` + - `remove it from KNOWN_VIOLATIONS in ${__filename.replace(workspaceRoot + sep, '')} to lock the fix in.`, + `${result.fixture} no longer pulls in ${fixed.join(', ')} - ` + + `remove it from config.knownViolations in ${relativeToWorkspace( + options.configPath, + options.workspaceRoot, + )} to lock the fix in.`, ); } } - const verified = new Set(results.map(result => result.subpath)); - for (const [subpath, packages] of Object.entries(KNOWN_VIOLATIONS)) { - if (!verified.has(subpath)) { - failures.push(`KNOWN_VIOLATIONS lists "${subpath}" (${packages.join(', ')}) which is not an entry point.`); + const verified = new Set(results.map(result => result.fixture)); + for (const [fixture, packages] of Object.entries(options.config.knownViolations)) { + if (!verified.has(fixture)) { + failures.push( + `config.knownViolations lists "${fixture}" (${packages.join(', ')}) which is not a bundle-size fixture.`, + ); } } return failures; } +/** @param {string} root */ +function findFixtures(root) { + if (!existsSync(root)) { + return []; + } + + return readdirSync(root, { recursive: true, withFileTypes: true }) + .filter(entry => entry.isFile() && entry.name.endsWith('.fixture.js')) + .map(entry => { + const path = join(entry.parentPath, entry.name); + return path.slice(root.length + 1); + }) + .sort(); +} + +/** + * @param {{configPath: string, workspaceRoot: string}} options + * @returns {Config} + */ +function loadConfig(options) { + const config = readJson(options.configPath); + const schema = /** @type {object} */ (readJson(schemaPath)); + const validate = new Ajv({ allErrors: true }).compile(schema); + + if (!validate(config)) { + const errors = (validate.errors ?? []) + .map(/** @param {import('ajv').ErrorObject} error */ error => `${error.instancePath || '/'} ${error.message}`) + .join('\n '); + throw new Error( + `Invalid bundle isolation configuration at ${relativeToWorkspace( + options.configPath, + options.workspaceRoot, + )}:\n ${errors}`, + ); + } + + return /** @type {Config} */ (config); +} + /** @param {string} startDir */ function findWorkspaceRoot(startDir) { let dir = startDir; @@ -295,8 +348,11 @@ function readJson(filePath) { return JSON.parse(readFileSync(filePath, 'utf-8')); } -/** @param {string} modulePath */ -function relativeToWorkspace(modulePath) { +/** + * @param {string} modulePath + * @param {string} workspaceRoot + */ +function relativeToWorkspace(modulePath, workspaceRoot) { const absolute = isAbsolute(modulePath) ? modulePath : resolve(workspaceRoot, modulePath); return absolute.startsWith(workspaceRoot + sep) ? absolute.slice(workspaceRoot.length + 1) : modulePath; } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json new file mode 100644 index 00000000000000..9d168893578fb3 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json @@ -0,0 +1,49 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema", + "title": "Bundle isolation configuration", + "type": "object", + "additionalProperties": false, + "properties": { + "$schema": { + "type": "string", + "const": "./scripts/verify-bundle-isolation/schema.json" + }, + "fixturesRoot": { + "description": "Package-relative directory containing bundle-size fixtures.", + "type": "string", + "minLength": 1 + }, + "externals": { + "description": "Modules supplied by the consuming application rather than this bundle.", + "type": "array", + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + }, + "forbiddenPackages": { + "description": "Package names or scoped package globs that must not survive tree shaking.", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + }, + "knownViolations": { + "description": "Bundle-size fixture paths mapped to forbidden packages tolerated temporarily.", + "type": "object", + "additionalProperties": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + } + } + }, + "required": ["fixturesRoot", "externals", "forbiddenPackages", "knownViolations"] +} From ccd95cc3cee0baea4c9e8384c7897b7af11bb4eb Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 4 Aug 2026 22:17:58 +0200 Subject: [PATCH 03/14] ci: verify headless bundle isolation in PRs --- .github/workflows/pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index f82fd687e1402e..85d83e704cf12e 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -74,7 +74,7 @@ jobs: - name: build, test, lint, test-ssr (affected) run: | - FLUENT_JEST_WORKER=2 yarn nx affected -t build test lint type-check test-ssr test-integration verify-packaging --nxBail + FLUENT_JEST_WORKER=2 yarn nx affected -t build test lint type-check test-ssr test-integration verify-packaging verify-bundle-isolation --nxBail - name: 'Check for unstaged changes' run: | From 5efc0627d9ae9ef37ef86d5f298f3e83420f7a03 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 4 Aug 2026 23:35:59 +0200 Subject: [PATCH 04/14] refactor(react-headless-components-preview): verify bundle isolation with webpack Replaces the esbuild check with a webpack plugin so the verdict comes from the same bundler that produces the bundle-size numbers. Failures now name the exports that survived tree shaking and the modules importing them, instead of only listing retained module paths. Attribution intersects usedExports with active import connections and requires the importing module to survive into a chunk, so packages whose icon imports were eliminated are no longer blamed. Adds --analyze to emit a webpack-bundle-analyzer treemap per fixture. --- .../library/project.json | 5 +- .../scripts/verify-bundle-isolation/README.md | 27 +- .../bundle-isolation-plugin.js | 226 +++++++++++++++ .../scripts/verify-bundle-isolation/cli.js | 267 ++++++++---------- 4 files changed, 375 insertions(+), 150 deletions(-) create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js diff --git a/packages/react-components/react-headless-components-preview/library/project.json b/packages/react-components/react-headless-components-preview/library/project.json index 085163037e6f0c..5a55e2aea8d3d1 100644 --- a/packages/react-components/react-headless-components-preview/library/project.json +++ b/packages/react-components/react-headless-components-preview/library/project.json @@ -24,10 +24,11 @@ "{projectRoot}/scripts/verify-bundle-isolation/schema.json", "{projectRoot}/bundle-size/**/*", "{projectRoot}/package.json", - { "externalDependencies": ["ajv", "esbuild"] } + { "externalDependencies": ["ajv", "webpack"] } ], + "outputs": ["{projectRoot}/dist/bundle-isolation"], "metadata": { - "technologies": ["esbuild"], + "technologies": ["webpack"], "description": "Assert entry points do not bundle tabster, Griffel or react-icons" } } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md index 030684a069a602..f488638e8fd26b 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md @@ -2,7 +2,26 @@ Verifies that headless package bundle-size fixtures do not retain browser runtimes that the headless API must avoid. -The check bundles each `*.fixture.js` file with esbuild and inspects the retained module graph. It fails when a configured forbidden package survives tree shaking, when bundling resolves to package source instead of built output, or when a known violation becomes stale. +The check bundles each `*.fixture.js` file with webpack โ€” the same bundler that produces the bundle-size numbers โ€” and inspects the resulting module graph. It fails when a configured forbidden package survives tree shaking, when bundling resolves to package source instead of built output, or when a known violation becomes stale. + +On failure it names the exact exports that kept the package alive and the modules importing them: + +``` +AllComponents.fixture.js pulls in forbidden runtime: + @fluentui/react-icons - 9 modules retained + ChevronDownRegular <- packages/.../react-tag-picker/library/lib/components/TagPickerControl/useTagPickerControl.js + DismissRegular <- packages/.../react-teaching-popover/library/lib/components/TeachingPopoverTitle/useTeachingPopoverTitle.js +``` + +Attribution intersects webpack's `usedExports` with active import connections, and counts an importer only when that module survived into a chunk. Import edges alone are recorded before tree shaking, so a package importing an unused icon is not reported as a leak. + +The analysis lives in [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) as a standard webpack plugin, so it can also be applied to an existing build instead of the one the CLI creates: + +```js +new BundleIsolationPlugin({ forbiddenPackages, workspaceRoot, onReport }); +``` + +It requires `optimization.concatenateModules: false` โ€” scope hoisting merges modules into a `ConcatenatedModule` with no per-module `resource`, which hides the packages being looked for. ## Usage @@ -18,6 +37,12 @@ Use a different package-root-relative configuration file with: node scripts/verify-bundle-isolation/cli.js --config ./bundle-isolation.config.json ``` +Emit a webpack-bundle-analyzer treemap per fixture into `dist/bundle-isolation/` with: + +```sh +node scripts/verify-bundle-isolation/cli.js --analyze +``` + The Nx target builds dependencies and runs the check with the correct working directory: ```sh diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js new file mode 100644 index 00000000000000..b1f0c74504390a --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js @@ -0,0 +1,226 @@ +// @ts-check +/** + * Reports which forbidden packages survived tree shaking, the exports keeping them alive and the + * modules importing those exports. + * + * Written as a plugin so the analysis can run inside any webpack build - a purpose-built bundle + * like the one the CLI creates, or an existing one such as the monosize bundle-size build. + * + * Requires `optimization.concatenateModules: false`; scope hoisting merges modules into a + * `ConcatenatedModule` with no per-module `resource`, which hides the packages being looked for. + */ +const { existsSync, readFileSync } = require('node:fs'); +const { dirname, isAbsolute, join } = require('node:path'); + +/** @typedef {{modules: number, exports: Array<{name: string, importers: string[]}>}} Leak */ +/** @typedef {{leaks: Record, sourceResolved: string[]}} BundleIsolationReport */ +/** @typedef {{forbiddenPackages: string[], workspaceRoot: string}} AnalysisOptions */ + +/** `ExportInfo.getUsed()` returns this when nothing references the export. */ +const UNUSED = 0; + +const PLUGIN_NAME = 'BundleIsolationPlugin'; + +class BundleIsolationPlugin { + /** @param {AnalysisOptions & {onReport: (report: BundleIsolationReport) => void}} options */ + constructor(options) { + this.options = options; + } + + /** @param {import('webpack').Compiler} compiler */ + apply(compiler) { + compiler.hooks.afterEmit.tap(PLUGIN_NAME, compilation => { + this.options.onReport(collectLeaks(compilation, this.options)); + }); + } +} + +/** + * webpack records import edges for modules whose imports were later eliminated, so edges alone + * over-report. A package counts as leaked only when its modules are in a chunk, an export is + * reported as used, and the importing module survived as well. + * + * @param {import('webpack').Compilation} compilation + * @param {AnalysisOptions} options + * @returns {BundleIsolationReport} + */ +function collectLeaks(compilation, options) { + const { chunkGraph, moduleGraph } = compilation; + const ownerOf = createForbiddenOwnerResolver(options); + /** @type {Record}>}>} */ + const collected = {}; + /** @type {string[]} */ + const sourceResolved = []; + + for (const module of compilation.modules) { + const resource = resourceOf(module); + if (!resource || chunkGraph.getNumberOfModuleChunks(module) === 0) { + continue; + } + + if (/[/\\]library[/\\]src[/\\]/.test(resource)) { + sourceResolved.push(resource); + } + + const owner = ownerOf(resource); + if (!owner) { + continue; + } + + const leak = (collected[owner] ??= { modules: 0, exports: new Map() }); + leak.modules++; + + const [runtime] = chunkGraph.getModuleRuntimes(module); + + for (const name of usedExportNames(moduleGraph, runtime, module)) { + const importers = externalImporters(moduleGraph, chunkGraph, runtime, module, name, ownerOf); + // Exports only referenced inside the forbidden package are plumbing, not entry points. + if (importers.length === 0) { + continue; + } + // Keyed per module so two modules exporting the same name are not merged. + const key = `${name}\u0000${resource}`; + const known = leak.exports.get(key) ?? { name, importers: new Set() }; + importers.forEach(importer => known.importers.add(importer)); + leak.exports.set(key, known); + } + } + + /** @type {Record} */ + const leaks = {}; + for (const [name, leak] of Object.entries(collected)) { + leaks[name] = { + modules: leak.modules, + exports: [...leak.exports.values()] + .map(({ name: exportName, importers }) => ({ name: exportName, importers: [...importers].sort() })) + .sort((a, b) => a.name.localeCompare(b.name)), + }; + } + + return { leaks, sourceResolved }; +} + +/** + * @param {import('webpack').ModuleGraph} moduleGraph + * @param {import('webpack').RuntimeSpec} runtime + * @param {import('webpack').Module} module + * @returns {string[]} + */ +function usedExportNames(moduleGraph, runtime, module) { + const names = []; + + for (const exportInfo of moduleGraph.getExportsInfo(module).orderedExports) { + if (exportInfo.getUsed(runtime) !== UNUSED) { + names.push(exportInfo.name); + } + } + + return names; +} + +/** + * @param {import('webpack').ModuleGraph} moduleGraph + * @param {import('webpack').ChunkGraph} chunkGraph + * @param {import('webpack').RuntimeSpec} runtime + * @param {import('webpack').Module} module + * @param {string} exportName + * @param {(modulePath: string) => string | null} ownerOf + * @returns {string[]} + */ +function externalImporters(moduleGraph, chunkGraph, runtime, module, exportName, ownerOf) { + /** @type {Set} */ + const importers = new Set(); + + for (const connection of moduleGraph.getIncomingConnections(module)) { + // An eliminated importer keeps an active connection, so its own retention decides. + if (!connection.originModule || chunkGraph.getNumberOfModuleChunks(connection.originModule) === 0) { + continue; + } + const origin = resourceOf(connection.originModule); + if (!origin || ownerOf(origin) || connection.getActiveState(runtime) === false) { + continue; + } + if (importedIds(connection.dependency, moduleGraph)[0] === exportName) { + importers.add(origin); + } + } + + return [...importers]; +} + +/** + * @param {unknown} dependency + * @param {import('webpack').ModuleGraph} moduleGraph + * @returns {string[]} + */ +function importedIds(dependency, moduleGraph) { + const candidate = /** @type {{getIds?: (graph: import('webpack').ModuleGraph) => string[], ids?: string[]}} */ ( + dependency + ); + + if (typeof candidate?.getIds === 'function') { + return candidate.getIds(moduleGraph) ?? []; + } + + return candidate?.ids ?? []; +} + +/** + * @param {import('webpack').Module} module + * @returns {string | null} + */ +function resourceOf(module) { + const candidate = /** @type {{resource?: string}} */ (/** @type {unknown} */ (module)); + return candidate.resource ?? module.nameForCondition() ?? null; +} + +/** + * Maps a module path to the forbidden package owning it, or `null`. + * + * Ownership is resolved by walking up to the nearest `package.json`, which handles both + * `node_modules` dependencies and workspace packages (webpack resolves symlinked workspace + * packages to their real path, so there is no `node_modules` segment to match on). + * + * @param {AnalysisOptions} options + */ +function createForbiddenOwnerResolver(options) { + const exact = new Set(options.forbiddenPackages.filter(pattern => !pattern.endsWith('/*'))); + const scopes = options.forbiddenPackages + .filter(pattern => pattern.endsWith('/*')) + .map(pattern => pattern.slice(0, -1)); + /** @type {Map} */ + const cache = new Map(); + + /** @param {string} modulePath */ + return function ownerOf(modulePath) { + let dir = dirname(isAbsolute(modulePath) ? modulePath : join(options.workspaceRoot, modulePath)); + const visited = []; + + while (dir && dir !== dirname(dir)) { + if (cache.has(dir)) { + const cached = cache.get(dir) ?? null; + visited.forEach(seen => cache.set(seen, cached)); + return cached; + } + visited.push(dir); + + const manifest = join(dir, 'package.json'); + if (existsSync(manifest)) { + const { name } = JSON.parse(readFileSync(manifest, 'utf-8')); + // Nested manifests without a name (e.g. `{ "type": "module" }` markers) are not package roots. + if (name) { + const owner = exact.has(name) || scopes.some(scope => name.startsWith(scope)) ? name : null; + visited.forEach(seen => cache.set(seen, owner)); + return owner; + } + } + + dir = dirname(dir); + } + + visited.forEach(seen => cache.set(seen, null)); + return null; + }; +} + +module.exports = { BundleIsolationPlugin }; diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js index 72f943ed06e95a..969b003c9df9dc 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js @@ -3,20 +3,24 @@ * Asserts that no bundle-size fixture in this package bundles a runtime the headless API is * meant to stay free of. * - * Runs against the built `lib/` output (never `src/`) because tree shaking depends on - * `/*#__PURE__*\/` annotations that swc only adds at build time. + * Bundles with webpack so the verdict comes from the same bundler that produces the bundle-size + * numbers, and so `usedExports` can name the exact symbols that survived tree shaking. * - * Usage: node scripts/verify-bundle-isolation/cli.js + * Usage: node scripts/verify-bundle-isolation/cli.js [--config ] [--analyze] */ const { existsSync, readdirSync, readFileSync } = require('node:fs'); const { dirname, isAbsolute, join, resolve, sep } = require('node:path'); const { parseArgs } = require('node:util'); const Ajv = /** @type {typeof import('ajv').default} */ (/** @type {unknown} */ (require('ajv'))); -const esbuild = require('esbuild'); +const webpack = require('webpack'); + +const { BundleIsolationPlugin } = require('./bundle-isolation-plugin'); /** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], knownViolations: Record}} Config */ -/** @typedef {{configPath: string, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ +/** @typedef {{configPath: string, analyze: boolean, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ +/** @typedef {import('./bundle-isolation-plugin').Leak} Leak */ +/** @typedef {import('./bundle-isolation-plugin').BundleIsolationReport} BundleIsolationReport */ const schemaPath = join(__dirname, 'schema.json'); @@ -25,7 +29,7 @@ main(processArgs()).catch(error => { process.exit(1); }); -/** @param {{configPath: string}} options */ +/** @param {{configPath: string, analyze: boolean}} options */ async function main(options) { const packageRoot = dirname(options.configPath); const workspaceRoot = findWorkspaceRoot(packageRoot); @@ -45,6 +49,11 @@ async function main(options) { const failures = collectFailures(results, runtimeOptions); + if (options.analyze) { + const reports = join(packageRoot, 'dist', 'bundle-isolation'); + console.log(`Analyzer reports written to ${relativeToWorkspace(reports, workspaceRoot)}`); + } + if (failures.length === 0) { console.log( `OK ${packageJson.name}: ${fixtures.length} bundle-size fixtures free of ${config.forbiddenPackages.join(', ')}`, @@ -64,11 +73,15 @@ function processArgs() { type: 'string', default: 'bundle-isolation.config.json', }, + analyze: { + type: 'boolean', + default: false, + }, }, allowPositionals: false, }); - return { configPath: resolve(process.cwd(), values.config) }; + return { configPath: resolve(process.cwd(), values.config), analyze: values.analyze }; } /** @@ -76,157 +89,103 @@ function processArgs() { * @param {RuntimeOptions} options */ async function verifyFixture(fixture, options) { - /** @type {{fixture: string, found: string[], rootCauses: string[], chains: Record, sourceResolved: string[], error?: string}} */ - const result = { fixture, found: [], rootCauses: [], chains: {}, sourceResolved: [] }; + /** @type {{fixture: string, found: string[], leaks: Record, sourceResolved: string[], error?: string}} */ + const result = { fixture, found: [], leaks: {}, sourceResolved: [] }; + + let stats; + /** @type {BundleIsolationReport | undefined} */ + let report; - let metafile; try { - ({ metafile } = await esbuild.build({ - entryPoints: [join(options.fixturesRoot, fixture)], - bundle: true, - write: false, - metafile: true, - format: 'esm', - platform: 'browser', - external: options.config.externals, - absWorkingDir: options.workspaceRoot, - logLevel: 'silent', - // `tsconfig.base.json` maps every `@fluentui/*` specifier to `library/src/index.ts`, and esbuild - // honours those paths - which would verify sources instead of the published output. An inline - // empty config disables tsconfig discovery so resolution goes through `exports` only. - tsconfigRaw: {}, - conditions: ['import'], - })); + stats = await bundleFixture(fixture, options, value => { + report = value; + }); } catch (error) { result.error = error instanceof Error ? error.message : String(error); return result; } - const output = Object.values(metafile.outputs)[0]; - const kept = new Set(Object.keys(output.inputs)); - const entry = output.entryPoint; - const ownerOf = createForbiddenOwnerResolver(options); - - if (!entry) { - result.error = 'esbuild did not report an entry point for the fixture output'; + if (stats.hasErrors()) { + result.error = (stats.toJson({ all: false, errors: true }).errors ?? []).map(error => error.message).join('\n '); return result; } - result.sourceResolved = [...kept].filter(modulePath => /[/\\]library[/\\]src[/\\]/.test(modulePath)); - - // The output's module list is authoritative for *what* leaked. - result.found = [...new Set([...kept].map(ownerOf).filter(Boolean))].sort(); - - // Chains are best effort: prefer a path made only of retained modules, but esbuild records - // import edges from the pre-shaking graph, so a retained module's only recorded importer may - // itself have been eliminated. Fall back to the full graph so every leak still gets a chain. - const keptChains = traceForbiddenPackages(metafile, entry, ownerOf, modulePath => kept.has(modulePath)); - const allChains = result.found.every(name => name in keptChains) - ? keptChains - : traceForbiddenPackages(metafile, entry, ownerOf, () => true); - result.chains = Object.fromEntries(result.found.map(name => [name, keptChains[name] ?? allChains[name] ?? []])); + if (!report) { + result.error = 'the bundle isolation plugin did not report on this build'; + return result; + } - // A package reached *through* another forbidden package is a symptom, not a cause. - result.rootCauses = result.found.filter(name => - result.chains[name].slice(0, -1).every(step => { - const owner = ownerOf(step); - return owner === null || owner === name; - }), - ); + result.leaks = report.leaks; + result.sourceResolved = report.sourceResolved; + result.found = Object.keys(report.leaks).sort(); return result; } /** - * Breadth-first walk of the module graph, recording the shortest import chain from the - * entry to the first module of every forbidden package it reaches. - * - * One traversal (rather than a search per package) guarantees each reported chain is - * genuinely reachable and is the shortest one. - * - * @param {import('esbuild').Metafile} metafile - * @param {string} entry - * @param {(modulePath: string) => string | null} ownerOf - * @param {(modulePath: string) => boolean} allowEdge - * @returns {Record} + * @param {string} fixture + * @param {RuntimeOptions} options + * @param {(report: BundleIsolationReport) => void} onReport + * @returns {Promise} */ -function traceForbiddenPackages(metafile, entry, ownerOf, allowEdge) { - /** @type {Map} */ - const previous = new Map([[entry, null]]); - const queue = [entry]; - /** @type {Record} */ - const chains = {}; - - while (queue.length > 0) { - const current = /** @type {string} */ (queue.shift()); - const owner = current === entry ? null : ownerOf(current); - - if (owner && !(owner in chains)) { - const chain = []; - for (let node = /** @type {string | null} */ (current); node; node = previous.get(node) ?? null) { - chain.unshift(node); - } - chains[owner] = chain.slice(1); // drop the synthetic entry module - } - - for (const imported of metafile.inputs[current]?.imports ?? []) { - if (!allowEdge(imported.path) || previous.has(imported.path)) { - continue; - } - previous.set(imported.path, current); - queue.push(imported.path); - } - } - - return chains; +function bundleFixture(fixture, options, onReport) { + const compiler = webpack(createWebpackConfig(fixture, options, onReport)); + + return new Promise((resolveStats, rejectStats) => { + compiler.run((error, stats) => { + compiler.close(() => { + if (error || !stats) { + rejectStats(error ?? new Error('webpack finished without producing stats')); + return; + } + resolveStats(stats); + }); + }); + }); } /** - * Maps a module path to the forbidden package owning it, or `null`. - * - * Ownership is resolved by walking up to the nearest `package.json`, which handles both - * `node_modules` dependencies and workspace packages (esbuild resolves symlinked workspace - * packages to their real path, so there is no `node_modules` segment to match on). + * @param {string} fixture + * @param {RuntimeOptions} options + * @param {(report: BundleIsolationReport) => void} onReport + * @returns {import('webpack').Configuration} */ -/** @param {RuntimeOptions} options */ -function createForbiddenOwnerResolver(options) { - const exact = new Set(options.config.forbiddenPackages.filter(pattern => !pattern.endsWith('/*'))); - const scopes = options.config.forbiddenPackages - .filter(pattern => pattern.endsWith('/*')) - .map(pattern => pattern.slice(0, -1)); - /** @type {Map} */ - const cache = new Map(); - - /** @param {string} modulePath */ - return function ownerOf(modulePath) { - let dir = dirname(isAbsolute(modulePath) ? modulePath : join(options.workspaceRoot, modulePath)); - const visited = []; - - while (dir && dir !== dirname(dir)) { - if (cache.has(dir)) { - const cached = cache.get(dir) ?? null; - visited.forEach(seen => cache.set(seen, cached)); - return cached; - } - visited.push(dir); - - const manifest = join(dir, 'package.json'); - if (existsSync(manifest)) { - const name = readJson(manifest).name; - // Nested manifests without a name (e.g. `{ "type": "module" }` markers) are not package roots. - if (name) { - const owner = exact.has(name) || scopes.some(scope => name.startsWith(scope)) ? name : null; - visited.forEach(seen => cache.set(seen, owner)); - return owner; - } - } +function createWebpackConfig(fixture, options, onReport) { + const outputPath = join(options.packageRoot, 'dist', 'bundle-isolation', fixture.replace(/\.fixture\.js$/, '')); + + return { + name: 'bundle-isolation', + target: 'web', + mode: 'production', + context: options.workspaceRoot, + entry: join(options.fixturesRoot, fixture), + externals: Object.fromEntries(options.config.externals.map(name => [name, name])), + output: { path: outputPath, filename: 'index.js' }, + performance: { hints: false }, + // Scope hoisting and minification change how code is emitted, not which modules and exports + // survive tree shaking, so both stay off to keep the module graph 1:1 for attribution. + optimization: { concatenateModules: false, minimize: false }, + plugins: [ + new BundleIsolationPlugin({ + forbiddenPackages: options.config.forbiddenPackages, + workspaceRoot: options.workspaceRoot, + onReport, + }), + ...(options.analyze ? [createAnalyzerPlugin(outputPath)] : []), + ], + }; +} - dir = dirname(dir); - } +/** @param {string} outputPath */ +function createAnalyzerPlugin(outputPath) { + const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); - visited.forEach(seen => cache.set(seen, null)); - return null; - }; + return new BundleAnalyzerPlugin({ + analyzerMode: 'static', + reportFilename: join(outputPath, 'report.html'), + openAnalyzer: false, + logLevel: 'silent', + }); } /** @@ -255,18 +214,8 @@ function collectFailures(results, options) { const fixed = allowed.filter(name => !result.found.includes(name)); if (regressions.length > 0) { - const causes = regressions.filter(name => result.rootCauses.includes(name)); - const symptoms = regressions.filter(name => !causes.includes(name)); - const details = (causes.length > 0 ? causes : regressions) - .map(name => { - const chain = result.chains[name].map( - (step, index) => `${' '.repeat(index + 3)}|- ${relativeToWorkspace(step, options.workspaceRoot)}`, - ); - return ` ${name}\n${chain.join('\n')}`; - }) - .join('\n'); - const trailer = symptoms.length > 0 ? `\n ...which also pulls in ${symptoms.join(', ')}` : ''; - failures.push(`${result.fixture} pulls in forbidden runtime:\n${details}${trailer}`); + const details = regressions.map(name => describeLeak(name, result.leaks[name], options)).join('\n'); + failures.push(`${result.fixture} pulls in forbidden runtime:\n${details}`); } if (fixed.length > 0) { @@ -292,6 +241,30 @@ function collectFailures(results, options) { return failures; } +/** + * @param {string} name + * @param {Leak} leak + * @param {RuntimeOptions} options + */ +function describeLeak(name, leak, options) { + const header = ` ${name} - ${leak.modules} module${leak.modules === 1 ? '' : 's'} retained`; + + if (leak.exports.length === 0) { + return `${header}\n no importing symbol identified - rerun with --analyze to inspect the bundle`; + } + + const listed = leak.exports.slice(0, 5).map(({ name: exportName, importers }) => { + const shown = importers.slice(0, 2).map(importer => relativeToWorkspace(importer, options.workspaceRoot)); + const hidden = importers.length - shown.length; + return ` ${exportName} <- ${shown.join(', ')}${hidden > 0 ? ` (+${hidden} more)` : ''}`; + }); + const rest = leak.exports.length - listed.length; + + return `${header}\n${listed.join('\n')}${ + rest > 0 ? `\n ...and ${rest} more export${rest === 1 ? '' : 's'}` : '' + }`; +} + /** @param {string} root */ function findFixtures(root) { if (!existsSync(root)) { From 2d26783ce504392a0cfeaaeb342ad98dd62d22f4 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 4 Aug 2026 23:41:45 +0200 Subject: [PATCH 05/14] docs(react-headless-components-preview): tighten bundle isolation readme Describes the check in package-agnostic terms, drops the Nx target reference since the CLI runs standalone, and collapses the flag examples into a single table. --- .../scripts/verify-bundle-isolation/README.md | 63 ++++++++----------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md index f488638e8fd26b..e84d6102a9820d 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md @@ -1,53 +1,34 @@ # Bundle isolation verification -Verifies that headless package bundle-size fixtures do not retain browser runtimes that the headless API must avoid. +Fails when a bundle-size fixture retains a runtime the package is meant to stay free of, such as a styling engine or icon set that should have been tree shaken away. -The check bundles each `*.fixture.js` file with webpack โ€” the same bundler that produces the bundle-size numbers โ€” and inspects the resulting module graph. It fails when a configured forbidden package survives tree shaking, when bundling resolves to package source instead of built output, or when a known violation becomes stale. +## How it works -On failure it names the exact exports that kept the package alive and the modules importing them: +Each `*.fixture.js` is bundled with webpack โ€” the same bundler behind the bundle-size numbers โ€” and the resulting module graph is inspected. The check fails when a forbidden package survives tree shaking, when bundling resolves to sources instead of built output, or when a baseline entry is no longer reachable. + +Failures name the exports that kept the package alive and the modules importing them: ``` AllComponents.fixture.js pulls in forbidden runtime: @fluentui/react-icons - 9 modules retained - ChevronDownRegular <- packages/.../react-tag-picker/library/lib/components/TagPickerControl/useTagPickerControl.js - DismissRegular <- packages/.../react-teaching-popover/library/lib/components/TeachingPopoverTitle/useTeachingPopoverTitle.js -``` - -Attribution intersects webpack's `usedExports` with active import connections, and counts an importer only when that module survived into a chunk. Import edges alone are recorded before tree shaking, so a package importing an unused icon is not reported as a leak. - -The analysis lives in [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) as a standard webpack plugin, so it can also be applied to an existing build instead of the one the CLI creates: - -```js -new BundleIsolationPlugin({ forbiddenPackages, workspaceRoot, onReport }); + ChevronDownRegular <- .../react-tag-picker/lib/components/TagPickerControl/useTagPickerControl.js + DismissRegular <- .../react-teaching-popover/lib/components/TeachingPopoverTitle/useTeachingPopoverTitle.js ``` -It requires `optimization.concatenateModules: false` โ€” scope hoisting merges modules into a `ConcatenatedModule` with no per-module `resource`, which hides the packages being looked for. +Attribution intersects webpack's `usedExports` with active import connections, and counts an importer only when that module itself survived into a chunk. Import edges are recorded before tree shaking, so a module importing something it no longer uses is not reported. ## Usage -Run from the package root after building the package and its dependencies: +Run from the package root, once the package and its dependencies are built: ```sh node scripts/verify-bundle-isolation/cli.js ``` -Use a different package-root-relative configuration file with: - -```sh -node scripts/verify-bundle-isolation/cli.js --config ./bundle-isolation.config.json -``` - -Emit a webpack-bundle-analyzer treemap per fixture into `dist/bundle-isolation/` with: - -```sh -node scripts/verify-bundle-isolation/cli.js --analyze -``` - -The Nx target builds dependencies and runs the check with the correct working directory: - -```sh -yarn nx run react-headless-components-preview:verify-bundle-isolation -``` +| Flag | Default | Description | +| ----------------- | ------------------------------ | ------------------------------------------------------------------------------- | +| `--config ` | `bundle-isolation.config.json` | Configuration file, resolved from the working directory | +| `--analyze` | off | Write a webpack-bundle-analyzer treemap per fixture to `dist/bundle-isolation/` | ## Configuration @@ -77,16 +58,16 @@ All configured paths are resolved relative to the package root: Fixtures follow the existing Monosize convention in `bundle-size/*.fixture.js`. A fixture imports the public API under test and uses the import observably so tree shaking cannot discard it. ```js -import * as HeadlessButton from '@fluentui/react-headless-components-preview/button'; +import * as Button from '@scope/package/button'; -console.log(HeadlessButton); +console.log(Button); export default { - name: 'HeadlessButton', + name: 'Button', }; ``` -Using the same fixtures keeps bundle isolation and bundle-size measurements aligned. +Sharing fixtures keeps isolation checks and bundle-size measurements aligned. ## Known violations @@ -97,3 +78,13 @@ Using the same fixtures keeps bundle isolation and bundle-size measurements alig - A baseline entry for a missing fixture fails the check. This prevents fixed leaks from being silently reintroduced. + +## Reuse in another build + +The analysis lives in [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) as a standard webpack plugin, so it can run inside an existing build instead of the one the CLI creates: + +```js +new BundleIsolationPlugin({ forbiddenPackages, workspaceRoot, onReport }); +``` + +It requires `optimization.concatenateModules: false`, because scope hoisting merges modules into a `ConcatenatedModule` with no per-module `resource`. From 6ff6f3febc446e8b6617764386baa512a96d2345 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 4 Aug 2026 23:56:58 +0200 Subject: [PATCH 06/14] feat(react-headless-components-preview): trace bundle leaks back to the package Naming the module that imports a forbidden package is not actionable when that module belongs to a dependency: Griffel is reported against react-portal, which this package never imports directly. Each importer is now walked back over retained modules to the first module owned by the package under test, reported as 'via'. That surfaces lib/tag-picker.js as the origin, since it re-exports a render function that mounts a portal. The origin is omitted when the importer is already owned by the package. --- .../scripts/verify-bundle-isolation/README.md | 15 +++- .../bundle-isolation-plugin.js | 81 ++++++++++++++++--- .../scripts/verify-bundle-isolation/cli.js | 13 ++- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md index e84d6102a9820d..58d6c904de1e58 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md @@ -6,15 +6,20 @@ Fails when a bundle-size fixture retains a runtime the package is meant to stay Each `*.fixture.js` is bundled with webpack โ€” the same bundler behind the bundle-size numbers โ€” and the resulting module graph is inspected. The check fails when a forbidden package survives tree shaking, when bundling resolves to sources instead of built output, or when a baseline entry is no longer reachable. -Failures name the exports that kept the package alive and the modules importing them: +Failures name the exports that kept the package alive, the modules importing them, and the module in this package that pulled those modules in: ``` AllComponents.fixture.js pulls in forbidden runtime: @fluentui/react-icons - 9 modules retained - ChevronDownRegular <- .../react-tag-picker/lib/components/TagPickerControl/useTagPickerControl.js - DismissRegular <- .../react-teaching-popover/lib/components/TeachingPopoverTitle/useTeachingPopoverTitle.js + ChevronDownRegular + <- .../react-tag-picker/lib/components/TagPickerControl/useTagPickerControl.js (via lib/tag-picker.js) + @griffel/core - 11 modules retained + mergeClasses + <- .../react-portal/lib/components/Portal/usePortalMountNode.js (via lib/tag-picker.js) ``` +`via` matters when a leak arrives through a dependency: above, nothing imports `react-portal` directly - `lib/tag-picker.js` re-exports a render function that mounts a portal, which is what drags Griffel in. + Attribution intersects webpack's `usedExports` with active import connections, and counts an importer only when that module itself survived into a chunk. Import edges are recorded before tree shaking, so a module importing something it no longer uses is not reported. ## Usage @@ -84,7 +89,9 @@ This prevents fixed leaks from being silently reintroduced. The analysis lives in [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) as a standard webpack plugin, so it can run inside an existing build instead of the one the CLI creates: ```js -new BundleIsolationPlugin({ forbiddenPackages, workspaceRoot, onReport }); +new BundleIsolationPlugin({ forbiddenPackages, workspaceRoot, packageRoot, onReport }); ``` +`packageRoot` is optional and only powers the `via` origin. + It requires `optimization.concatenateModules: false`, because scope hoisting merges modules into a `ConcatenatedModule` with no per-module `resource`. diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js index b1f0c74504390a..1b82e5390adc6f 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js @@ -10,11 +10,12 @@ * `ConcatenatedModule` with no per-module `resource`, which hides the packages being looked for. */ const { existsSync, readFileSync } = require('node:fs'); -const { dirname, isAbsolute, join } = require('node:path'); +const { dirname, isAbsolute, join, relative, sep } = require('node:path'); -/** @typedef {{modules: number, exports: Array<{name: string, importers: string[]}>}} Leak */ +/** @typedef {{module: string, via: string | null}} Importer */ +/** @typedef {{modules: number, exports: Array<{name: string, importers: Importer[]}>}} Leak */ /** @typedef {{leaks: Record, sourceResolved: string[]}} BundleIsolationReport */ -/** @typedef {{forbiddenPackages: string[], workspaceRoot: string}} AnalysisOptions */ +/** @typedef {{forbiddenPackages: string[], workspaceRoot: string, packageRoot?: string}} AnalysisOptions */ /** `ExportInfo.getUsed()` returns this when nothing references the export. */ const UNUSED = 0; @@ -47,7 +48,7 @@ class BundleIsolationPlugin { function collectLeaks(compilation, options) { const { chunkGraph, moduleGraph } = compilation; const ownerOf = createForbiddenOwnerResolver(options); - /** @type {Record}>}>} */ + /** @type {Record}>}>} */ const collected = {}; /** @type {string[]} */ const sourceResolved = []; @@ -80,8 +81,14 @@ function collectLeaks(compilation, options) { } // Keyed per module so two modules exporting the same name are not merged. const key = `${name}\u0000${resource}`; - const known = leak.exports.get(key) ?? { name, importers: new Set() }; - importers.forEach(importer => known.importers.add(importer)); + const known = leak.exports.get(key) ?? { name, importers: new Map() }; + for (const importer of importers) { + const importerResource = /** @type {string} */ (resourceOf(importer)); + known.importers.set(importerResource, { + module: importerResource, + via: packageOriginOf(moduleGraph, chunkGraph, importer, options.packageRoot), + }); + } leak.exports.set(key, known); } } @@ -92,7 +99,10 @@ function collectLeaks(compilation, options) { leaks[name] = { modules: leak.modules, exports: [...leak.exports.values()] - .map(({ name: exportName, importers }) => ({ name: exportName, importers: [...importers].sort() })) + .map(({ name: exportName, importers }) => ({ + name: exportName, + importers: [...importers.values()].sort((a, b) => a.module.localeCompare(b.module)), + })) .sort((a, b) => a.name.localeCompare(b.name)), }; } @@ -125,11 +135,11 @@ function usedExportNames(moduleGraph, runtime, module) { * @param {import('webpack').Module} module * @param {string} exportName * @param {(modulePath: string) => string | null} ownerOf - * @returns {string[]} + * @returns {import('webpack').Module[]} */ function externalImporters(moduleGraph, chunkGraph, runtime, module, exportName, ownerOf) { - /** @type {Set} */ - const importers = new Set(); + /** @type {Map} */ + const importers = new Map(); for (const connection of moduleGraph.getIncomingConnections(module)) { // An eliminated importer keeps an active connection, so its own retention decides. @@ -141,11 +151,58 @@ function externalImporters(moduleGraph, chunkGraph, runtime, module, exportName, continue; } if (importedIds(connection.dependency, moduleGraph)[0] === exportName) { - importers.add(origin); + importers.set(origin, connection.originModule); + } + } + + return [...importers.values()]; +} + +/** + * Walks back over retained modules to the first one owned by the package under test, so a leak + * reached through a dependency points at the code that pulled that dependency in. + * + * @param {import('webpack').ModuleGraph} moduleGraph + * @param {import('webpack').ChunkGraph} chunkGraph + * @param {import('webpack').Module} module + * @param {string | undefined} packageRoot + * @returns {string | null} + */ +function packageOriginOf(moduleGraph, chunkGraph, module, packageRoot) { + if (!packageRoot) { + return null; + } + + /** @param {import('webpack').Module} candidate */ + const owned = candidate => { + const resource = resourceOf(candidate); + return Boolean(resource && resource.startsWith(packageRoot + sep)); + }; + + if (owned(module)) { + return null; + } + + const visited = new Set([module]); + const queue = [module]; + + while (queue.length > 0) { + const current = /** @type {import('webpack').Module} */ (queue.shift()); + + for (const connection of moduleGraph.getIncomingConnections(current)) { + const origin = connection.originModule; + if (!origin || visited.has(origin) || chunkGraph.getNumberOfModuleChunks(origin) === 0) { + continue; + } + visited.add(origin); + if (owned(origin)) { + return relative(packageRoot, /** @type {string} */ (resourceOf(origin))); + } + queue.push(origin); } } - return [...importers]; + return null; } /** diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js index 969b003c9df9dc..8ea8ef2b05f0dc 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js @@ -169,6 +169,7 @@ function createWebpackConfig(fixture, options, onReport) { new BundleIsolationPlugin({ forbiddenPackages: options.config.forbiddenPackages, workspaceRoot: options.workspaceRoot, + packageRoot: options.packageRoot, onReport, }), ...(options.analyze ? [createAnalyzerPlugin(outputPath)] : []), @@ -254,9 +255,15 @@ function describeLeak(name, leak, options) { } const listed = leak.exports.slice(0, 5).map(({ name: exportName, importers }) => { - const shown = importers.slice(0, 2).map(importer => relativeToWorkspace(importer, options.workspaceRoot)); - const hidden = importers.length - shown.length; - return ` ${exportName} <- ${shown.join(', ')}${hidden > 0 ? ` (+${hidden} more)` : ''}`; + const lines = importers.slice(0, 2).map(importer => { + const module = relativeToWorkspace(importer.module, options.workspaceRoot); + return ` <- ${module}${importer.via ? ` (via ${importer.via})` : ''}`; + }); + const hidden = importers.length - lines.length; + if (hidden > 0) { + lines.push(` <- +${hidden} more`); + } + return ` ${exportName}\n${lines.join('\n')}`; }); const rest = leak.exports.length - listed.length; From c2ca19c729175490a27eee984a03659e10434516 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 5 Aug 2026 10:26:31 +0200 Subject: [PATCH 07/14] feat(react-headless-components-preview): report bundle isolation debt honestly The check reported "N fixtures free of tabster, @griffel/*, @fluentui/react-icons" whenever nothing failed, which included the case where forbidden packages did survive bundling and were merely allowlisted. It claimed a bundle was clean while 26 modules across three forbidden packages were in it. Splits passing into two verdicts: PASS only when no forbidden package survived, PASS WITH DEBT when every survivor is allowlisted. The debt case lists each package with its module and export counts and the entry points dragging it in, so the cleanup work is visible from the console instead of only in the JSON. Adds --strict to reject allowlisted violations outright, for ratcheting. Stale and orphaned allowlist entries now report as their own findings rather than being folded into a generic failure, and a fixture can carry several findings at once. Counts come from an unminified build, so they measure retention rather than shipped bytes and are deliberately reported as modules, not kB. Also writes summary.json unconditionally - it is the cheap artifact CI wants - leaving --analyze to gate only the webpack-bundle-analyzer treemap and its underlying report.json. Renames knownViolations to allowedViolations, since the entries are tolerated debt rather than merely known. --- .../library/bundle-isolation.config.json | 2 +- .../scripts/verify-bundle-isolation/README.md | 48 +- .../scripts/verify-bundle-isolation/cli.js | 444 +++++++++++++++--- .../verify-bundle-isolation/schema.json | 6 +- 4 files changed, 415 insertions(+), 85 deletions(-) diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index b2e422f148a784..94674dcb7c47d3 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -3,7 +3,7 @@ "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], - "knownViolations": { + "allowedViolations": { "AllComponents.fixture.js": ["@fluentui/react-icons", "@griffel/core", "@griffel/react"] } } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md index 58d6c904de1e58..7e10d0b0dc0851 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md @@ -30,10 +30,36 @@ Run from the package root, once the package and its dependencies are built: node scripts/verify-bundle-isolation/cli.js ``` -| Flag | Default | Description | -| ----------------- | ------------------------------ | ------------------------------------------------------------------------------- | -| `--config ` | `bundle-isolation.config.json` | Configuration file, resolved from the working directory | -| `--analyze` | off | Write a webpack-bundle-analyzer treemap per fixture to `dist/bundle-isolation/` | +| Flag | Default | Description | +| ----------------- | ------------------------------ | -------------------------------------------------------------------- | +| `--config ` | `bundle-isolation.config.json` | Configuration file, resolved from the working directory | +| `--analyze` | off | Also write webpack-bundle-analyzer artifacts per fixture | +| `--strict` | off | Fail on allowed violations too, so the allowlist cannot be relied on | + +## Verdicts + +| Verdict | Exit | Meaning | +| ---------------- | ---- | ---------------------------------------------------------------------------------------------------------------------------------- | +| `PASS` | 0 | No forbidden package survived bundling. Only this verdict claims a bundle is free of them. | +| `PASS WITH DEBT` | 0 | Every surviving forbidden package is on the allowlist. The leaks are listed with their module counts and entry points. | +| `FAIL` | 1 | A regression, a stale or orphaned allowlist entry, a fixture that failed to bundle, or - under `--strict` - any allowed violation. | + +Per fixture the report labels each finding `CLEAN`, `ALLOWED`, `REGRESSION`, `STALE` or `ERROR`; a single fixture can +carry more than one label. Module and export counts come from a build with `minimize: false`, so they measure how much +of a package is retained, not what it costs to ship - use monosize for bytes. + +## Output + +`dist/bundle-isolation/` is wiped on every run, so it only ever contains the fixtures that currently exist. + +| Path | Written | Contents | +| ----------------------- | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `summary.json` | always | The console verdict in structured form - overall `status`, and per fixture its `status`, `allowedViolations`, `tolerated`, `regressions`, `stale` and full `leaks` map | +| `/report.html` | with `--analyze` | webpack-bundle-analyzer treemap | +| `/report.json` | with `--analyze` | The same data the treemap renders from - module tree with `statSize`, `parsedSize` and `gzipSize` | + +`leaks` maps a forbidden package to the exports that survived tree shaking and the modules importing them, so the +summary answers _what_ leaked and _why_, while the analyzer output answers _how much_ it costs. ## Configuration @@ -45,7 +71,7 @@ The default configuration is `bundle-isolation.config.json` in the package root. "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], - "knownViolations": { + "allowedViolations": { "AllComponents.fixture.js": ["@fluentui/react-icons"] } } @@ -56,7 +82,7 @@ All configured paths are resolved relative to the package root: - `fixturesRoot` is the directory containing bundle-size fixtures. - `externals` lists host-provided modules excluded from the bundle. - `forbiddenPackages` lists exact package names or scoped globs such as `@griffel/*`. -- `knownViolations` maps fixture paths, relative to `fixturesRoot`, to temporarily tolerated forbidden packages. +- `allowedViolations` maps fixture paths, relative to `fixturesRoot`, to tolerated forbidden packages. ## Fixtures @@ -74,15 +100,15 @@ export default { Sharing fixtures keeps isolation checks and bundle-size measurements aligned. -## Known violations +## Allowed violations -`knownViolations` is a shrink-only baseline: +`allowedViolations` is tracked debt, not an exemption. It is shrink-only: - A newly retained forbidden package fails the check. -- A package that no longer survives bundling also fails the check until its baseline entry is removed. -- A baseline entry for a missing fixture fails the check. +- A package that no longer survives bundling also fails the check until its entry is removed. +- An entry for a missing fixture fails the check. -This prevents fixed leaks from being silently reintroduced. +This prevents fixed leaks from being silently reintroduced. Deleting an entry is the goal; adding one is a regression. ## Reuse in another build diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js index 8ea8ef2b05f0dc..e66a70a09189c9 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js @@ -6,9 +6,9 @@ * Bundles with webpack so the verdict comes from the same bundler that produces the bundle-size * numbers, and so `usedExports` can name the exact symbols that survived tree shaking. * - * Usage: node scripts/verify-bundle-isolation/cli.js [--config ] [--analyze] + * Usage: node scripts/verify-bundle-isolation/cli.js [--config ] [--analyze] [--strict] */ -const { existsSync, readdirSync, readFileSync } = require('node:fs'); +const { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } = require('node:fs'); const { dirname, isAbsolute, join, resolve, sep } = require('node:path'); const { parseArgs } = require('node:util'); @@ -17,10 +17,12 @@ const webpack = require('webpack'); const { BundleIsolationPlugin } = require('./bundle-isolation-plugin'); -/** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], knownViolations: Record}} Config */ -/** @typedef {{configPath: string, analyze: boolean, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ +/** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], allowedViolations: Record}} Config */ +/** @typedef {{configPath: string, analyze: boolean, strict: boolean, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ /** @typedef {import('./bundle-isolation-plugin').Leak} Leak */ /** @typedef {import('./bundle-isolation-plugin').BundleIsolationReport} BundleIsolationReport */ +/** @typedef {{fixture: string, found: string[], leaks: Record, sourceResolved: string[], error?: string}} FixtureResult */ +/** @typedef {FixtureResult & {status: 'error' | 'regression' | 'stale' | 'allowed' | 'clean', allowed: string[], tolerated: string[], regressions: string[], stale: string[]}} Outcome */ const schemaPath = join(__dirname, 'schema.json'); @@ -29,7 +31,7 @@ main(processArgs()).catch(error => { process.exit(1); }); -/** @param {{configPath: string, analyze: boolean}} options */ +/** @param {{configPath: string, analyze: boolean, strict: boolean}} options */ async function main(options) { const packageRoot = dirname(options.configPath); const workspaceRoot = findWorkspaceRoot(packageRoot); @@ -45,25 +47,24 @@ async function main(options) { /** @type {RuntimeOptions} */ const runtimeOptions = { ...options, config, fixturesRoot, packageRoot, workspaceRoot }; + + // Fixtures come and go; a stale output directory would otherwise be mistaken for a fresh report. + rmSync(outputRoot(runtimeOptions), { recursive: true, force: true }); + const results = await Promise.all(fixtures.map(fixture => verifyFixture(fixture, runtimeOptions))); + const outcomes = results.map(result => classify(result, runtimeOptions)); + const orphans = orphanedAllowlistEntries(fixtures, runtimeOptions); + const failed = hasFailed(outcomes, orphans, runtimeOptions); - const failures = collectFailures(results, runtimeOptions); + const summaryPath = writeSummary(outcomes, orphans, packageJson.name, runtimeOptions); + const report = formatReport(outcomes, orphans, packageJson.name, runtimeOptions, summaryPath); - if (options.analyze) { - const reports = join(packageRoot, 'dist', 'bundle-isolation'); - console.log(`Analyzer reports written to ${relativeToWorkspace(reports, workspaceRoot)}`); - } + // One stream for the whole report - splitting it would let the shell interleave the verdict. + (failed ? console.error : console.log)(report); - if (failures.length === 0) { - console.log( - `OK ${packageJson.name}: ${fixtures.length} bundle-size fixtures free of ${config.forbiddenPackages.join(', ')}`, - ); - return; + if (failed) { + process.exit(1); } - - console.error(`Bundle isolation check failed for ${packageJson.name}:\n`); - failures.forEach(failure => console.error(` ${failure}\n`)); - process.exit(1); } function processArgs() { @@ -77,19 +78,24 @@ function processArgs() { type: 'boolean', default: false, }, + strict: { + type: 'boolean', + default: false, + }, }, allowPositionals: false, }); - return { configPath: resolve(process.cwd(), values.config), analyze: values.analyze }; + return { configPath: resolve(process.cwd(), values.config), analyze: values.analyze, strict: values.strict }; } /** * @param {string} fixture * @param {RuntimeOptions} options + * @returns {Promise} */ async function verifyFixture(fixture, options) { - /** @type {{fixture: string, found: string[], leaks: Record, sourceResolved: string[], error?: string}} */ + /** @type {FixtureResult} */ const result = { fixture, found: [], leaks: {}, sourceResolved: [] }; let stats; @@ -151,7 +157,7 @@ function bundleFixture(fixture, options, onReport) { * @returns {import('webpack').Configuration} */ function createWebpackConfig(fixture, options, onReport) { - const outputPath = join(options.packageRoot, 'dist', 'bundle-isolation', fixture.replace(/\.fixture\.js$/, '')); + const outputPath = fixtureOutputPath(fixture, options); return { name: 'bundle-isolation', @@ -172,74 +178,372 @@ function createWebpackConfig(fixture, options, onReport) { packageRoot: options.packageRoot, onReport, }), - ...(options.analyze ? [createAnalyzerPlugin(outputPath)] : []), + ...(options.analyze ? createAnalyzerPlugins(outputPath) : []), ], }; } -/** @param {string} outputPath */ -function createAnalyzerPlugin(outputPath) { +/** + * One instance per output format - `analyzerMode` is single valued, so the treemap and its + * underlying data need separate plugins. + * + * @param {string} outputPath + */ +function createAnalyzerPlugins(outputPath) { const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); - return new BundleAnalyzerPlugin({ - analyzerMode: 'static', - reportFilename: join(outputPath, 'report.html'), - openAnalyzer: false, - logLevel: 'silent', - }); + return [ + new BundleAnalyzerPlugin({ + analyzerMode: 'static', + reportFilename: join(outputPath, 'report.html'), + openAnalyzer: false, + logLevel: 'silent', + }), + new BundleAnalyzerPlugin({ + analyzerMode: 'json', + reportFilename: join(outputPath, 'report.json'), + logLevel: 'silent', + }), + ]; } /** - * @param {Array extends Promise ? T : never>} results + * @param {FixtureResult} result * @param {RuntimeOptions} options + * @returns {Outcome} */ -function collectFailures(results, options) { - const failures = []; +function classify(result, options) { + const allowed = options.config.allowedViolations[result.fixture] ?? []; + const regressions = result.found.filter(name => !allowed.includes(name)); + const stale = allowed.filter(name => !result.found.includes(name)); + const tolerated = allowed.filter(name => result.found.includes(name)); + + const status = + result.error || result.sourceResolved.length > 0 + ? 'error' + : regressions.length > 0 + ? 'regression' + : stale.length > 0 + ? 'stale' + : tolerated.length > 0 + ? 'allowed' + : 'clean'; + + return { ...result, status, allowed, tolerated, regressions, stale }; +} - for (const result of results) { - if (result.error) { - failures.push(`${result.fixture} could not be bundled - is the package built?\n ${result.error}`); - continue; - } +/** + * @param {string[]} fixtures + * @param {RuntimeOptions} options + */ +function orphanedAllowlistEntries(fixtures, options) { + return Object.entries(options.config.allowedViolations) + .filter(([fixture]) => !fixtures.includes(fixture)) + .map(([fixture, packages]) => ({ fixture, packages })); +} - if (result.sourceResolved.length > 0) { - failures.push( - `${result.fixture} resolved to package sources instead of built output, so the result is meaningless.\n` + - ` e.g. ${result.sourceResolved[0]}`, - ); - continue; - } +/** + * @param {Outcome[]} outcomes + * @param {ReturnType} orphans + * @param {RuntimeOptions} options + */ +function hasFailed(outcomes, orphans, options) { + return ( + orphans.length > 0 || + outcomes.some( + outcome => + outcome.status === 'error' || + outcome.regressions.length > 0 || + outcome.stale.length > 0 || + (options.strict && outcome.tolerated.length > 0), + ) + ); +} - const allowed = options.config.knownViolations[result.fixture] ?? []; - const regressions = result.found.filter(name => !allowed.includes(name)); - const fixed = allowed.filter(name => !result.found.includes(name)); +/** + * @param {Outcome[]} outcomes + * @param {ReturnType} orphans + * @param {string} packageName + * @param {RuntimeOptions} options + * @param {string} summaryPath + */ +function formatReport(outcomes, orphans, packageName, options, summaryPath) { + const lines = [`Bundle isolation ยท ${packageName}`, `forbidden: ${options.config.forbiddenPackages.join(', ')}`, '']; - if (regressions.length > 0) { - const details = regressions.map(name => describeLeak(name, result.leaks[name], options)).join('\n'); - failures.push(`${result.fixture} pulls in forbidden runtime:\n${details}`); - } + for (const outcome of outcomes) { + lines.push(...formatFixture(outcome, options), ''); + } - if (fixed.length > 0) { - failures.push( - `${result.fixture} no longer pulls in ${fixed.join(', ')} - ` + - `remove it from config.knownViolations in ${relativeToWorkspace( - options.configPath, - options.workspaceRoot, - )} to lock the fix in.`, - ); - } + for (const orphan of orphans) { + lines.push( + ` ORPHAN ${orphan.fixture} - allowlisted (${orphan.packages.join(', ')}) but not a bundle-size fixture`, + ` remove the entry from allowedViolations in ${configPathLabel(options)}`, + '', + ); } - const verified = new Set(results.map(result => result.fixture)); - for (const [fixture, packages] of Object.entries(options.config.knownViolations)) { - if (!verified.has(fixture)) { - failures.push( - `config.knownViolations lists "${fixture}" (${packages.join(', ')}) which is not a bundle-size fixture.`, - ); - } + lines.push(...formatVerdict(outcomes, orphans, options), '', ...formatArtifacts(options, summaryPath)); + + return lines.join('\n'); +} + +/** + * @param {Outcome} outcome + * @param {RuntimeOptions} options + */ +function formatFixture(outcome, options) { + if (outcome.status === 'error') { + return [` ERROR ${outcome.fixture}`, ...formatError(outcome, options)]; + } + + if (outcome.status === 'clean') { + return [` CLEAN ${outcome.fixture}`]; + } + + const lines = []; + + if (outcome.regressions.length > 0) { + lines.push( + ` REGRESSION ${outcome.fixture} - ${count( + outcome.regressions.length, + 'forbidden package', + )} not on the allowlist`, + ...outcome.regressions.map(name => describeLeak(name, outcome.leaks[name], options)), + ); + } + + if (outcome.stale.length > 0) { + lines.push( + ` STALE ${outcome.fixture} - no longer pulls in ${outcome.stale.join(', ')}`, + ` remove it from allowedViolations in ${configPathLabel(options)} to lock the fix in`, + ); } - return failures; + if (outcome.tolerated.length > 0) { + const modules = outcome.tolerated.reduce((total, name) => total + outcome.leaks[name].modules, 0); + lines.push( + ` ALLOWED ${outcome.fixture} - ${count(outcome.tolerated.length, 'forbidden package')}, ${count( + modules, + 'module', + )}`, + ...formatTolerated(outcome, options), + ); + } + + return lines; +} + +/** + * @param {Outcome} outcome + * @param {RuntimeOptions} options + */ +function formatError(outcome, options) { + if (outcome.error) { + return [ + ` could not be bundled - is the package built?`, + ...outcome.error.split('\n').map(line => ` ${line.trim()}`), + ]; + } + + return [ + ` resolved to package sources instead of built output, so the result is meaningless`, + ` e.g. ${relativeToWorkspace(outcome.sourceResolved[0], options.workspaceRoot)}`, + ]; +} + +/** + * Ordered by module count so the most expensive debt to pay down is listed first. + * + * @param {Outcome} outcome + * @param {RuntimeOptions} options + */ +function formatTolerated(outcome, options) { + const rows = outcome.tolerated + .map(name => ({ name, leak: outcome.leaks[name] })) + .sort((a, b) => b.leak.modules - a.leak.modules || a.name.localeCompare(b.name)); + + const nameWidth = Math.max(...rows.map(row => row.name.length)); + const moduleWidth = Math.max(...rows.map(row => count(row.leak.modules, 'module').length)); + + return rows.flatMap(({ name, leak }) => [ + ` ${name.padEnd(nameWidth)} ${count(leak.modules, 'module').padStart(moduleWidth)} ${count( + leak.exports.length, + 'export', + )}`, + ...originsOf(leak, options).map(origin => ` via ${origin}`), + ]); +} + +/** + * @param {Leak} leak + * @param {RuntimeOptions} options + */ +function originsOf(leak, options) { + const origins = new Set( + leak.exports.flatMap(({ importers }) => + importers.map(importer => importer.via ?? relativeToWorkspace(importer.module, options.workspaceRoot)), + ), + ); + const listed = [...origins].sort().slice(0, 3); + const hidden = origins.size - listed.length; + + return hidden > 0 ? [...listed, `+${count(hidden, 'more entry point')}`] : listed; +} + +/** + * @param {Outcome[]} outcomes + * @param {ReturnType} orphans + * @param {RuntimeOptions} options + */ +function formatVerdict(outcomes, orphans, options) { + const totals = { + errors: outcomes.filter(outcome => outcome.status === 'error').length, + regressions: outcomes.reduce((total, outcome) => total + outcome.regressions.length, 0), + stale: outcomes.reduce((total, outcome) => total + outcome.stale.length, 0), + tolerated: outcomes.reduce((total, outcome) => total + outcome.tolerated.length, 0), + }; + const fixtures = count(outcomes.length, 'fixture'); + + if (hasFailed(outcomes, orphans, options)) { + const parts = [ + totals.errors > 0 && count(totals.errors, 'fixture') + ' failed to bundle', + totals.regressions > 0 && count(totals.regressions, 'regression'), + totals.stale > 0 && count(totals.stale, 'stale allowlist entry', 'stale allowlist entries'), + orphans.length > 0 && count(orphans.length, 'orphaned allowlist entry', 'orphaned allowlist entries'), + options.strict && totals.tolerated > 0 && count(totals.tolerated, 'allowed violation') + ' rejected by --strict', + ].filter(Boolean); + + return [`FAIL - ${fixtures}: ${parts.join(', ')}`]; + } + + if (totals.tolerated === 0) { + return [`PASS - ${fixtures} free of ${options.config.forbiddenPackages.join(', ')}`]; + } + + const leaked = [...new Set(outcomes.flatMap(outcome => outcome.tolerated))].sort(); + const keptOut = options.config.forbiddenPackages.filter( + pattern => !leaked.some(name => matchesPackagePattern(pattern, name)), + ); + + return [ + `PASS WITH DEBT - ${fixtures}, 0 regressions, ${count(totals.tolerated, 'allowed violation')}`, + ...(keptOut.length > 0 ? [` kept out: ${keptOut.join(', ')}`] : []), + ` allowlist: ${leaked.join(', ')}`, + ` tracked in ${configPathLabel(options)} - deleting an entry is the goal, adding one is a regression`, + ]; +} + +/** + * @param {RuntimeOptions} options + * @param {string} summaryPath + */ +function formatArtifacts(options, summaryPath) { + const lines = [`summary: ${relativeToWorkspace(summaryPath, options.workspaceRoot)}`]; + + if (options.analyze) { + lines.push( + `analyzer: ${relativeToWorkspace( + outputRoot(options), + options.workspaceRoot, + )}//report.html + report.json`, + ); + } else { + lines.push(`analyzer: rerun with --analyze for per-fixture treemaps`); + } + + return lines; +} + +/** + * @param {string} pattern + * @param {string} name + */ +function matchesPackagePattern(pattern, name) { + return pattern.endsWith('/*') ? name.startsWith(pattern.slice(0, -1)) : name === pattern; +} + +/** + * @param {number} value + * @param {string} singular + * @param {string} [plural] + */ +function count(value, singular, plural) { + return `${value} ${value === 1 ? singular : plural ?? `${singular}s`}`; +} + +/** @param {RuntimeOptions} options */ +function configPathLabel(options) { + return relativeToWorkspace(options.configPath, options.workspaceRoot); +} + +/** + * Companion to the analyzer treemap: the same verdict as the console output, but structured so it + * can be diffed between runs or handed to another tool. Always written - it is the cheap artifact. + * + * @param {Outcome[]} outcomes + * @param {ReturnType} orphans + * @param {string} packageName + * @param {RuntimeOptions} options + */ +function writeSummary(outcomes, orphans, packageName, options) { + const toWorkspacePath = (/** @type {string} */ path) => relativeToWorkspace(path, options.workspaceRoot); + + const summary = { + package: packageName, + config: toWorkspacePath(options.configPath), + strict: options.strict, + status: hasFailed(outcomes, orphans, options) + ? 'failed' + : outcomes.some(outcome => outcome.tolerated.length > 0) + ? 'passed-with-debt' + : 'passed', + forbiddenPackages: options.config.forbiddenPackages, + orphanedAllowlistEntries: orphans, + fixtures: outcomes.map(outcome => ({ + fixture: outcome.fixture, + status: outcome.status, + analyzerReport: options.analyze + ? toWorkspacePath(join(fixtureOutputPath(outcome.fixture, options), 'report.json')) + : null, + error: outcome.error ?? null, + sourceResolved: outcome.sourceResolved.map(toWorkspacePath), + allowedViolations: outcome.allowed, + tolerated: outcome.tolerated, + regressions: outcome.regressions, + stale: outcome.stale, + leaks: Object.fromEntries( + Object.entries(outcome.leaks).map(([name, leak]) => [ + name, + { + modules: leak.modules, + exports: leak.exports.map(({ name: exportName, importers }) => ({ + name: exportName, + importers: importers.map(importer => ({ module: toWorkspacePath(importer.module), via: importer.via })), + })), + }, + ]), + ), + })), + }; + + const summaryPath = join(outputRoot(options), 'summary.json'); + mkdirSync(dirname(summaryPath), { recursive: true }); + writeFileSync(summaryPath, JSON.stringify(summary, null, 2) + '\n'); + + return summaryPath; +} + +/** @param {Pick} options */ +function outputRoot(options) { + return join(options.packageRoot, 'dist', 'bundle-isolation'); +} + +/** + * @param {string} fixture + * @param {Pick} options + */ +function fixtureOutputPath(fixture, options) { + return join(outputRoot(options), fixture.replace(/\.fixture\.js$/, '')); } /** diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json index 9d168893578fb3..422e6b2583bc4c 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json @@ -32,8 +32,8 @@ "minLength": 1 } }, - "knownViolations": { - "description": "Bundle-size fixture paths mapped to forbidden packages tolerated temporarily.", + "allowedViolations": { + "description": "Bundle-size fixture paths mapped to forbidden packages tolerated as tracked debt.", "type": "object", "additionalProperties": { "type": "array", @@ -45,5 +45,5 @@ } } }, - "required": ["fixturesRoot", "externals", "forbiddenPackages", "knownViolations"] + "required": ["fixturesRoot", "externals", "forbiddenPackages", "allowedViolations"] } From 00c97ea2d2b28d7474c7ba46bb55c3e20d9b5816 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 5 Aug 2026 10:59:34 +0200 Subject: [PATCH 08/14] refactor(react-headless-components-preview): split and test the bundle isolation check The check was a single 500 line file whose logic could only be exercised by running webpack against the real package, so the two false-attribution bugs found while building it were caught by hand rather than by a test. Splits it along the seam that matters: config.js owns loading and path conventions, report.js turns raw results into a verdict and renders it, and cli.js does argument parsing and the webpack run. report.js touches neither webpack nor the file system, so the verdict is testable directly - including the regression test for the claim that a bundle is "free of" packages that are merely allowlisted. Adds 49 tests. bundle-isolation-plugin.spec.js bundles a purpose-built module graph and asserts the attribution rules that were previously only verified by inspection: an eliminated importer is not blamed, only used exports are named, and a leak arriving through a dependency is traced back to the importing module. Two things the fixture had to account for - webpack inlines constant exports and drops the module, and macOS temp paths are symlinks that webpack reports resolved. Also folds the manual pluralisation, badge padding and repeated path shortening into shared helpers, renames single-letter sort parameters, and widens the Nx target inputs to the whole script directory - bundle-isolation-plugin.js was missing, so edits to the attribution logic did not bust the cache. --- .../library/project.json | 3 +- .../scripts/verify-bundle-isolation/README.md | 12 + .../bundle-isolation-plugin.js | 4 +- .../bundle-isolation-plugin.spec.js | 182 +++++++ .../scripts/verify-bundle-isolation/cli.js | 510 ++---------------- .../scripts/verify-bundle-isolation/config.js | 100 ++++ .../verify-bundle-isolation/config.spec.js | 98 ++++ .../scripts/verify-bundle-isolation/report.js | 398 ++++++++++++++ .../verify-bundle-isolation/report.spec.js | 312 +++++++++++ 9 files changed, 1140 insertions(+), 479 deletions(-) create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js create mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js diff --git a/packages/react-components/react-headless-components-preview/library/project.json b/packages/react-components/react-headless-components-preview/library/project.json index 5a55e2aea8d3d1..b7e38080952135 100644 --- a/packages/react-components/react-headless-components-preview/library/project.json +++ b/packages/react-components/react-headless-components-preview/library/project.json @@ -19,9 +19,8 @@ "cwd": "{projectRoot}" }, "inputs": [ - "{projectRoot}/scripts/verify-bundle-isolation/cli.js", + "{projectRoot}/scripts/verify-bundle-isolation/**/*", "{projectRoot}/bundle-isolation.config.json", - "{projectRoot}/scripts/verify-bundle-isolation/schema.json", "{projectRoot}/bundle-size/**/*", "{projectRoot}/package.json", { "externalDependencies": ["ajv", "webpack"] } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md index 7e10d0b0dc0851..9750a764d9593c 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md @@ -110,6 +110,18 @@ Sharing fixtures keeps isolation checks and bundle-size measurements aligned. This prevents fixed leaks from being silently reintroduced. Deleting an entry is the goal; adding one is a regression. +## Layout + +| File | Responsibility | +| ------------------------------------------------------------ | ------------------------------------------------------------------------------------- | +| [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) | The analysis - which forbidden packages survived, and why. A standard webpack plugin. | +| [`config.js`](./config.js) | Configuration loading, fixture discovery, path conventions | +| [`report.js`](./report.js) | Turns raw results into a verdict and renders it. No webpack, no file system. | +| [`cli.js`](./cli.js) | Argument parsing and the webpack run that feeds the above | + +Keeping `report.js` free of webpack and I/O is what makes the verdict testable without bundling anything; +`bundle-isolation-plugin.spec.js` covers attribution by bundling a purpose-built module graph. + ## Reuse in another build The analysis lives in [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) as a standard webpack plugin, so it can run inside an existing build instead of the one the CLI creates: diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js index 1b82e5390adc6f..f2b92a7fd1afbb 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js @@ -101,9 +101,9 @@ function collectLeaks(compilation, options) { exports: [...leak.exports.values()] .map(({ name: exportName, importers }) => ({ name: exportName, - importers: [...importers.values()].sort((a, b) => a.module.localeCompare(b.module)), + importers: [...importers.values()].sort((left, right) => left.module.localeCompare(right.module)), })) - .sort((a, b) => a.name.localeCompare(b.name)), + .sort((left, right) => left.name.localeCompare(right.name)), }; } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js new file mode 100644 index 00000000000000..5e610a57cefe91 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js @@ -0,0 +1,182 @@ +/* + * @jest-environment node + */ +// @ts-check +const { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } = require('node:fs'); +const { tmpdir } = require('node:os'); +const { dirname, join } = require('node:path'); + +const webpack = require('webpack'); + +const { BundleIsolationPlugin } = require('./bundle-isolation-plugin'); + +/** @typedef {import('./bundle-isolation-plugin').BundleIsolationReport} BundleIsolationReport */ + +jest.setTimeout(60_000); + +describe('BundleIsolationPlugin', () => { + /** @type {string} */ + let root; + + beforeEach(() => { + // webpack reports resolved real paths, which on macOS differ from the symlinked temp path. + root = realpathSync(mkdtempSync(join(tmpdir(), 'bundle-isolation-plugin-'))); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + describe('attribution', () => { + /** @type {BundleIsolationReport} */ + let report; + + beforeEach(async () => { + writeFiles(root, { + 'package.json': manifest('test-workspace'), + + // Exports are functions because webpack inlines constant exports and drops the module. + 'node_modules/forbidden-pkg/package.json': manifest('forbidden-pkg'), + 'node_modules/forbidden-pkg/index.js': `export const alpha = () => Date.now();\nexport const beta = () => Math.random();\n`, + + 'node_modules/@scope/styles/package.json': manifest('@scope/styles'), + 'node_modules/@scope/styles/index.js': `export const style = () => Date.now();\n`, + + // Reached only through the package under test, so it must be reported with a `via` origin. + 'node_modules/dep-pkg/package.json': manifest('dep-pkg'), + 'node_modules/dep-pkg/index.js': `import { alpha } from 'forbidden-pkg';\nexport const fromDep = () => alpha();\n`, + + // Imports the same forbidden package but is eliminated, so it must not be blamed. + 'node_modules/innocent-pkg/package.json': manifest('innocent-pkg'), + 'node_modules/innocent-pkg/index.js': `import { beta } from 'forbidden-pkg';\nexport const fromInnocent = () => beta();\n`, + + 'my-pkg/package.json': manifest('my-pkg'), + 'my-pkg/lib/live.js': `import { fromDep } from 'dep-pkg';\nexport const live = () => fromDep();\n`, + 'my-pkg/lib/direct.js': `import { style } from '@scope/styles';\nexport const direct = () => style();\n`, + 'my-pkg/lib/dead.js': `import { fromInnocent } from 'innocent-pkg';\nexport const dead = () => fromInnocent();\n`, + 'my-pkg/lib/index.js': `export * from './live';\nexport * from './direct';\nexport * from './dead';\n`, + + 'entry.js': `import { live, direct } from './my-pkg/lib/index.js';\nconsole.log(live(), direct());\n`, + }); + + report = await bundle({ + root, + packageRoot: join(root, 'my-pkg'), + forbiddenPackages: ['forbidden-pkg', '@scope/*'], + }); + }); + + it('reports forbidden packages that survived tree shaking', () => { + expect(Object.keys(report.leaks).sort()).toEqual(['@scope/styles', 'forbidden-pkg']); + }); + + it('does not blame an importer that was eliminated', () => { + const importers = report.leaks['forbidden-pkg'].exports.flatMap(({ importers: found }) => + found.map(importer => importer.module), + ); + + expect(importers).toEqual([join(root, 'node_modules/dep-pkg/index.js')]); + expect(importers.join()).not.toContain('innocent-pkg'); + }); + + it('names only the exports that are actually used', () => { + expect(report.leaks['forbidden-pkg'].exports.map(({ name }) => name)).toEqual(['alpha']); + }); + + it('traces a leak arriving through a dependency back to the importing module', () => { + expect(report.leaks['forbidden-pkg'].exports[0].importers[0].via).toBe(join('lib', 'live.js')); + }); + + it('reports no origin when the package under test imports the leak itself', () => { + expect(report.leaks['@scope/styles'].exports[0].importers[0].via).toBeNull(); + }); + + it('matches scoped globs', () => { + expect(report.leaks['@scope/styles'].modules).toBe(1); + }); + }); + + it('ignores a package that is not forbidden', async () => { + writeFiles(root, { + 'package.json': manifest('test-workspace'), + 'node_modules/allowed-pkg/package.json': manifest('allowed-pkg'), + 'node_modules/allowed-pkg/index.js': `export const value = () => Date.now();\n`, + 'my-pkg/package.json': manifest('my-pkg'), + 'my-pkg/lib/index.js': `import { value } from 'allowed-pkg';\nexport const use = () => value();\n`, + 'entry.js': `import { use } from './my-pkg/lib/index.js';\nconsole.log(use());\n`, + }); + + const report = await bundle({ root, packageRoot: join(root, 'my-pkg'), forbiddenPackages: ['forbidden-pkg'] }); + + expect(report.leaks).toEqual({}); + }); + + it('flags a bundle that resolved to package sources, since its verdict would be meaningless', async () => { + writeFiles(root, { + 'package.json': manifest('test-workspace'), + 'my-pkg/library/src/index.js': `export const fromSource = () => Date.now();\n`, + 'entry.js': `import { fromSource } from './my-pkg/library/src/index.js';\nconsole.log(fromSource());\n`, + }); + + const report = await bundle({ root, packageRoot: join(root, 'my-pkg'), forbiddenPackages: ['forbidden-pkg'] }); + + expect(report.sourceResolved).toEqual([join(root, 'my-pkg/library/src/index.js')]); + }); +}); + +/** + * @param {{root: string, packageRoot: string, forbiddenPackages: string[]}} options + * @returns {Promise} + */ +function bundle({ root, packageRoot, forbiddenPackages }) { + /** @type {BundleIsolationReport | undefined} */ + let report; + + const compiler = webpack({ + target: 'web', + mode: 'production', + context: root, + entry: join(root, 'entry.js'), + output: { path: join(root, 'out'), filename: 'index.js' }, + optimization: { concatenateModules: false, minimize: false }, + plugins: [ + new BundleIsolationPlugin({ + forbiddenPackages, + workspaceRoot: root, + packageRoot, + onReport: value => { + report = value; + }, + }), + ], + }); + + return new Promise((resolvePromise, rejectPromise) => { + compiler.run((error, stats) => { + compiler.close(() => { + if (error || stats?.hasErrors()) { + rejectPromise(error ?? new Error(stats?.toString({ errors: true }))); + return; + } + resolvePromise(/** @type {BundleIsolationReport} */ (report)); + }); + }); + }); +} + +/** + * @param {string} root + * @param {Record} files + */ +function writeFiles(root, files) { + for (const [path, contents] of Object.entries(files)) { + const target = join(root, path); + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, contents); + } +} + +/** @param {string} name */ +function manifest(name) { + return JSON.stringify({ name, version: '1.0.0', sideEffects: false }); +} diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js index e66a70a09189c9..a215acb95e3890 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js @@ -8,36 +8,33 @@ * * Usage: node scripts/verify-bundle-isolation/cli.js [--config ] [--analyze] [--strict] */ -const { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } = require('node:fs'); -const { dirname, isAbsolute, join, resolve, sep } = require('node:path'); +const { mkdirSync, rmSync, writeFileSync } = require('node:fs'); +const { dirname, join, resolve } = require('node:path'); const { parseArgs } = require('node:util'); -const Ajv = /** @type {typeof import('ajv').default} */ (/** @type {unknown} */ (require('ajv'))); const webpack = require('webpack'); const { BundleIsolationPlugin } = require('./bundle-isolation-plugin'); +const { findFixtures, findWorkspaceRoot, fixtureOutputPath, loadConfig, outputRoot, readJson } = require('./config'); +const { createReport, createSummary, formatReport } = require('./report'); -/** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], allowedViolations: Record}} Config */ -/** @typedef {{configPath: string, analyze: boolean, strict: boolean, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ -/** @typedef {import('./bundle-isolation-plugin').Leak} Leak */ +/** @typedef {import('./report').Report} Report */ +/** @typedef {import('./report').RuntimeOptions} RuntimeOptions */ +/** @typedef {import('./report').FixtureResult} FixtureResult */ /** @typedef {import('./bundle-isolation-plugin').BundleIsolationReport} BundleIsolationReport */ -/** @typedef {{fixture: string, found: string[], leaks: Record, sourceResolved: string[], error?: string}} FixtureResult */ -/** @typedef {FixtureResult & {status: 'error' | 'regression' | 'stale' | 'allowed' | 'clean', allowed: string[], tolerated: string[], regressions: string[], stale: string[]}} Outcome */ - -const schemaPath = join(__dirname, 'schema.json'); main(processArgs()).catch(error => { console.error(error); process.exit(1); }); -/** @param {{configPath: string, analyze: boolean, strict: boolean}} options */ -async function main(options) { - const packageRoot = dirname(options.configPath); +/** @param {{configPath: string, analyze: boolean, strict: boolean}} args */ +async function main(args) { + const packageRoot = dirname(args.configPath); const workspaceRoot = findWorkspaceRoot(packageRoot); - const config = loadConfig({ ...options, workspaceRoot }); - const fixturesRoot = resolve(packageRoot, config.fixturesRoot); + const config = loadConfig(args.configPath, workspaceRoot); const packageJson = readJson(join(packageRoot, 'package.json')); + const fixturesRoot = resolve(packageRoot, config.fixturesRoot); const fixtures = findFixtures(fixturesRoot); if (fixtures.length === 0) { @@ -46,23 +43,19 @@ async function main(options) { } /** @type {RuntimeOptions} */ - const runtimeOptions = { ...options, config, fixturesRoot, packageRoot, workspaceRoot }; + const options = { ...args, config, fixturesRoot, packageRoot, workspaceRoot }; // Fixtures come and go; a stale output directory would otherwise be mistaken for a fresh report. - rmSync(outputRoot(runtimeOptions), { recursive: true, force: true }); + rmSync(outputRoot(packageRoot), { recursive: true, force: true }); - const results = await Promise.all(fixtures.map(fixture => verifyFixture(fixture, runtimeOptions))); - const outcomes = results.map(result => classify(result, runtimeOptions)); - const orphans = orphanedAllowlistEntries(fixtures, runtimeOptions); - const failed = hasFailed(outcomes, orphans, runtimeOptions); - - const summaryPath = writeSummary(outcomes, orphans, packageJson.name, runtimeOptions); - const report = formatReport(outcomes, orphans, packageJson.name, runtimeOptions, summaryPath); + const results = await Promise.all(fixtures.map(fixture => verifyFixture(fixture, options))); + const report = createReport({ packageName: packageJson.name, results, fixtures, options }); + const summaryPath = writeSummary(report); // One stream for the whole report - splitting it would let the shell interleave the verdict. - (failed ? console.error : console.log)(report); + (report.failed ? console.error : console.log)(formatReport(report, summaryPath)); - if (failed) { + if (report.failed) { process.exit(1); } } @@ -70,18 +63,9 @@ async function main(options) { function processArgs() { const { values } = parseArgs({ options: { - config: { - type: 'string', - default: 'bundle-isolation.config.json', - }, - analyze: { - type: 'boolean', - default: false, - }, - strict: { - type: 'boolean', - default: false, - }, + config: { type: 'string', default: 'bundle-isolation.config.json' }, + analyze: { type: 'boolean', default: false }, + strict: { type: 'boolean', default: false }, }, allowPositionals: false, }); @@ -98,13 +82,13 @@ async function verifyFixture(fixture, options) { /** @type {FixtureResult} */ const result = { fixture, found: [], leaks: {}, sourceResolved: [] }; - let stats; /** @type {BundleIsolationReport | undefined} */ - let report; + let analysis; + let stats; try { - stats = await bundleFixture(fixture, options, value => { - report = value; + stats = await bundleFixture(fixture, options, report => { + analysis = report; }); } catch (error) { result.error = error instanceof Error ? error.message : String(error); @@ -116,14 +100,14 @@ async function verifyFixture(fixture, options) { return result; } - if (!report) { + if (!analysis) { result.error = 'the bundle isolation plugin did not report on this build'; return result; } - result.leaks = report.leaks; - result.sourceResolved = report.sourceResolved; - result.found = Object.keys(report.leaks).sort(); + result.leaks = analysis.leaks; + result.sourceResolved = analysis.sourceResolved; + result.found = Object.keys(analysis.leaks).sort(); return result; } @@ -157,7 +141,7 @@ function bundleFixture(fixture, options, onReport) { * @returns {import('webpack').Configuration} */ function createWebpackConfig(fixture, options, onReport) { - const outputPath = fixtureOutputPath(fixture, options); + const outputPath = fixtureOutputPath(fixture, options.packageRoot); return { name: 'bundle-isolation', @@ -207,436 +191,12 @@ function createAnalyzerPlugins(outputPath) { ]; } -/** - * @param {FixtureResult} result - * @param {RuntimeOptions} options - * @returns {Outcome} - */ -function classify(result, options) { - const allowed = options.config.allowedViolations[result.fixture] ?? []; - const regressions = result.found.filter(name => !allowed.includes(name)); - const stale = allowed.filter(name => !result.found.includes(name)); - const tolerated = allowed.filter(name => result.found.includes(name)); - - const status = - result.error || result.sourceResolved.length > 0 - ? 'error' - : regressions.length > 0 - ? 'regression' - : stale.length > 0 - ? 'stale' - : tolerated.length > 0 - ? 'allowed' - : 'clean'; - - return { ...result, status, allowed, tolerated, regressions, stale }; -} - -/** - * @param {string[]} fixtures - * @param {RuntimeOptions} options - */ -function orphanedAllowlistEntries(fixtures, options) { - return Object.entries(options.config.allowedViolations) - .filter(([fixture]) => !fixtures.includes(fixture)) - .map(([fixture, packages]) => ({ fixture, packages })); -} - -/** - * @param {Outcome[]} outcomes - * @param {ReturnType} orphans - * @param {RuntimeOptions} options - */ -function hasFailed(outcomes, orphans, options) { - return ( - orphans.length > 0 || - outcomes.some( - outcome => - outcome.status === 'error' || - outcome.regressions.length > 0 || - outcome.stale.length > 0 || - (options.strict && outcome.tolerated.length > 0), - ) - ); -} - -/** - * @param {Outcome[]} outcomes - * @param {ReturnType} orphans - * @param {string} packageName - * @param {RuntimeOptions} options - * @param {string} summaryPath - */ -function formatReport(outcomes, orphans, packageName, options, summaryPath) { - const lines = [`Bundle isolation ยท ${packageName}`, `forbidden: ${options.config.forbiddenPackages.join(', ')}`, '']; - - for (const outcome of outcomes) { - lines.push(...formatFixture(outcome, options), ''); - } - - for (const orphan of orphans) { - lines.push( - ` ORPHAN ${orphan.fixture} - allowlisted (${orphan.packages.join(', ')}) but not a bundle-size fixture`, - ` remove the entry from allowedViolations in ${configPathLabel(options)}`, - '', - ); - } - - lines.push(...formatVerdict(outcomes, orphans, options), '', ...formatArtifacts(options, summaryPath)); - - return lines.join('\n'); -} - -/** - * @param {Outcome} outcome - * @param {RuntimeOptions} options - */ -function formatFixture(outcome, options) { - if (outcome.status === 'error') { - return [` ERROR ${outcome.fixture}`, ...formatError(outcome, options)]; - } - - if (outcome.status === 'clean') { - return [` CLEAN ${outcome.fixture}`]; - } - - const lines = []; - - if (outcome.regressions.length > 0) { - lines.push( - ` REGRESSION ${outcome.fixture} - ${count( - outcome.regressions.length, - 'forbidden package', - )} not on the allowlist`, - ...outcome.regressions.map(name => describeLeak(name, outcome.leaks[name], options)), - ); - } - - if (outcome.stale.length > 0) { - lines.push( - ` STALE ${outcome.fixture} - no longer pulls in ${outcome.stale.join(', ')}`, - ` remove it from allowedViolations in ${configPathLabel(options)} to lock the fix in`, - ); - } - - if (outcome.tolerated.length > 0) { - const modules = outcome.tolerated.reduce((total, name) => total + outcome.leaks[name].modules, 0); - lines.push( - ` ALLOWED ${outcome.fixture} - ${count(outcome.tolerated.length, 'forbidden package')}, ${count( - modules, - 'module', - )}`, - ...formatTolerated(outcome, options), - ); - } - - return lines; -} - -/** - * @param {Outcome} outcome - * @param {RuntimeOptions} options - */ -function formatError(outcome, options) { - if (outcome.error) { - return [ - ` could not be bundled - is the package built?`, - ...outcome.error.split('\n').map(line => ` ${line.trim()}`), - ]; - } - - return [ - ` resolved to package sources instead of built output, so the result is meaningless`, - ` e.g. ${relativeToWorkspace(outcome.sourceResolved[0], options.workspaceRoot)}`, - ]; -} - -/** - * Ordered by module count so the most expensive debt to pay down is listed first. - * - * @param {Outcome} outcome - * @param {RuntimeOptions} options - */ -function formatTolerated(outcome, options) { - const rows = outcome.tolerated - .map(name => ({ name, leak: outcome.leaks[name] })) - .sort((a, b) => b.leak.modules - a.leak.modules || a.name.localeCompare(b.name)); - - const nameWidth = Math.max(...rows.map(row => row.name.length)); - const moduleWidth = Math.max(...rows.map(row => count(row.leak.modules, 'module').length)); - - return rows.flatMap(({ name, leak }) => [ - ` ${name.padEnd(nameWidth)} ${count(leak.modules, 'module').padStart(moduleWidth)} ${count( - leak.exports.length, - 'export', - )}`, - ...originsOf(leak, options).map(origin => ` via ${origin}`), - ]); -} - -/** - * @param {Leak} leak - * @param {RuntimeOptions} options - */ -function originsOf(leak, options) { - const origins = new Set( - leak.exports.flatMap(({ importers }) => - importers.map(importer => importer.via ?? relativeToWorkspace(importer.module, options.workspaceRoot)), - ), - ); - const listed = [...origins].sort().slice(0, 3); - const hidden = origins.size - listed.length; - - return hidden > 0 ? [...listed, `+${count(hidden, 'more entry point')}`] : listed; -} - -/** - * @param {Outcome[]} outcomes - * @param {ReturnType} orphans - * @param {RuntimeOptions} options - */ -function formatVerdict(outcomes, orphans, options) { - const totals = { - errors: outcomes.filter(outcome => outcome.status === 'error').length, - regressions: outcomes.reduce((total, outcome) => total + outcome.regressions.length, 0), - stale: outcomes.reduce((total, outcome) => total + outcome.stale.length, 0), - tolerated: outcomes.reduce((total, outcome) => total + outcome.tolerated.length, 0), - }; - const fixtures = count(outcomes.length, 'fixture'); - - if (hasFailed(outcomes, orphans, options)) { - const parts = [ - totals.errors > 0 && count(totals.errors, 'fixture') + ' failed to bundle', - totals.regressions > 0 && count(totals.regressions, 'regression'), - totals.stale > 0 && count(totals.stale, 'stale allowlist entry', 'stale allowlist entries'), - orphans.length > 0 && count(orphans.length, 'orphaned allowlist entry', 'orphaned allowlist entries'), - options.strict && totals.tolerated > 0 && count(totals.tolerated, 'allowed violation') + ' rejected by --strict', - ].filter(Boolean); - - return [`FAIL - ${fixtures}: ${parts.join(', ')}`]; - } - - if (totals.tolerated === 0) { - return [`PASS - ${fixtures} free of ${options.config.forbiddenPackages.join(', ')}`]; - } - - const leaked = [...new Set(outcomes.flatMap(outcome => outcome.tolerated))].sort(); - const keptOut = options.config.forbiddenPackages.filter( - pattern => !leaked.some(name => matchesPackagePattern(pattern, name)), - ); - - return [ - `PASS WITH DEBT - ${fixtures}, 0 regressions, ${count(totals.tolerated, 'allowed violation')}`, - ...(keptOut.length > 0 ? [` kept out: ${keptOut.join(', ')}`] : []), - ` allowlist: ${leaked.join(', ')}`, - ` tracked in ${configPathLabel(options)} - deleting an entry is the goal, adding one is a regression`, - ]; -} - -/** - * @param {RuntimeOptions} options - * @param {string} summaryPath - */ -function formatArtifacts(options, summaryPath) { - const lines = [`summary: ${relativeToWorkspace(summaryPath, options.workspaceRoot)}`]; - - if (options.analyze) { - lines.push( - `analyzer: ${relativeToWorkspace( - outputRoot(options), - options.workspaceRoot, - )}//report.html + report.json`, - ); - } else { - lines.push(`analyzer: rerun with --analyze for per-fixture treemaps`); - } - - return lines; -} - -/** - * @param {string} pattern - * @param {string} name - */ -function matchesPackagePattern(pattern, name) { - return pattern.endsWith('/*') ? name.startsWith(pattern.slice(0, -1)) : name === pattern; -} - -/** - * @param {number} value - * @param {string} singular - * @param {string} [plural] - */ -function count(value, singular, plural) { - return `${value} ${value === 1 ? singular : plural ?? `${singular}s`}`; -} +/** @param {Report} report */ +function writeSummary(report) { + const summaryPath = join(outputRoot(report.options.packageRoot), 'summary.json'); -/** @param {RuntimeOptions} options */ -function configPathLabel(options) { - return relativeToWorkspace(options.configPath, options.workspaceRoot); -} - -/** - * Companion to the analyzer treemap: the same verdict as the console output, but structured so it - * can be diffed between runs or handed to another tool. Always written - it is the cheap artifact. - * - * @param {Outcome[]} outcomes - * @param {ReturnType} orphans - * @param {string} packageName - * @param {RuntimeOptions} options - */ -function writeSummary(outcomes, orphans, packageName, options) { - const toWorkspacePath = (/** @type {string} */ path) => relativeToWorkspace(path, options.workspaceRoot); - - const summary = { - package: packageName, - config: toWorkspacePath(options.configPath), - strict: options.strict, - status: hasFailed(outcomes, orphans, options) - ? 'failed' - : outcomes.some(outcome => outcome.tolerated.length > 0) - ? 'passed-with-debt' - : 'passed', - forbiddenPackages: options.config.forbiddenPackages, - orphanedAllowlistEntries: orphans, - fixtures: outcomes.map(outcome => ({ - fixture: outcome.fixture, - status: outcome.status, - analyzerReport: options.analyze - ? toWorkspacePath(join(fixtureOutputPath(outcome.fixture, options), 'report.json')) - : null, - error: outcome.error ?? null, - sourceResolved: outcome.sourceResolved.map(toWorkspacePath), - allowedViolations: outcome.allowed, - tolerated: outcome.tolerated, - regressions: outcome.regressions, - stale: outcome.stale, - leaks: Object.fromEntries( - Object.entries(outcome.leaks).map(([name, leak]) => [ - name, - { - modules: leak.modules, - exports: leak.exports.map(({ name: exportName, importers }) => ({ - name: exportName, - importers: importers.map(importer => ({ module: toWorkspacePath(importer.module), via: importer.via })), - })), - }, - ]), - ), - })), - }; - - const summaryPath = join(outputRoot(options), 'summary.json'); mkdirSync(dirname(summaryPath), { recursive: true }); - writeFileSync(summaryPath, JSON.stringify(summary, null, 2) + '\n'); + writeFileSync(summaryPath, JSON.stringify(createSummary(report), null, 2) + '\n'); return summaryPath; } - -/** @param {Pick} options */ -function outputRoot(options) { - return join(options.packageRoot, 'dist', 'bundle-isolation'); -} - -/** - * @param {string} fixture - * @param {Pick} options - */ -function fixtureOutputPath(fixture, options) { - return join(outputRoot(options), fixture.replace(/\.fixture\.js$/, '')); -} - -/** - * @param {string} name - * @param {Leak} leak - * @param {RuntimeOptions} options - */ -function describeLeak(name, leak, options) { - const header = ` ${name} - ${leak.modules} module${leak.modules === 1 ? '' : 's'} retained`; - - if (leak.exports.length === 0) { - return `${header}\n no importing symbol identified - rerun with --analyze to inspect the bundle`; - } - - const listed = leak.exports.slice(0, 5).map(({ name: exportName, importers }) => { - const lines = importers.slice(0, 2).map(importer => { - const module = relativeToWorkspace(importer.module, options.workspaceRoot); - return ` <- ${module}${importer.via ? ` (via ${importer.via})` : ''}`; - }); - const hidden = importers.length - lines.length; - if (hidden > 0) { - lines.push(` <- +${hidden} more`); - } - return ` ${exportName}\n${lines.join('\n')}`; - }); - const rest = leak.exports.length - listed.length; - - return `${header}\n${listed.join('\n')}${ - rest > 0 ? `\n ...and ${rest} more export${rest === 1 ? '' : 's'}` : '' - }`; -} - -/** @param {string} root */ -function findFixtures(root) { - if (!existsSync(root)) { - return []; - } - - return readdirSync(root, { recursive: true, withFileTypes: true }) - .filter(entry => entry.isFile() && entry.name.endsWith('.fixture.js')) - .map(entry => { - const path = join(entry.parentPath, entry.name); - return path.slice(root.length + 1); - }) - .sort(); -} - -/** - * @param {{configPath: string, workspaceRoot: string}} options - * @returns {Config} - */ -function loadConfig(options) { - const config = readJson(options.configPath); - const schema = /** @type {object} */ (readJson(schemaPath)); - const validate = new Ajv({ allErrors: true }).compile(schema); - - if (!validate(config)) { - const errors = (validate.errors ?? []) - .map(/** @param {import('ajv').ErrorObject} error */ error => `${error.instancePath || '/'} ${error.message}`) - .join('\n '); - throw new Error( - `Invalid bundle isolation configuration at ${relativeToWorkspace( - options.configPath, - options.workspaceRoot, - )}:\n ${errors}`, - ); - } - - return /** @type {Config} */ (config); -} - -/** @param {string} startDir */ -function findWorkspaceRoot(startDir) { - let dir = startDir; - while (dir !== dirname(dir)) { - if (existsSync(join(dir, 'nx.json'))) { - return dir; - } - dir = dirname(dir); - } - throw new Error(`Could not locate the workspace root above ${startDir}`); -} - -/** @param {string} filePath */ -function readJson(filePath) { - return JSON.parse(readFileSync(filePath, 'utf-8')); -} - -/** - * @param {string} modulePath - * @param {string} workspaceRoot - */ -function relativeToWorkspace(modulePath, workspaceRoot) { - const absolute = isAbsolute(modulePath) ? modulePath : resolve(workspaceRoot, modulePath); - return absolute.startsWith(workspaceRoot + sep) ? absolute.slice(workspaceRoot.length + 1) : modulePath; -} diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js new file mode 100644 index 00000000000000..23daf035a0e70c --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js @@ -0,0 +1,100 @@ +// @ts-check +/** + * Configuration loading, fixture discovery and the path conventions shared by the CLI and the + * report. + */ +const { existsSync, readdirSync, readFileSync } = require('node:fs'); +const { dirname, isAbsolute, join, sep } = require('node:path'); + +const Ajv = /** @type {typeof import('ajv').default} */ (/** @type {unknown} */ (require('ajv'))); + +/** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], allowedViolations: Record}} Config */ + +const schemaPath = join(__dirname, 'schema.json'); +const FIXTURE_SUFFIX = '.fixture.js'; + +/** + * @param {string} configPath + * @param {string} workspaceRoot + * @returns {Config} + */ +function loadConfig(configPath, workspaceRoot) { + const config = readJson(configPath); + const schema = /** @type {object} */ (readJson(schemaPath)); + const validate = new Ajv({ allErrors: true }).compile(schema); + + if (!validate(config)) { + const errors = (validate.errors ?? []) + .map(/** @param {import('ajv').ErrorObject} error */ error => `${error.instancePath || '/'} ${error.message}`) + .join('\n '); + + throw new Error( + `Invalid bundle isolation configuration at ${relativeToWorkspace(configPath, workspaceRoot)}:\n ${errors}`, + ); + } + + return /** @type {Config} */ (config); +} + +/** @param {string} fixturesRoot */ +function findFixtures(fixturesRoot) { + if (!existsSync(fixturesRoot)) { + return []; + } + + return readdirSync(fixturesRoot, { recursive: true, withFileTypes: true }) + .filter(entry => entry.isFile() && entry.name.endsWith(FIXTURE_SUFFIX)) + .map(entry => join(entry.parentPath, entry.name).slice(fixturesRoot.length + 1)) + .sort(); +} + +/** @param {string} startDir */ +function findWorkspaceRoot(startDir) { + let dir = startDir; + + while (dir !== dirname(dir)) { + if (existsSync(join(dir, 'nx.json'))) { + return dir; + } + dir = dirname(dir); + } + + throw new Error(`Could not locate the workspace root above ${startDir}`); +} + +/** @param {string} filePath */ +function readJson(filePath) { + return JSON.parse(readFileSync(filePath, 'utf-8')); +} + +/** + * @param {string} modulePath + * @param {string} workspaceRoot + */ +function relativeToWorkspace(modulePath, workspaceRoot) { + const absolute = isAbsolute(modulePath) ? modulePath : join(workspaceRoot, modulePath); + return absolute.startsWith(workspaceRoot + sep) ? absolute.slice(workspaceRoot.length + 1) : modulePath; +} + +/** @param {string} packageRoot */ +function outputRoot(packageRoot) { + return join(packageRoot, 'dist', 'bundle-isolation'); +} + +/** + * @param {string} fixture + * @param {string} packageRoot + */ +function fixtureOutputPath(fixture, packageRoot) { + return join(outputRoot(packageRoot), fixture.slice(0, -FIXTURE_SUFFIX.length)); +} + +module.exports = { + findFixtures, + findWorkspaceRoot, + fixtureOutputPath, + loadConfig, + outputRoot, + readJson, + relativeToWorkspace, +}; diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js new file mode 100644 index 00000000000000..9d00f062b1815c --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js @@ -0,0 +1,98 @@ +// @ts-check +const { mkdirSync, mkdtempSync, rmSync, writeFileSync } = require('node:fs'); +const { tmpdir } = require('node:os'); +const { join } = require('node:path'); + +const { findFixtures, loadConfig, outputRoot, fixtureOutputPath, relativeToWorkspace } = require('./config'); + +describe('loadConfig', () => { + /** @type {string} */ + let root; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'bundle-isolation-config-')); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + const valid = { + fixturesRoot: './bundle-size', + externals: ['react'], + forbiddenPackages: ['tabster'], + allowedViolations: {}, + }; + + /** @param {object} config */ + const load = config => { + const configPath = join(root, 'bundle-isolation.config.json'); + writeFileSync(configPath, JSON.stringify(config)); + return loadConfig(configPath, root); + }; + + it('returns a valid configuration', () => { + expect(load(valid)).toEqual(valid); + }); + + it('rejects a missing required field rather than silently checking nothing', () => { + expect(() => load({ ...valid, forbiddenPackages: undefined })).toThrow(/must have required property/); + }); + + it('rejects an empty forbidden list, which would make the check meaningless', () => { + expect(() => load({ ...valid, forbiddenPackages: [] })).toThrow(/must NOT have fewer than 1 items/); + }); + + it('rejects unknown fields, so a typo cannot be mistaken for configuration', () => { + expect(() => load({ ...valid, knownViolations: {} })).toThrow(/must NOT have additional properties/); + }); + + it('reports the offending path relative to the workspace', () => { + expect(() => load({ ...valid, externals: 'react' })).toThrow(/bundle-isolation\.config\.json/); + }); +}); + +describe('findFixtures', () => { + /** @type {string} */ + let root; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'bundle-isolation-fixtures-')); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('returns an empty list when the directory does not exist', () => { + expect(findFixtures(join(root, 'missing'))).toEqual([]); + }); + + it('finds fixtures recursively and ignores everything else', () => { + mkdirSync(join(root, 'nested'), { recursive: true }); + writeFileSync(join(root, 'B.fixture.js'), ''); + writeFileSync(join(root, 'A.fixture.js'), ''); + writeFileSync(join(root, 'readme.md'), ''); + writeFileSync(join(root, 'nested', 'C.fixture.js'), ''); + + expect(findFixtures(root)).toEqual(['A.fixture.js', 'B.fixture.js', join('nested', 'C.fixture.js')]); + }); +}); + +describe('paths', () => { + it('derives the output directory from the package root', () => { + expect(outputRoot('/ws/packages/thing')).toBe('/ws/packages/thing/dist/bundle-isolation'); + }); + + it('gives each fixture its own output directory', () => { + expect(fixtureOutputPath('A.fixture.js', '/ws/packages/thing')).toBe('/ws/packages/thing/dist/bundle-isolation/A'); + }); + + it('shortens workspace paths for display', () => { + expect(relativeToWorkspace('/ws/packages/thing/index.js', '/ws')).toBe('packages/thing/index.js'); + }); + + it('leaves paths outside the workspace alone', () => { + expect(relativeToWorkspace('/elsewhere/index.js', '/ws')).toBe('/elsewhere/index.js'); + }); +}); diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js new file mode 100644 index 00000000000000..be3e08fdcc2cab --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js @@ -0,0 +1,398 @@ +// @ts-check +/** + * Turns raw per-fixture bundling results into a verdict, and renders that verdict for the console + * and for `summary.json`. Kept free of webpack and of the file system so it can be tested directly. + */ +const { join } = require('node:path'); + +const { fixtureOutputPath, relativeToWorkspace } = require('./config'); + +/** @typedef {import('./config').Config} Config */ +/** @typedef {import('./bundle-isolation-plugin').Leak} Leak */ +/** @typedef {{configPath: string, analyze: boolean, strict: boolean, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ +/** @typedef {{fixture: string, found: string[], leaks: Record, sourceResolved: string[], error?: string}} FixtureResult */ +/** @typedef {'error' | 'regression' | 'stale' | 'allowed' | 'clean'} FixtureStatus */ +/** @typedef {FixtureResult & {status: FixtureStatus, allowed: string[], tolerated: string[], regressions: string[], stale: string[]}} Outcome */ +/** @typedef {{fixture: string, packages: string[]}} Orphan */ +/** @typedef {{packageName: string, options: RuntimeOptions, outcomes: Outcome[], orphans: Orphan[], totals: Totals, failed: boolean, status: 'passed' | 'passed-with-debt' | 'failed'}} Report */ +/** @typedef {{errors: number, regressions: number, stale: number, tolerated: number}} Totals */ + +/** Widest badge plus its trailing gap, so every fixture line starts at the same column. */ +const BADGE_WIDTH = 'REGRESSION'.length + 2; +const MAX_ORIGINS = 3; +const MAX_EXPORTS = 5; +const MAX_IMPORTERS = 2; + +/** + * @param {{packageName: string, results: FixtureResult[], fixtures: string[], options: RuntimeOptions}} input + * @returns {Report} + */ +function createReport({ packageName, results, fixtures, options }) { + const outcomes = results.map(result => classify(result, options.config.allowedViolations[result.fixture] ?? [])); + const orphans = orphanedAllowlistEntries(fixtures, options.config.allowedViolations); + const totals = { + errors: outcomes.filter(outcome => outcome.status === 'error').length, + regressions: sumBy(outcomes, outcome => outcome.regressions.length), + stale: sumBy(outcomes, outcome => outcome.stale.length), + tolerated: sumBy(outcomes, outcome => outcome.tolerated.length), + }; + + const failed = + orphans.length > 0 || + totals.errors > 0 || + totals.regressions > 0 || + totals.stale > 0 || + (options.strict && totals.tolerated > 0); + + return { + packageName, + options, + outcomes, + orphans, + totals, + failed, + status: failed ? 'failed' : totals.tolerated > 0 ? 'passed-with-debt' : 'passed', + }; +} + +/** + * @param {FixtureResult} result + * @param {string[]} allowed + * @returns {Outcome} + */ +function classify(result, allowed) { + const regressions = result.found.filter(name => !allowed.includes(name)); + const stale = allowed.filter(name => !result.found.includes(name)); + const tolerated = allowed.filter(name => result.found.includes(name)); + + /** @type {FixtureStatus} */ + let status = 'clean'; + if (result.error || result.sourceResolved.length > 0) { + status = 'error'; + } else if (regressions.length > 0) { + status = 'regression'; + } else if (stale.length > 0) { + status = 'stale'; + } else if (tolerated.length > 0) { + status = 'allowed'; + } + + return { ...result, status, allowed, tolerated, regressions, stale }; +} + +/** + * @param {string[]} fixtures + * @param {Record} allowedViolations + * @returns {Orphan[]} + */ +function orphanedAllowlistEntries(fixtures, allowedViolations) { + return Object.entries(allowedViolations) + .filter(([fixture]) => !fixtures.includes(fixture)) + .map(([fixture, packages]) => ({ fixture, packages })); +} + +/** + * @param {Report} report + * @param {string} summaryPath + */ +function formatReport(report, summaryPath) { + const { options } = report; + const lines = [ + `Bundle isolation ยท ${report.packageName}`, + `forbidden: ${options.config.forbiddenPackages.join(', ')}`, + '', + ]; + + for (const outcome of report.outcomes) { + lines.push(...formatFixture(outcome, options), ''); + } + + for (const orphan of report.orphans) { + lines.push( + `${badge('ORPHAN')}${orphan.fixture} - allowlisted (${orphan.packages.join(', ')}) but not a bundle-size fixture`, + ` remove the entry from allowedViolations in ${configLabel(options)}`, + '', + ); + } + + lines.push(...formatVerdict(report), '', ...formatArtifacts(options, summaryPath)); + + return lines.join('\n'); +} + +/** + * @param {Outcome} outcome + * @param {RuntimeOptions} options + */ +function formatFixture(outcome, options) { + if (outcome.status === 'error') { + return [`${badge('ERROR')}${outcome.fixture}`, ...formatError(outcome, options.workspaceRoot)]; + } + + if (outcome.status === 'clean') { + return [`${badge('CLEAN')}${outcome.fixture}`]; + } + + const lines = []; + + if (outcome.regressions.length > 0) { + lines.push( + `${badge('REGRESSION')}${outcome.fixture} - ${count( + outcome.regressions.length, + 'forbidden package', + )} not on the allowlist`, + ...outcome.regressions.flatMap(name => describeLeak(name, outcome.leaks[name], options.workspaceRoot)), + ); + } + + if (outcome.stale.length > 0) { + lines.push( + `${badge('STALE')}${outcome.fixture} - no longer pulls in ${outcome.stale.join(', ')}`, + ` remove it from allowedViolations in ${configLabel(options)} to lock the fix in`, + ); + } + + if (outcome.tolerated.length > 0) { + const modules = sumBy(outcome.tolerated, name => outcome.leaks[name].modules); + lines.push( + `${badge('ALLOWED')}${outcome.fixture} - ${count(outcome.tolerated.length, 'forbidden package')}, ${count( + modules, + 'module', + )}`, + ...formatTolerated(outcome, options.workspaceRoot), + ); + } + + return lines; +} + +/** + * @param {Outcome} outcome + * @param {string} workspaceRoot + */ +function formatError(outcome, workspaceRoot) { + if (outcome.error) { + return [ + ' could not be bundled - is the package built?', + ...outcome.error.split('\n').map(line => ` ${line.trim()}`), + ]; + } + + return [ + ' resolved to package sources instead of built output, so the result is meaningless', + ` e.g. ${relativeToWorkspace(outcome.sourceResolved[0], workspaceRoot)}`, + ]; +} + +/** + * Ordered by module count so the most expensive debt to pay down is listed first. + * + * @param {Outcome} outcome + * @param {string} workspaceRoot + */ +function formatTolerated(outcome, workspaceRoot) { + const rows = outcome.tolerated + .map(name => ({ name, leak: outcome.leaks[name] })) + .sort((left, right) => right.leak.modules - left.leak.modules || left.name.localeCompare(right.name)); + + const nameWidth = Math.max(...rows.map(row => row.name.length)); + const moduleWidth = Math.max(...rows.map(row => count(row.leak.modules, 'module').length)); + + return rows.flatMap(({ name, leak }) => [ + ` ${name.padEnd(nameWidth)} ${count(leak.modules, 'module').padStart(moduleWidth)} ${count( + leak.exports.length, + 'export', + )}`, + ...originsOf(leak, workspaceRoot).map(origin => ` via ${origin}`), + ]); +} + +/** + * @param {string} name + * @param {Leak} leak + * @param {string} workspaceRoot + */ +function describeLeak(name, leak, workspaceRoot) { + const lines = [` ${name} - ${count(leak.modules, 'module')} retained`]; + + if (leak.exports.length === 0) { + lines.push(' no importing symbol identified - rerun with --analyze to inspect the bundle'); + return lines; + } + + for (const { name: exportName, importers } of leak.exports.slice(0, MAX_EXPORTS)) { + lines.push(` ${exportName}`); + + for (const importer of importers.slice(0, MAX_IMPORTERS)) { + const module = relativeToWorkspace(importer.module, workspaceRoot); + lines.push(` <- ${module}${importer.via ? ` (via ${importer.via})` : ''}`); + } + + const hiddenImporters = importers.length - MAX_IMPORTERS; + if (hiddenImporters > 0) { + lines.push(` <- +${hiddenImporters} more`); + } + } + + const hiddenExports = leak.exports.length - MAX_EXPORTS; + if (hiddenExports > 0) { + lines.push(` ...and ${count(hiddenExports, 'more export')}`); + } + + return lines; +} + +/** + * @param {Leak} leak + * @param {string} workspaceRoot + */ +function originsOf(leak, workspaceRoot) { + const origins = new Set( + leak.exports.flatMap(({ importers }) => + importers.map(importer => importer.via ?? relativeToWorkspace(importer.module, workspaceRoot)), + ), + ); + + const listed = [...origins].sort().slice(0, MAX_ORIGINS); + const hidden = origins.size - listed.length; + + return hidden > 0 ? [...listed, `+${count(hidden, 'more entry point')}`] : listed; +} + +/** @param {Report} report */ +function formatVerdict(report) { + const { options, totals, orphans } = report; + const fixtures = count(report.outcomes.length, 'fixture'); + + if (report.failed) { + const parts = [ + totals.errors > 0 && `${count(totals.errors, 'fixture')} failed to bundle`, + totals.regressions > 0 && count(totals.regressions, 'regression'), + totals.stale > 0 && count(totals.stale, 'stale allowlist entry', 'stale allowlist entries'), + orphans.length > 0 && count(orphans.length, 'orphaned allowlist entry', 'orphaned allowlist entries'), + options.strict && totals.tolerated > 0 && `${count(totals.tolerated, 'allowed violation')} rejected by --strict`, + ].filter(Boolean); + + return [`FAIL - ${fixtures}: ${parts.join(', ')}`]; + } + + if (totals.tolerated === 0) { + return [`PASS - ${fixtures} free of ${options.config.forbiddenPackages.join(', ')}`]; + } + + const leaked = [...new Set(report.outcomes.flatMap(outcome => outcome.tolerated))].sort(); + const keptOut = options.config.forbiddenPackages.filter( + pattern => !leaked.some(name => matchesPackagePattern(pattern, name)), + ); + + return [ + `PASS WITH DEBT - ${fixtures}, 0 regressions, ${count(totals.tolerated, 'allowed violation')}`, + ...(keptOut.length > 0 ? [` kept out: ${keptOut.join(', ')}`] : []), + ` allowlist: ${leaked.join(', ')}`, + ` tracked in ${configLabel(options)} - deleting an entry is the goal, adding one is a regression`, + ]; +} + +/** + * @param {RuntimeOptions} options + * @param {string} summaryPath + */ +function formatArtifacts(options, summaryPath) { + const analyzer = options.analyze + ? `${relativeToWorkspace(fixtureOutputPath('.fixture.js', options.packageRoot), options.workspaceRoot)}/` + + 'report.html + report.json' + : 'rerun with --analyze for per-fixture treemaps'; + + return [`summary: ${relativeToWorkspace(summaryPath, options.workspaceRoot)}`, `analyzer: ${analyzer}`]; +} + +/** + * Companion to the analyzer treemap: the same verdict, structured so it can be diffed between runs + * or handed to another tool. + * + * @param {Report} report + */ +function createSummary(report) { + const { options } = report; + const toWorkspacePath = (/** @type {string} */ path) => relativeToWorkspace(path, options.workspaceRoot); + + return { + package: report.packageName, + config: toWorkspacePath(options.configPath), + strict: options.strict, + status: report.status, + forbiddenPackages: options.config.forbiddenPackages, + orphanedAllowlistEntries: report.orphans, + fixtures: report.outcomes.map(outcome => ({ + fixture: outcome.fixture, + status: outcome.status, + analyzerReport: options.analyze + ? toWorkspacePath(join(fixtureOutputPath(outcome.fixture, options.packageRoot), 'report.json')) + : null, + error: outcome.error ?? null, + sourceResolved: outcome.sourceResolved.map(toWorkspacePath), + allowedViolations: outcome.allowed, + tolerated: outcome.tolerated, + regressions: outcome.regressions, + stale: outcome.stale, + leaks: Object.fromEntries( + Object.entries(outcome.leaks).map(([name, leak]) => [ + name, + { + modules: leak.modules, + exports: leak.exports.map(({ name: exportName, importers }) => ({ + name: exportName, + importers: importers.map(importer => ({ module: toWorkspacePath(importer.module), via: importer.via })), + })), + }, + ]), + ), + })), + }; +} + +/** + * @param {string} pattern + * @param {string} name + */ +function matchesPackagePattern(pattern, name) { + return pattern.endsWith('/*') ? name.startsWith(pattern.slice(0, -1)) : name === pattern; +} + +/** + * @param {number} value + * @param {string} singular + * @param {string} [plural] + */ +function count(value, singular, plural) { + return `${value} ${value === 1 ? singular : plural ?? `${singular}s`}`; +} + +/** @param {string} label */ +function badge(label) { + return ` ${label.padEnd(BADGE_WIDTH)}`; +} + +/** @param {RuntimeOptions} options */ +function configLabel(options) { + return relativeToWorkspace(options.configPath, options.workspaceRoot); +} + +/** + * @template TItem + * @param {TItem[]} items + * @param {(item: TItem) => number} valueOf + */ +function sumBy(items, valueOf) { + return items.reduce((total, item) => total + valueOf(item), 0); +} + +module.exports = { + classify, + count, + createReport, + createSummary, + formatReport, + matchesPackagePattern, + orphanedAllowlistEntries, +}; diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js new file mode 100644 index 00000000000000..0b0d79a004bfe8 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js @@ -0,0 +1,312 @@ +// @ts-check +const { classify, count, createReport, createSummary, formatReport, matchesPackagePattern } = require('./report'); + +/** @typedef {import('./report').FixtureResult} FixtureResult */ +/** @typedef {import('./report').RuntimeOptions} RuntimeOptions */ + +const workspaceRoot = '/ws'; +const packageRoot = '/ws/packages/thing'; + +describe('classify', () => { + it('reports a fixture with no leaks as clean', () => { + expect(classify(fixtureResult(), []).status).toBe('clean'); + }); + + it('separates tolerated leaks from regressions', () => { + const outcome = classify(fixtureResult({ found: ['allowed-pkg', 'new-pkg'] }), ['allowed-pkg']); + + expect(outcome).toMatchObject({ status: 'regression', tolerated: ['allowed-pkg'], regressions: ['new-pkg'] }); + }); + + it('flags an allowlist entry that no longer leaks as stale', () => { + const outcome = classify(fixtureResult(), ['fixed-pkg']); + + expect(outcome).toMatchObject({ status: 'stale', stale: ['fixed-pkg'], tolerated: [] }); + }); + + it('reports regressions and stale entries from the same fixture', () => { + const outcome = classify(fixtureResult({ found: ['new-pkg'] }), ['fixed-pkg']); + + expect(outcome).toMatchObject({ regressions: ['new-pkg'], stale: ['fixed-pkg'] }); + }); + + it('treats a build failure as an error regardless of the allowlist', () => { + expect(classify(fixtureResult({ error: 'boom' }), []).status).toBe('error'); + }); + + it('treats resolving to package sources as an error, since the verdict would be meaningless', () => { + const outcome = classify(fixtureResult({ sourceResolved: ['/ws/packages/thing/library/src/index.ts'] }), []); + + expect(outcome.status).toBe('error'); + }); +}); + +describe('createReport', () => { + it('passes when nothing leaked', () => { + const report = createReport(input({ results: [fixtureResult()] })); + + expect(report).toMatchObject({ failed: false, status: 'passed' }); + }); + + it('passes with debt when every leak is allowlisted', () => { + const report = createReport( + input({ + results: [fixtureResult({ found: ['allowed-pkg'] })], + allowedViolations: { 'A.fixture.js': ['allowed-pkg'] }, + }), + ); + + expect(report).toMatchObject({ failed: false, status: 'passed-with-debt' }); + }); + + it('fails allowlisted leaks under --strict', () => { + const report = createReport( + input({ + results: [fixtureResult({ found: ['allowed-pkg'] })], + allowedViolations: { 'A.fixture.js': ['allowed-pkg'] }, + strict: true, + }), + ); + + expect(report).toMatchObject({ failed: true, status: 'failed' }); + }); + + it('fails on an allowlist entry for a fixture that does not exist', () => { + const report = createReport(input({ results: [fixtureResult()], allowedViolations: { 'Gone.fixture.js': ['x'] } })); + + expect(report.orphans).toEqual([{ fixture: 'Gone.fixture.js', packages: ['x'] }]); + expect(report.failed).toBe(true); + }); + + it('totals findings across fixtures', () => { + const report = createReport( + input({ + results: [fixtureResult({ found: ['a-pkg'] }), fixtureResult({ fixture: 'B.fixture.js', found: ['b-pkg'] })], + fixtures: ['A.fixture.js', 'B.fixture.js'], + }), + ); + + expect(report.totals).toEqual({ errors: 0, regressions: 2, stale: 0, tolerated: 0 }); + }); +}); + +describe('formatReport', () => { + it('claims a bundle is free of forbidden packages only when nothing leaked', () => { + const report = createReport(input({ results: [fixtureResult()] })); + + expect(formatReport(report, '/ws/summary.json')).toContain('PASS - 1 fixture free of forbidden-pkg, @scope/*'); + }); + + it('never claims a bundle is free of a package that is merely allowlisted', () => { + const text = formatReport( + createReport( + input({ + results: [fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak() } })], + allowedViolations: { 'A.fixture.js': ['forbidden-pkg'] }, + }), + ), + '/ws/summary.json', + ); + + expect(text).not.toContain('free of'); + expect(text).toContain('PASS WITH DEBT - 1 fixture, 0 regressions, 1 allowed violation'); + }); + + it('lists allowlisted leaks with their size and entry points, ordered by cost', () => { + const text = formatReport( + createReport( + input({ + results: [ + fixtureResult({ + found: ['forbidden-pkg', '@scope/styles'], + leaks: { + 'forbidden-pkg': leak({ modules: 3 }), + '@scope/styles': leak({ modules: 9, via: 'lib/entry.js' }), + }, + }), + ], + allowedViolations: { 'A.fixture.js': ['forbidden-pkg', '@scope/styles'] }, + }), + ), + '/ws/summary.json', + ); + + expect(text).toContain(' ALLOWED A.fixture.js - 2 forbidden packages, 12 modules'); + expect(text).toContain(' via lib/entry.js'); + + const rows = text.split('\n').filter(line => /^ {4}(@scope\/styles|forbidden-pkg)\b/.test(line)); + expect(rows).toEqual([' @scope/styles 9 modules 1 export', ' forbidden-pkg 3 modules 1 export']); + }); + + it('names the packages still kept out, so the allowlist is not read as total defeat', () => { + const text = formatReport( + createReport( + input({ + results: [fixtureResult({ found: ['@scope/styles'], leaks: { '@scope/styles': leak() } })], + allowedViolations: { 'A.fixture.js': ['@scope/styles'] }, + }), + ), + '/ws/summary.json', + ); + + expect(text).toContain(' kept out: forbidden-pkg'); + expect(text).toContain(' allowlist: @scope/styles'); + }); + + it('traces a regression to the importing module and the entry point that pulled it in', () => { + const text = formatReport( + createReport( + input({ + results: [ + fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak({ via: 'lib/entry.js' }) } }), + ], + }), + ), + '/ws/summary.json', + ); + + expect(text).toContain(' REGRESSION A.fixture.js - 1 forbidden package not on the allowlist'); + expect(text).toContain(' forbidden-pkg - 2 modules retained'); + expect(text).toContain(' <- packages/other/lib/importer.js (via lib/entry.js)'); + expect(text).toContain('FAIL - 1 fixture: 1 regression'); + }); + + it('tells the reader how to lock in a fix rather than reporting it as a plain failure', () => { + const text = formatReport( + createReport(input({ results: [fixtureResult()], allowedViolations: { 'A.fixture.js': ['fixed-pkg'] } })), + '/ws/summary.json', + ); + + expect(text).toContain(' STALE A.fixture.js - no longer pulls in fixed-pkg'); + expect(text).toContain('remove it from allowedViolations in packages/thing/config.json to lock the fix in'); + }); + + it('attributes a --strict failure to the flag rather than to a regression', () => { + const text = formatReport( + createReport( + input({ + results: [fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak() } })], + allowedViolations: { 'A.fixture.js': ['forbidden-pkg'] }, + strict: true, + }), + ), + '/ws/summary.json', + ); + + expect(text).toContain('FAIL - 1 fixture: 1 allowed violation rejected by --strict'); + }); + + it('points at the analyzer artifacts only when they were produced', () => { + const withoutAnalyze = formatReport(createReport(input({ results: [fixtureResult()] })), '/ws/summary.json'); + const withAnalyze = formatReport( + createReport(input({ results: [fixtureResult()], analyze: true })), + '/ws/summary.json', + ); + + expect(withoutAnalyze).toContain('analyzer: rerun with --analyze'); + expect(withAnalyze).toContain('packages/thing/dist/bundle-isolation//report.html + report.json'); + }); +}); + +describe('createSummary', () => { + it('mirrors the console verdict', () => { + const summary = createSummary( + createReport( + input({ + results: [ + fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak({ via: 'lib/entry.js' }) } }), + ], + allowedViolations: { 'A.fixture.js': ['forbidden-pkg'] }, + }), + ), + ); + + expect(summary).toMatchObject({ + package: '@fluentui/thing', + status: 'passed-with-debt', + strict: false, + fixtures: [ + { + fixture: 'A.fixture.js', + status: 'allowed', + tolerated: ['forbidden-pkg'], + regressions: [], + leaks: { + 'forbidden-pkg': { + modules: 2, + exports: [ + { name: 'used', importers: [{ module: 'packages/other/lib/importer.js', via: 'lib/entry.js' }] }, + ], + }, + }, + }, + ], + }); + }); + + it('does not point at an analyzer report that was never written', () => { + const summary = createSummary(createReport(input({ results: [fixtureResult()] }))); + + expect(summary.fixtures[0].analyzerReport).toBeNull(); + }); + + it('points at the analyzer report when one was written', () => { + const summary = createSummary(createReport(input({ results: [fixtureResult()], analyze: true }))); + + expect(summary.fixtures[0].analyzerReport).toBe('packages/thing/dist/bundle-isolation/A/report.json'); + }); +}); + +describe('count', () => { + it.each([ + [1, '1 module'], + [0, '0 modules'], + [2, '2 modules'], + ])('pluralises %i', (value, expected) => { + expect(count(value, 'module')).toBe(expected); + }); + + it('uses an explicit plural when appending an s would be wrong', () => { + expect(count(2, 'stale allowlist entry', 'stale allowlist entries')).toBe('2 stale allowlist entries'); + }); +}); + +describe('matchesPackagePattern', () => { + it.each([ + ['@scope/*', '@scope/styles', true], + ['@scope/*', '@other/styles', false], + ['forbidden-pkg', 'forbidden-pkg', true], + ['forbidden-pkg', 'forbidden-pkg-extra', false], + ])('%s vs %s', (pattern, name, expected) => { + expect(matchesPackagePattern(pattern, name)).toBe(expected); + }); +}); + +/** @returns {FixtureResult} */ +function fixtureResult({ fixture = 'A.fixture.js', found = [], leaks = {}, sourceResolved = [], error } = {}) { + return { fixture, found, leaks, sourceResolved, ...(error ? { error } : {}) }; +} + +function leak({ modules = 2, via = null } = {}) { + return { modules, exports: [{ name: 'used', importers: [{ module: '/ws/packages/other/lib/importer.js', via }] }] }; +} + +function input({ results, fixtures = ['A.fixture.js'], allowedViolations = {}, strict = false, analyze = false }) { + /** @type {RuntimeOptions} */ + const options = { + configPath: '/ws/packages/thing/config.json', + analyze, + strict, + fixturesRoot: '/ws/packages/thing/bundle-size', + packageRoot, + workspaceRoot, + config: { + fixturesRoot: './bundle-size', + externals: [], + forbiddenPackages: ['forbidden-pkg', '@scope/*'], + allowedViolations, + }, + }; + + return { packageName: '@fluentui/thing', results, fixtures, options }; +} From 9b47c288449c2ffb2e8b93a1ccfe9aa1228c52f7 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 5 Aug 2026 11:27:24 +0200 Subject: [PATCH 09/14] refactor(verify-bundle-isolation): extract the check into a reusable tool Nothing in the check was specific to the headless package - fixturesRoot, externals, forbiddenPackages and allowedViolations are all config driven, and the analysis was already a standalone webpack plugin. Living inside a component package meant webpack, webpack-bundle-analyzer and ajv were implied dependencies of a component library, and there was nowhere natural for its tests, since the library's jest only covers src/. Moves it to tools/verify-bundle-isolation as @fluentui/verify-bundle-isolation and ports it to TypeScript. Follows the react-integration-tester and scripts-test-ssr pattern of registerTsProject in the bin rather than a build step, so the check gains no build dependency: consumers declare the tool as a devDependency pinned to * and run `yarn run -T verify-bundle-isolation`. The port removed the casts that JSDoc forced on the ajv import, but webpack does not export RuntimeSpec, so it is recovered from a public signature instead. Editor completions now come from a json.schemas mapping in .vscode/settings.json rather than a $schema path in each config. Workspace packages hoist to the root node_modules, so a package-relative $schema would have had to reach back up the tree, and the schema's const on that value could not survive consumers at different depths. --- .vscode/settings.json | 7 + package.json | 1 + .../library/bundle-isolation.config.json | 1 - .../library/package.json | 1 + .../library/project.json | 4 +- .../scripts/verify-bundle-isolation/config.js | 100 --------- .../verify-bundle-isolation/README.md | 95 ++++++--- .../bin/verify-bundle-isolation.js | 15 ++ .../verify-bundle-isolation/eslint.config.js | 18 ++ tools/verify-bundle-isolation/jest.config.js | 14 ++ tools/verify-bundle-isolation/package.json | 16 ++ tools/verify-bundle-isolation/project.json | 7 + .../verify-bundle-isolation/schema.json | 4 +- .../src/bundle-isolation-plugin.spec.ts | 50 ++--- .../src/bundle-isolation-plugin.ts | 157 +++++++------- .../verify-bundle-isolation/src/cli.ts | 109 +++++----- .../src/config.spec.ts | 18 +- tools/verify-bundle-isolation/src/config.ts | 76 +++++++ .../src/report.spec.ts | 43 +++- .../verify-bundle-isolation/src/report.ts | 192 ++++++++---------- tools/verify-bundle-isolation/tsconfig.json | 22 ++ .../verify-bundle-isolation/tsconfig.lib.json | 12 ++ .../tsconfig.spec.json | 10 + yarn.lock | 34 ++++ 24 files changed, 567 insertions(+), 439 deletions(-) delete mode 100644 packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js rename {packages/react-components/react-headless-components-preview/library/scripts => tools}/verify-bundle-isolation/README.md (54%) create mode 100755 tools/verify-bundle-isolation/bin/verify-bundle-isolation.js create mode 100644 tools/verify-bundle-isolation/eslint.config.js create mode 100644 tools/verify-bundle-isolation/jest.config.js create mode 100644 tools/verify-bundle-isolation/package.json create mode 100644 tools/verify-bundle-isolation/project.json rename {packages/react-components/react-headless-components-preview/library/scripts => tools}/verify-bundle-isolation/schema.json (92%) rename packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js => tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts (85%) rename packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js => tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts (64%) rename packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js => tools/verify-bundle-isolation/src/cli.ts (65%) rename packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js => tools/verify-bundle-isolation/src/config.spec.ts (87%) create mode 100644 tools/verify-bundle-isolation/src/config.ts rename packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js => tools/verify-bundle-isolation/src/report.spec.ts (93%) rename packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js => tools/verify-bundle-isolation/src/report.ts (73%) create mode 100644 tools/verify-bundle-isolation/tsconfig.json create mode 100644 tools/verify-bundle-isolation/tsconfig.lib.json create mode 100644 tools/verify-bundle-isolation/tsconfig.spec.json diff --git a/.vscode/settings.json b/.vscode/settings.json index 41454c24a61a1a..8a899daa1fe4e9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,6 +1,13 @@ { // Controls the rendering size of tabs in characters. "editor.tabSize": 2, + // Schemas for repo-local config files, so a `$schema` path is not needed in every consumer. + "json.schemas": [ + { + "fileMatch": ["**/bundle-isolation.config.json"], + "url": "./tools/verify-bundle-isolation/schema.json" + } + ], // When opening a file, `editor.tabSize` and `editor.insertSpaces` will NOT be detected based on the file contents. "editor.detectIndentation": false, "editor.formatOnSave": true, diff --git a/package.json b/package.json index ac69f9009b53f1..8f2ff3ee76b36c 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "@fluentui/react-integration-tester": "*", "@fluentui/scripts-test-ssr": "*", "@fluentui/storybook-llms-extractor": "*", + "@fluentui/verify-bundle-isolation": "*", "@griffel/babel-preset": "1.5.8", "@griffel/eslint-plugin": "^2.0.0", "@griffel/jest-serializer": "1.1.24", diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index 94674dcb7c47d3..5b02c0467e20c7 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -1,5 +1,4 @@ { - "$schema": "./scripts/verify-bundle-isolation/schema.json", "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], diff --git a/packages/react-components/react-headless-components-preview/library/package.json b/packages/react-components/react-headless-components-preview/library/package.json index e896063fdd0a65..5ea5c4f77d04af 100644 --- a/packages/react-components/react-headless-components-preview/library/package.json +++ b/packages/react-components/react-headless-components-preview/library/package.json @@ -386,6 +386,7 @@ }, "devDependencies": { "@fluentui/scripts-cypress": "*", + "@fluentui/verify-bundle-isolation": "*", "@oddbird/popover-polyfill": "^0.6.1" } } diff --git a/packages/react-components/react-headless-components-preview/library/project.json b/packages/react-components/react-headless-components-preview/library/project.json index b7e38080952135..64b4a776a4de8f 100644 --- a/packages/react-components/react-headless-components-preview/library/project.json +++ b/packages/react-components/react-headless-components-preview/library/project.json @@ -14,15 +14,15 @@ "verify-bundle-isolation": { "cache": true, "dependsOn": ["build", "^build"], - "command": "node scripts/verify-bundle-isolation/cli.js", + "command": "yarn run -T verify-bundle-isolation", "options": { "cwd": "{projectRoot}" }, "inputs": [ - "{projectRoot}/scripts/verify-bundle-isolation/**/*", "{projectRoot}/bundle-isolation.config.json", "{projectRoot}/bundle-size/**/*", "{projectRoot}/package.json", + "{workspaceRoot}/tools/verify-bundle-isolation/**/*", { "externalDependencies": ["ajv", "webpack"] } ], "outputs": ["{projectRoot}/dist/bundle-isolation"], diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js b/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js deleted file mode 100644 index 23daf035a0e70c..00000000000000 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.js +++ /dev/null @@ -1,100 +0,0 @@ -// @ts-check -/** - * Configuration loading, fixture discovery and the path conventions shared by the CLI and the - * report. - */ -const { existsSync, readdirSync, readFileSync } = require('node:fs'); -const { dirname, isAbsolute, join, sep } = require('node:path'); - -const Ajv = /** @type {typeof import('ajv').default} */ (/** @type {unknown} */ (require('ajv'))); - -/** @typedef {{fixturesRoot: string, externals: string[], forbiddenPackages: string[], allowedViolations: Record}} Config */ - -const schemaPath = join(__dirname, 'schema.json'); -const FIXTURE_SUFFIX = '.fixture.js'; - -/** - * @param {string} configPath - * @param {string} workspaceRoot - * @returns {Config} - */ -function loadConfig(configPath, workspaceRoot) { - const config = readJson(configPath); - const schema = /** @type {object} */ (readJson(schemaPath)); - const validate = new Ajv({ allErrors: true }).compile(schema); - - if (!validate(config)) { - const errors = (validate.errors ?? []) - .map(/** @param {import('ajv').ErrorObject} error */ error => `${error.instancePath || '/'} ${error.message}`) - .join('\n '); - - throw new Error( - `Invalid bundle isolation configuration at ${relativeToWorkspace(configPath, workspaceRoot)}:\n ${errors}`, - ); - } - - return /** @type {Config} */ (config); -} - -/** @param {string} fixturesRoot */ -function findFixtures(fixturesRoot) { - if (!existsSync(fixturesRoot)) { - return []; - } - - return readdirSync(fixturesRoot, { recursive: true, withFileTypes: true }) - .filter(entry => entry.isFile() && entry.name.endsWith(FIXTURE_SUFFIX)) - .map(entry => join(entry.parentPath, entry.name).slice(fixturesRoot.length + 1)) - .sort(); -} - -/** @param {string} startDir */ -function findWorkspaceRoot(startDir) { - let dir = startDir; - - while (dir !== dirname(dir)) { - if (existsSync(join(dir, 'nx.json'))) { - return dir; - } - dir = dirname(dir); - } - - throw new Error(`Could not locate the workspace root above ${startDir}`); -} - -/** @param {string} filePath */ -function readJson(filePath) { - return JSON.parse(readFileSync(filePath, 'utf-8')); -} - -/** - * @param {string} modulePath - * @param {string} workspaceRoot - */ -function relativeToWorkspace(modulePath, workspaceRoot) { - const absolute = isAbsolute(modulePath) ? modulePath : join(workspaceRoot, modulePath); - return absolute.startsWith(workspaceRoot + sep) ? absolute.slice(workspaceRoot.length + 1) : modulePath; -} - -/** @param {string} packageRoot */ -function outputRoot(packageRoot) { - return join(packageRoot, 'dist', 'bundle-isolation'); -} - -/** - * @param {string} fixture - * @param {string} packageRoot - */ -function fixtureOutputPath(fixture, packageRoot) { - return join(outputRoot(packageRoot), fixture.slice(0, -FIXTURE_SUFFIX.length)); -} - -module.exports = { - findFixtures, - findWorkspaceRoot, - fixtureOutputPath, - loadConfig, - outputRoot, - readJson, - relativeToWorkspace, -}; diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md b/tools/verify-bundle-isolation/README.md similarity index 54% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md rename to tools/verify-bundle-isolation/README.md index 9750a764d9593c..831b3ac662885f 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/README.md +++ b/tools/verify-bundle-isolation/README.md @@ -1,35 +1,62 @@ -# Bundle isolation verification +# @fluentui/verify-bundle-isolation -Fails when a bundle-size fixture retains a runtime the package is meant to stay free of, such as a styling engine or icon set that should have been tree shaken away. +Fails when a bundle-size fixture retains a runtime a package is meant to stay free of, such as a styling engine or icon +set that should have been tree shaken away. ## How it works -Each `*.fixture.js` is bundled with webpack โ€” the same bundler behind the bundle-size numbers โ€” and the resulting module graph is inspected. The check fails when a forbidden package survives tree shaking, when bundling resolves to sources instead of built output, or when a baseline entry is no longer reachable. - -Failures name the exports that kept the package alive, the modules importing them, and the module in this package that pulled those modules in: +Each `*.fixture.js` is bundled with webpack โ€” the same bundler behind the bundle-size numbers โ€” and the resulting module +graph is inspected. A forbidden package that survives tree shaking is reported with the exports that kept it alive, the +modules importing them, and the module in the package under test that pulled those modules in: ``` -AllComponents.fixture.js pulls in forbidden runtime: - @fluentui/react-icons - 9 modules retained - ChevronDownRegular - <- .../react-tag-picker/lib/components/TagPickerControl/useTagPickerControl.js (via lib/tag-picker.js) + REGRESSION AllComponents.fixture.js - 1 forbidden package not on the allowlist @griffel/core - 11 modules retained mergeClasses <- .../react-portal/lib/components/Portal/usePortalMountNode.js (via lib/tag-picker.js) ``` -`via` matters when a leak arrives through a dependency: above, nothing imports `react-portal` directly - `lib/tag-picker.js` re-exports a render function that mounts a portal, which is what drags Griffel in. +`via` matters when a leak arrives through a dependency: above, nothing imports `react-portal` directly โ€” +`lib/tag-picker.js` re-exports a render function that mounts a portal, which is what drags Griffel in. -Attribution intersects webpack's `usedExports` with active import connections, and counts an importer only when that module itself survived into a chunk. Import edges are recorded before tree shaking, so a module importing something it no longer uses is not reported. +Attribution intersects webpack's `usedExports` with active import connections, and counts an importer only when that +module itself survived into a chunk. Import edges are recorded before tree shaking, so a module importing something it +no longer uses is not reported. ## Usage -Run from the package root, once the package and its dependencies are built: +Add the tool as a devDependency of the package to check and give it a target: -```sh -node scripts/verify-bundle-isolation/cli.js +```jsonc +// package.json +{ "devDependencies": { "@fluentui/verify-bundle-isolation": "*" } } ``` +```jsonc +// project.json +{ + "targets": { + "verify-bundle-isolation": { + "cache": true, + "dependsOn": ["build", "^build"], + "command": "yarn run -T verify-bundle-isolation", + "options": { "cwd": "{projectRoot}" }, + "inputs": [ + "{projectRoot}/bundle-isolation.config.json", + "{projectRoot}/bundle-size/**/*", + "{projectRoot}/package.json", + "{workspaceRoot}/tools/verify-bundle-isolation/**/*", + { "externalDependencies": ["ajv", "webpack"] } + ], + "outputs": ["{projectRoot}/dist/bundle-isolation"] + } + } +} +``` + +The check must run against built output, hence `dependsOn`. It reports an error if bundling resolves to package sources +instead, because the verdict would not reflect what ships. + | Flag | Default | Description | | ----------------- | ------------------------------ | -------------------------------------------------------------------- | | `--config ` | `bundle-isolation.config.json` | Configuration file, resolved from the working directory | @@ -42,11 +69,11 @@ node scripts/verify-bundle-isolation/cli.js | ---------------- | ---- | ---------------------------------------------------------------------------------------------------------------------------------- | | `PASS` | 0 | No forbidden package survived bundling. Only this verdict claims a bundle is free of them. | | `PASS WITH DEBT` | 0 | Every surviving forbidden package is on the allowlist. The leaks are listed with their module counts and entry points. | -| `FAIL` | 1 | A regression, a stale or orphaned allowlist entry, a fixture that failed to bundle, or - under `--strict` - any allowed violation. | +| `FAIL` | 1 | A regression, a stale or orphaned allowlist entry, a fixture that failed to bundle, or โ€” under `--strict` โ€” any allowed violation. | Per fixture the report labels each finding `CLEAN`, `ALLOWED`, `REGRESSION`, `STALE` or `ERROR`; a single fixture can carry more than one label. Module and export counts come from a build with `minimize: false`, so they measure how much -of a package is retained, not what it costs to ship - use monosize for bytes. +of a package is retained, not what it costs to ship โ€” use monosize for bytes. ## Output @@ -54,20 +81,19 @@ of a package is retained, not what it costs to ship - use monosize for bytes. | Path | Written | Contents | | ----------------------- | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `summary.json` | always | The console verdict in structured form - overall `status`, and per fixture its `status`, `allowedViolations`, `tolerated`, `regressions`, `stale` and full `leaks` map | +| `summary.json` | always | The console verdict in structured form โ€” overall `status`, and per fixture its `status`, `allowedViolations`, `tolerated`, `regressions`, `stale` and full `leaks` map | | `/report.html` | with `--analyze` | webpack-bundle-analyzer treemap | -| `/report.json` | with `--analyze` | The same data the treemap renders from - module tree with `statSize`, `parsedSize` and `gzipSize` | +| `/report.json` | with `--analyze` | The same data the treemap renders from โ€” module tree with `statSize`, `parsedSize` and `gzipSize` | `leaks` maps a forbidden package to the exports that survived tree shaking and the modules importing them, so the summary answers _what_ leaked and _why_, while the analyzer output answers _how much_ it costs. ## Configuration -The default configuration is `bundle-isolation.config.json` in the package root. Its schema is [`schema.json`](./schema.json). +`bundle-isolation.config.json` in the package root, validated against [`schema.json`](./schema.json). ```json { - "$schema": "./scripts/verify-bundle-isolation/schema.json", "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], @@ -84,9 +110,13 @@ All configured paths are resolved relative to the package root: - `forbiddenPackages` lists exact package names or scoped globs such as `@griffel/*`. - `allowedViolations` maps fixture paths, relative to `fixturesRoot`, to tolerated forbidden packages. +Editor completions come from the `json.schemas` mapping in `.vscode/settings.json`, so consumers do not need a `$schema` +path. Add one only if you want the file to validate outside this repo. + ## Fixtures -Fixtures follow the existing Monosize convention in `bundle-size/*.fixture.js`. A fixture imports the public API under test and uses the import observably so tree shaking cannot discard it. +Fixtures follow the existing monosize convention in `bundle-size/*.fixture.js`. A fixture imports the public API under +test and uses the import observably so tree shaking cannot discard it. ```js import * as Button from '@scope/package/button'; @@ -112,24 +142,25 @@ This prevents fixed leaks from being silently reintroduced. Deleting an entry is ## Layout -| File | Responsibility | -| ------------------------------------------------------------ | ------------------------------------------------------------------------------------- | -| [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) | The analysis - which forbidden packages survived, and why. A standard webpack plugin. | -| [`config.js`](./config.js) | Configuration loading, fixture discovery, path conventions | -| [`report.js`](./report.js) | Turns raw results into a verdict and renders it. No webpack, no file system. | -| [`cli.js`](./cli.js) | Argument parsing and the webpack run that feeds the above | +| File | Responsibility | +| -------------------------------- | ------------------------------------------------------------------------------------- | +| `src/bundle-isolation-plugin.ts` | The analysis โ€” which forbidden packages survived, and why. A standard webpack plugin. | +| `src/config.ts` | Configuration loading, fixture discovery, path conventions | +| `src/report.ts` | Turns raw results into a verdict and renders it. No webpack, no file system. | +| `src/cli.ts` | Argument parsing and the webpack run that feeds the above | -Keeping `report.js` free of webpack and I/O is what makes the verdict testable without bundling anything; -`bundle-isolation-plugin.spec.js` covers attribution by bundling a purpose-built module graph. +Keeping `report.ts` free of webpack and I/O is what makes the verdict testable without bundling anything; +`bundle-isolation-plugin.spec.ts` covers attribution by bundling a purpose-built module graph. ## Reuse in another build -The analysis lives in [`bundle-isolation-plugin.js`](./bundle-isolation-plugin.js) as a standard webpack plugin, so it can run inside an existing build instead of the one the CLI creates: +The analysis is a standard webpack plugin, so it can run inside an existing build instead of the one the CLI creates: -```js +```ts new BundleIsolationPlugin({ forbiddenPackages, workspaceRoot, packageRoot, onReport }); ``` `packageRoot` is optional and only powers the `via` origin. -It requires `optimization.concatenateModules: false`, because scope hoisting merges modules into a `ConcatenatedModule` with no per-module `resource`. +It requires `optimization.concatenateModules: false`, because scope hoisting merges modules into a `ConcatenatedModule` +with no per-module `resource`. diff --git a/tools/verify-bundle-isolation/bin/verify-bundle-isolation.js b/tools/verify-bundle-isolation/bin/verify-bundle-isolation.js new file mode 100755 index 00000000000000..24f6e23c4c5a33 --- /dev/null +++ b/tools/verify-bundle-isolation/bin/verify-bundle-isolation.js @@ -0,0 +1,15 @@ +#!/usr/bin/env node + +// @ts-check + +const { joinPathFragments } = require('@nx/devkit'); +const { registerTsProject } = require('@nx/js/src/internal'); + +registerTsProject(joinPathFragments(__dirname, '..', 'tsconfig.lib.json')); + +const { cli } = require('../src/cli'); + +cli().catch(error => { + console.error(error); + process.exit(1); +}); diff --git a/tools/verify-bundle-isolation/eslint.config.js b/tools/verify-bundle-isolation/eslint.config.js new file mode 100644 index 00000000000000..909643866943c4 --- /dev/null +++ b/tools/verify-bundle-isolation/eslint.config.js @@ -0,0 +1,18 @@ +// @ts-check +const fluentPlugin = require('@fluentui/eslint-plugin'); + +/** @type {import("eslint").Linter.Config[]} */ +module.exports = [ + ...fluentPlugin.configs['flat/node'], + ...fluentPlugin.configs['flat/imports'], + { + rules: { + 'import/no-extraneous-dependencies': [ + 'error', + { + packageDir: ['.', '../../'], + }, + ], + }, + }, +]; diff --git a/tools/verify-bundle-isolation/jest.config.js b/tools/verify-bundle-isolation/jest.config.js new file mode 100644 index 00000000000000..626a3c4d85a083 --- /dev/null +++ b/tools/verify-bundle-isolation/jest.config.js @@ -0,0 +1,14 @@ +// @ts-check + +/** + * @type {import('@jest/types').Config.InitialOptions} + */ +module.exports = { + displayName: 'verify-bundle-isolation', + preset: '../../jest.preset.js', + transform: { + '^.+\\.tsx?$': ['@swc/jest', {}], + }, + coverageDirectory: './coverage', + testEnvironment: 'node', +}; diff --git a/tools/verify-bundle-isolation/package.json b/tools/verify-bundle-isolation/package.json new file mode 100644 index 00000000000000..adc518f486a30a --- /dev/null +++ b/tools/verify-bundle-isolation/package.json @@ -0,0 +1,16 @@ +{ + "name": "@fluentui/verify-bundle-isolation", + "version": "0.0.1", + "description": "Asserts that a package's bundle-size fixtures do not bundle forbidden runtimes", + "private": true, + "type": "commonjs", + "bin": "./bin/verify-bundle-isolation.js", + "dependencies": { + "ajv": "^8.13.0", + "webpack": "5.108.4", + "webpack-bundle-analyzer": "4.10.1" + }, + "devDependencies": { + "@fluentui/eslint-plugin": "*" + } +} diff --git a/tools/verify-bundle-isolation/project.json b/tools/verify-bundle-isolation/project.json new file mode 100644 index 00000000000000..25ea2041087390 --- /dev/null +++ b/tools/verify-bundle-isolation/project.json @@ -0,0 +1,7 @@ +{ + "name": "verify-bundle-isolation", + "$schema": "../../node_modules/nx/schemas/project-schema.json", + "sourceRoot": "tools/verify-bundle-isolation/src", + "projectType": "library", + "tags": ["platform:node", "tools"] +} diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json b/tools/verify-bundle-isolation/schema.json similarity index 92% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json rename to tools/verify-bundle-isolation/schema.json index 422e6b2583bc4c..cbba697a39fc71 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/schema.json +++ b/tools/verify-bundle-isolation/schema.json @@ -5,8 +5,8 @@ "additionalProperties": false, "properties": { "$schema": { - "type": "string", - "const": "./scripts/verify-bundle-isolation/schema.json" + "description": "Path to this schema, relative to the configuration file.", + "type": "string" }, "fixturesRoot": { "description": "Package-relative directory containing bundle-size fixtures.", diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts similarity index 85% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js rename to tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts index 5e610a57cefe91..e0823561c9e8c4 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.spec.js +++ b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts @@ -1,22 +1,15 @@ -/* - * @jest-environment node - */ -// @ts-check -const { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } = require('node:fs'); -const { tmpdir } = require('node:os'); -const { dirname, join } = require('node:path'); +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; -const webpack = require('webpack'); +import webpack from 'webpack'; -const { BundleIsolationPlugin } = require('./bundle-isolation-plugin'); - -/** @typedef {import('./bundle-isolation-plugin').BundleIsolationReport} BundleIsolationReport */ +import { BundleIsolationPlugin, type BundleIsolationReport } from './bundle-isolation-plugin'; jest.setTimeout(60_000); describe('BundleIsolationPlugin', () => { - /** @type {string} */ - let root; + let root: string; beforeEach(() => { // webpack reports resolved real paths, which on macOS differ from the symlinked temp path. @@ -28,8 +21,7 @@ describe('BundleIsolationPlugin', () => { }); describe('attribution', () => { - /** @type {BundleIsolationReport} */ - let report; + let report: BundleIsolationReport; beforeEach(async () => { writeFiles(root, { @@ -124,13 +116,16 @@ describe('BundleIsolationPlugin', () => { }); }); -/** - * @param {{root: string, packageRoot: string, forbiddenPackages: string[]}} options - * @returns {Promise} - */ -function bundle({ root, packageRoot, forbiddenPackages }) { - /** @type {BundleIsolationReport | undefined} */ - let report; +function bundle({ + root, + packageRoot, + forbiddenPackages, +}: { + root: string; + packageRoot: string; + forbiddenPackages: string[]; +}): Promise { + let report: BundleIsolationReport | undefined; const compiler = webpack({ target: 'web', @@ -158,17 +153,13 @@ function bundle({ root, packageRoot, forbiddenPackages }) { rejectPromise(error ?? new Error(stats?.toString({ errors: true }))); return; } - resolvePromise(/** @type {BundleIsolationReport} */ (report)); + resolvePromise(report as BundleIsolationReport); }); }); }); } -/** - * @param {string} root - * @param {Record} files - */ -function writeFiles(root, files) { +function writeFiles(root: string, files: Record) { for (const [path, contents] of Object.entries(files)) { const target = join(root, path); mkdirSync(dirname(target), { recursive: true }); @@ -176,7 +167,6 @@ function writeFiles(root, files) { } } -/** @param {string} name */ -function manifest(name) { +function manifest(name: string) { return JSON.stringify({ name, version: '1.0.0', sideEffects: false }); } diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts similarity index 64% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js rename to tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts index f2b92a7fd1afbb..eb26c42a3e1d7a 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/bundle-isolation-plugin.js +++ b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts @@ -1,4 +1,3 @@ -// @ts-check /** * Reports which forbidden packages survived tree shaking, the exports keeping them alive and the * modules importing those exports. @@ -9,27 +8,46 @@ * Requires `optimization.concatenateModules: false`; scope hoisting merges modules into a * `ConcatenatedModule` with no per-module `resource`, which hides the packages being looked for. */ -const { existsSync, readFileSync } = require('node:fs'); -const { dirname, isAbsolute, join, relative, sep } = require('node:path'); +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, isAbsolute, join, relative, sep } from 'node:path'; -/** @typedef {{module: string, via: string | null}} Importer */ -/** @typedef {{modules: number, exports: Array<{name: string, importers: Importer[]}>}} Leak */ -/** @typedef {{leaks: Record, sourceResolved: string[]}} BundleIsolationReport */ -/** @typedef {{forbiddenPackages: string[], workspaceRoot: string, packageRoot?: string}} AnalysisOptions */ +import type { ChunkGraph, Compilation, Compiler, Module, ModuleGraph } from 'webpack'; + +// webpack declares RuntimeSpec internally but does not export it, so recover it from a signature. +type RuntimeSpec = Parameters['add']>[0]; + +export interface Importer { + module: string; + via: string | null; +} + +export interface Leak { + modules: number; + exports: Array<{ name: string; importers: Importer[] }>; +} + +export interface BundleIsolationReport { + leaks: Record; + sourceResolved: string[]; +} + +export interface AnalysisOptions { + forbiddenPackages: string[]; + workspaceRoot: string; + packageRoot?: string; +} + +type ForbiddenOwnerResolver = (modulePath: string) => string | null; /** `ExportInfo.getUsed()` returns this when nothing references the export. */ const UNUSED = 0; const PLUGIN_NAME = 'BundleIsolationPlugin'; -class BundleIsolationPlugin { - /** @param {AnalysisOptions & {onReport: (report: BundleIsolationReport) => void}} options */ - constructor(options) { - this.options = options; - } +export class BundleIsolationPlugin { + constructor(private options: AnalysisOptions & { onReport: (report: BundleIsolationReport) => void }) {} - /** @param {import('webpack').Compiler} compiler */ - apply(compiler) { + public apply(compiler: Compiler) { compiler.hooks.afterEmit.tap(PLUGIN_NAME, compilation => { this.options.onReport(collectLeaks(compilation, this.options)); }); @@ -40,18 +58,15 @@ class BundleIsolationPlugin { * webpack records import edges for modules whose imports were later eliminated, so edges alone * over-report. A package counts as leaked only when its modules are in a chunk, an export is * reported as used, and the importing module survived as well. - * - * @param {import('webpack').Compilation} compilation - * @param {AnalysisOptions} options - * @returns {BundleIsolationReport} */ -function collectLeaks(compilation, options) { +export function collectLeaks(compilation: Compilation, options: AnalysisOptions): BundleIsolationReport { const { chunkGraph, moduleGraph } = compilation; const ownerOf = createForbiddenOwnerResolver(options); - /** @type {Record}>}>} */ - const collected = {}; - /** @type {string[]} */ - const sourceResolved = []; + const collected: Record< + string, + { modules: number; exports: Map }> } + > = {}; + const sourceResolved: string[] = []; for (const module of compilation.modules) { const resource = resourceOf(module); @@ -81,20 +96,21 @@ function collectLeaks(compilation, options) { } // Keyed per module so two modules exporting the same name are not merged. const key = `${name}\u0000${resource}`; - const known = leak.exports.get(key) ?? { name, importers: new Map() }; + const known = leak.exports.get(key) ?? { name, importers: new Map() }; + for (const importer of importers) { - const importerResource = /** @type {string} */ (resourceOf(importer)); + const importerResource = resourceOf(importer) as string; known.importers.set(importerResource, { module: importerResource, via: packageOriginOf(moduleGraph, chunkGraph, importer, options.packageRoot), }); } + leak.exports.set(key, known); } } - /** @type {Record} */ - const leaks = {}; + const leaks: Record = {}; for (const [name, leak] of Object.entries(collected)) { leaks[name] = { modules: leak.modules, @@ -110,13 +126,7 @@ function collectLeaks(compilation, options) { return { leaks, sourceResolved }; } -/** - * @param {import('webpack').ModuleGraph} moduleGraph - * @param {import('webpack').RuntimeSpec} runtime - * @param {import('webpack').Module} module - * @returns {string[]} - */ -function usedExportNames(moduleGraph, runtime, module) { +function usedExportNames(moduleGraph: ModuleGraph, runtime: RuntimeSpec, module: Module): string[] { const names = []; for (const exportInfo of moduleGraph.getExportsInfo(module).orderedExports) { @@ -128,28 +138,27 @@ function usedExportNames(moduleGraph, runtime, module) { return names; } -/** - * @param {import('webpack').ModuleGraph} moduleGraph - * @param {import('webpack').ChunkGraph} chunkGraph - * @param {import('webpack').RuntimeSpec} runtime - * @param {import('webpack').Module} module - * @param {string} exportName - * @param {(modulePath: string) => string | null} ownerOf - * @returns {import('webpack').Module[]} - */ -function externalImporters(moduleGraph, chunkGraph, runtime, module, exportName, ownerOf) { - /** @type {Map} */ - const importers = new Map(); +function externalImporters( + moduleGraph: ModuleGraph, + chunkGraph: ChunkGraph, + runtime: RuntimeSpec, + module: Module, + exportName: string, + ownerOf: ForbiddenOwnerResolver, +): Module[] { + const importers = new Map(); for (const connection of moduleGraph.getIncomingConnections(module)) { // An eliminated importer keeps an active connection, so its own retention decides. if (!connection.originModule || chunkGraph.getNumberOfModuleChunks(connection.originModule) === 0) { continue; } + const origin = resourceOf(connection.originModule); if (!origin || ownerOf(origin) || connection.getActiveState(runtime) === false) { continue; } + if (importedIds(connection.dependency, moduleGraph)[0] === exportName) { importers.set(origin, connection.originModule); } @@ -161,20 +170,18 @@ function externalImporters(moduleGraph, chunkGraph, runtime, module, exportName, /** * Walks back over retained modules to the first one owned by the package under test, so a leak * reached through a dependency points at the code that pulled that dependency in. - * - * @param {import('webpack').ModuleGraph} moduleGraph - * @param {import('webpack').ChunkGraph} chunkGraph - * @param {import('webpack').Module} module - * @param {string | undefined} packageRoot - * @returns {string | null} */ -function packageOriginOf(moduleGraph, chunkGraph, module, packageRoot) { +function packageOriginOf( + moduleGraph: ModuleGraph, + chunkGraph: ChunkGraph, + module: Module, + packageRoot: string | undefined, +): string | null { if (!packageRoot) { return null; } - /** @param {import('webpack').Module} candidate */ - const owned = candidate => { + const owned = (candidate: Module) => { const resource = resourceOf(candidate); return Boolean(resource && resource.startsWith(packageRoot + sep)); }; @@ -187,17 +194,19 @@ function packageOriginOf(moduleGraph, chunkGraph, module, packageRoot) { const queue = [module]; while (queue.length > 0) { - const current = /** @type {import('webpack').Module} */ (queue.shift()); + const current = queue.shift() as Module; for (const connection of moduleGraph.getIncomingConnections(current)) { const origin = connection.originModule; if (!origin || visited.has(origin) || chunkGraph.getNumberOfModuleChunks(origin) === 0) { continue; } + visited.add(origin); if (owned(origin)) { - return relative(packageRoot, /** @type {string} */ (resourceOf(origin))); + return relative(packageRoot, resourceOf(origin) as string); } + queue.push(origin); } } @@ -205,15 +214,8 @@ function packageOriginOf(moduleGraph, chunkGraph, module, packageRoot) { return null; } -/** - * @param {unknown} dependency - * @param {import('webpack').ModuleGraph} moduleGraph - * @returns {string[]} - */ -function importedIds(dependency, moduleGraph) { - const candidate = /** @type {{getIds?: (graph: import('webpack').ModuleGraph) => string[], ids?: string[]}} */ ( - dependency - ); +function importedIds(dependency: unknown, moduleGraph: ModuleGraph): string[] { + const candidate = dependency as { getIds?: (graph: ModuleGraph) => string[]; ids?: string[] }; if (typeof candidate?.getIds === 'function') { return candidate.getIds(moduleGraph) ?? []; @@ -222,13 +224,8 @@ function importedIds(dependency, moduleGraph) { return candidate?.ids ?? []; } -/** - * @param {import('webpack').Module} module - * @returns {string | null} - */ -function resourceOf(module) { - const candidate = /** @type {{resource?: string}} */ (/** @type {unknown} */ (module)); - return candidate.resource ?? module.nameForCondition() ?? null; +function resourceOf(module: Module): string | null { + return (module as unknown as { resource?: string }).resource ?? module.nameForCondition() ?? null; } /** @@ -237,21 +234,17 @@ function resourceOf(module) { * Ownership is resolved by walking up to the nearest `package.json`, which handles both * `node_modules` dependencies and workspace packages (webpack resolves symlinked workspace * packages to their real path, so there is no `node_modules` segment to match on). - * - * @param {AnalysisOptions} options */ -function createForbiddenOwnerResolver(options) { +function createForbiddenOwnerResolver(options: AnalysisOptions): ForbiddenOwnerResolver { const exact = new Set(options.forbiddenPackages.filter(pattern => !pattern.endsWith('/*'))); const scopes = options.forbiddenPackages .filter(pattern => pattern.endsWith('/*')) .map(pattern => pattern.slice(0, -1)); - /** @type {Map} */ - const cache = new Map(); + const cache = new Map(); - /** @param {string} modulePath */ - return function ownerOf(modulePath) { + return function ownerOf(modulePath: string) { let dir = dirname(isAbsolute(modulePath) ? modulePath : join(options.workspaceRoot, modulePath)); - const visited = []; + const visited: string[] = []; while (dir && dir !== dirname(dir)) { if (cache.has(dir)) { @@ -279,5 +272,3 @@ function createForbiddenOwnerResolver(options) { return null; }; } - -module.exports = { BundleIsolationPlugin }; diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js b/tools/verify-bundle-isolation/src/cli.ts similarity index 65% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js rename to tools/verify-bundle-isolation/src/cli.ts index a215acb95e3890..9a018d749c6336 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/cli.js +++ b/tools/verify-bundle-isolation/src/cli.ts @@ -1,35 +1,37 @@ -// @ts-check /** - * Asserts that no bundle-size fixture in this package bundles a runtime the headless API is - * meant to stay free of. + * Asserts that no bundle-size fixture in a package bundles a runtime its public API is meant to + * stay free of. * * Bundles with webpack so the verdict comes from the same bundler that produces the bundle-size * numbers, and so `usedExports` can name the exact symbols that survived tree shaking. * - * Usage: node scripts/verify-bundle-isolation/cli.js [--config ] [--analyze] [--strict] + * Usage: verify-bundle-isolation [--config ] [--analyze] [--strict] */ -const { mkdirSync, rmSync, writeFileSync } = require('node:fs'); -const { dirname, join, resolve } = require('node:path'); -const { parseArgs } = require('node:util'); - -const webpack = require('webpack'); - -const { BundleIsolationPlugin } = require('./bundle-isolation-plugin'); -const { findFixtures, findWorkspaceRoot, fixtureOutputPath, loadConfig, outputRoot, readJson } = require('./config'); -const { createReport, createSummary, formatReport } = require('./report'); - -/** @typedef {import('./report').Report} Report */ -/** @typedef {import('./report').RuntimeOptions} RuntimeOptions */ -/** @typedef {import('./report').FixtureResult} FixtureResult */ -/** @typedef {import('./bundle-isolation-plugin').BundleIsolationReport} BundleIsolationReport */ - -main(processArgs()).catch(error => { - console.error(error); - process.exit(1); -}); +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { parseArgs } from 'node:util'; + +import webpack, { type Configuration, type Stats, type WebpackPluginInstance } from 'webpack'; + +import { BundleIsolationPlugin, type BundleIsolationReport } from './bundle-isolation-plugin'; +import { findFixtures, findWorkspaceRoot, fixtureOutputPath, loadConfig, outputRoot, readJson } from './config'; +import { + type FixtureResult, + type Report, + type RuntimeOptions, + createReport, + createSummary, + formatReport, +} from './report'; + +interface Args { + configPath: string; + analyze: boolean; + strict: boolean; +} -/** @param {{configPath: string, analyze: boolean, strict: boolean}} args */ -async function main(args) { +export async function cli(): Promise { + const args = processArgs(); const packageRoot = dirname(args.configPath); const workspaceRoot = findWorkspaceRoot(packageRoot); const config = loadConfig(args.configPath, workspaceRoot); @@ -42,8 +44,7 @@ async function main(args) { process.exit(1); } - /** @type {RuntimeOptions} */ - const options = { ...args, config, fixturesRoot, packageRoot, workspaceRoot }; + const options: RuntimeOptions = { ...args, config, fixturesRoot, packageRoot, workspaceRoot }; // Fixtures come and go; a stale output directory would otherwise be mistaken for a fresh report. rmSync(outputRoot(packageRoot), { recursive: true, force: true }); @@ -60,7 +61,7 @@ async function main(args) { } } -function processArgs() { +function processArgs(): Args { const { values } = parseArgs({ options: { config: { type: 'string', default: 'bundle-isolation.config.json' }, @@ -70,21 +71,18 @@ function processArgs() { allowPositionals: false, }); - return { configPath: resolve(process.cwd(), values.config), analyze: values.analyze, strict: values.strict }; + return { + configPath: resolve(process.cwd(), values.config as string), + analyze: values.analyze as boolean, + strict: values.strict as boolean, + }; } -/** - * @param {string} fixture - * @param {RuntimeOptions} options - * @returns {Promise} - */ -async function verifyFixture(fixture, options) { - /** @type {FixtureResult} */ - const result = { fixture, found: [], leaks: {}, sourceResolved: [] }; +async function verifyFixture(fixture: string, options: RuntimeOptions): Promise { + const result: FixtureResult = { fixture, found: [], leaks: {}, sourceResolved: [] }; - /** @type {BundleIsolationReport | undefined} */ - let analysis; - let stats; + let analysis: BundleIsolationReport | undefined; + let stats: Stats; try { stats = await bundleFixture(fixture, options, report => { @@ -112,13 +110,11 @@ async function verifyFixture(fixture, options) { return result; } -/** - * @param {string} fixture - * @param {RuntimeOptions} options - * @param {(report: BundleIsolationReport) => void} onReport - * @returns {Promise} - */ -function bundleFixture(fixture, options, onReport) { +function bundleFixture( + fixture: string, + options: RuntimeOptions, + onReport: (report: BundleIsolationReport) => void, +): Promise { const compiler = webpack(createWebpackConfig(fixture, options, onReport)); return new Promise((resolveStats, rejectStats) => { @@ -134,13 +130,11 @@ function bundleFixture(fixture, options, onReport) { }); } -/** - * @param {string} fixture - * @param {RuntimeOptions} options - * @param {(report: BundleIsolationReport) => void} onReport - * @returns {import('webpack').Configuration} - */ -function createWebpackConfig(fixture, options, onReport) { +function createWebpackConfig( + fixture: string, + options: RuntimeOptions, + onReport: (report: BundleIsolationReport) => void, +): Configuration { const outputPath = fixtureOutputPath(fixture, options.packageRoot); return { @@ -170,10 +164,8 @@ function createWebpackConfig(fixture, options, onReport) { /** * One instance per output format - `analyzerMode` is single valued, so the treemap and its * underlying data need separate plugins. - * - * @param {string} outputPath */ -function createAnalyzerPlugins(outputPath) { +function createAnalyzerPlugins(outputPath: string): WebpackPluginInstance[] { const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); return [ @@ -191,8 +183,7 @@ function createAnalyzerPlugins(outputPath) { ]; } -/** @param {Report} report */ -function writeSummary(report) { +function writeSummary(report: Report): string { const summaryPath = join(outputRoot(report.options.packageRoot), 'summary.json'); mkdirSync(dirname(summaryPath), { recursive: true }); diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js b/tools/verify-bundle-isolation/src/config.spec.ts similarity index 87% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js rename to tools/verify-bundle-isolation/src/config.spec.ts index 9d00f062b1815c..f54dbeaac8e39f 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/config.spec.js +++ b/tools/verify-bundle-isolation/src/config.spec.ts @@ -1,13 +1,11 @@ -// @ts-check -const { mkdirSync, mkdtempSync, rmSync, writeFileSync } = require('node:fs'); -const { tmpdir } = require('node:os'); -const { join } = require('node:path'); +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; -const { findFixtures, loadConfig, outputRoot, fixtureOutputPath, relativeToWorkspace } = require('./config'); +import { findFixtures, fixtureOutputPath, loadConfig, outputRoot, relativeToWorkspace } from './config'; describe('loadConfig', () => { - /** @type {string} */ - let root; + let root: string; beforeEach(() => { root = mkdtempSync(join(tmpdir(), 'bundle-isolation-config-')); @@ -24,8 +22,7 @@ describe('loadConfig', () => { allowedViolations: {}, }; - /** @param {object} config */ - const load = config => { + const load = (config: object) => { const configPath = join(root, 'bundle-isolation.config.json'); writeFileSync(configPath, JSON.stringify(config)); return loadConfig(configPath, root); @@ -53,8 +50,7 @@ describe('loadConfig', () => { }); describe('findFixtures', () => { - /** @type {string} */ - let root; + let root: string; beforeEach(() => { root = mkdtempSync(join(tmpdir(), 'bundle-isolation-fixtures-')); diff --git a/tools/verify-bundle-isolation/src/config.ts b/tools/verify-bundle-isolation/src/config.ts new file mode 100644 index 00000000000000..cad34ab068c6e5 --- /dev/null +++ b/tools/verify-bundle-isolation/src/config.ts @@ -0,0 +1,76 @@ +/** + * Configuration loading, fixture discovery and the path conventions shared by the CLI and the + * report. + */ +import { existsSync, readFileSync, readdirSync } from 'node:fs'; +import { dirname, isAbsolute, join, sep } from 'node:path'; + +import Ajv, { type ErrorObject } from 'ajv'; + +export interface Config { + fixturesRoot: string; + externals: string[]; + forbiddenPackages: string[]; + allowedViolations: Record; +} + +const schemaPath = join(__dirname, '..', 'schema.json'); +const FIXTURE_SUFFIX = '.fixture.js'; + +export function loadConfig(configPath: string, workspaceRoot: string): Config { + const config = readJson(configPath); + const validate = new Ajv({ allErrors: true }).compile(readJson(schemaPath)); + + if (!validate(config)) { + const errors = (validate.errors ?? []) + .map((error: ErrorObject) => `${error.instancePath || '/'} ${error.message}`) + .join('\n '); + + throw new Error( + `Invalid bundle isolation configuration at ${relativeToWorkspace(configPath, workspaceRoot)}:\n ${errors}`, + ); + } + + return config as Config; +} + +export function findFixtures(fixturesRoot: string): string[] { + if (!existsSync(fixturesRoot)) { + return []; + } + + return readdirSync(fixturesRoot, { recursive: true, withFileTypes: true }) + .filter(entry => entry.isFile() && entry.name.endsWith(FIXTURE_SUFFIX)) + .map(entry => join(entry.parentPath, entry.name).slice(fixturesRoot.length + 1)) + .sort(); +} + +export function findWorkspaceRoot(startDir: string): string { + let dir = startDir; + + while (dir !== dirname(dir)) { + if (existsSync(join(dir, 'nx.json'))) { + return dir; + } + dir = dirname(dir); + } + + throw new Error(`Could not locate the workspace root above ${startDir}`); +} + +export function readJson(filePath: string) { + return JSON.parse(readFileSync(filePath, 'utf-8')); +} + +export function relativeToWorkspace(modulePath: string, workspaceRoot: string): string { + const absolute = isAbsolute(modulePath) ? modulePath : join(workspaceRoot, modulePath); + return absolute.startsWith(workspaceRoot + sep) ? absolute.slice(workspaceRoot.length + 1) : modulePath; +} + +export function outputRoot(packageRoot: string): string { + return join(packageRoot, 'dist', 'bundle-isolation'); +} + +export function fixtureOutputPath(fixture: string, packageRoot: string): string { + return join(outputRoot(packageRoot), fixture.slice(0, -FIXTURE_SUFFIX.length)); +} diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js b/tools/verify-bundle-isolation/src/report.spec.ts similarity index 93% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js rename to tools/verify-bundle-isolation/src/report.spec.ts index 0b0d79a004bfe8..a25fb1d89565fb 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.spec.js +++ b/tools/verify-bundle-isolation/src/report.spec.ts @@ -1,8 +1,13 @@ -// @ts-check -const { classify, count, createReport, createSummary, formatReport, matchesPackagePattern } = require('./report'); - -/** @typedef {import('./report').FixtureResult} FixtureResult */ -/** @typedef {import('./report').RuntimeOptions} RuntimeOptions */ +import { + type FixtureResult, + type RuntimeOptions, + classify, + count, + createReport, + createSummary, + formatReport, + matchesPackagePattern, +} from './report'; const workspaceRoot = '/ws'; const packageRoot = '/ws/packages/thing'; @@ -282,18 +287,34 @@ describe('matchesPackagePattern', () => { }); }); -/** @returns {FixtureResult} */ -function fixtureResult({ fixture = 'A.fixture.js', found = [], leaks = {}, sourceResolved = [], error } = {}) { +function fixtureResult({ + fixture = 'A.fixture.js', + found = [] as string[], + leaks = {} as FixtureResult['leaks'], + sourceResolved = [] as string[], + error, +}: Partial = {}): FixtureResult { return { fixture, found, leaks, sourceResolved, ...(error ? { error } : {}) }; } -function leak({ modules = 2, via = null } = {}) { +function leak({ modules = 2, via = null }: { modules?: number; via?: string | null } = {}) { return { modules, exports: [{ name: 'used', importers: [{ module: '/ws/packages/other/lib/importer.js', via }] }] }; } -function input({ results, fixtures = ['A.fixture.js'], allowedViolations = {}, strict = false, analyze = false }) { - /** @type {RuntimeOptions} */ - const options = { +function input({ + results, + fixtures = ['A.fixture.js'], + allowedViolations = {}, + strict = false, + analyze = false, +}: { + results: FixtureResult[]; + fixtures?: string[]; + allowedViolations?: Record; + strict?: boolean; + analyze?: boolean; +}) { + const options: RuntimeOptions = { configPath: '/ws/packages/thing/config.json', analyze, strict, diff --git a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js b/tools/verify-bundle-isolation/src/report.ts similarity index 73% rename from packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js rename to tools/verify-bundle-isolation/src/report.ts index be3e08fdcc2cab..e027ccd56eebd8 100644 --- a/packages/react-components/react-headless-components-preview/library/scripts/verify-bundle-isolation/report.js +++ b/tools/verify-bundle-isolation/src/report.ts @@ -1,21 +1,61 @@ -// @ts-check /** * Turns raw per-fixture bundling results into a verdict, and renders that verdict for the console * and for `summary.json`. Kept free of webpack and of the file system so it can be tested directly. */ -const { join } = require('node:path'); +import { join } from 'node:path'; + +import type { Leak } from './bundle-isolation-plugin'; +import { type Config, fixtureOutputPath, relativeToWorkspace } from './config'; + +export interface RuntimeOptions { + configPath: string; + analyze: boolean; + strict: boolean; + config: Config; + fixturesRoot: string; + packageRoot: string; + workspaceRoot: string; +} + +export interface FixtureResult { + fixture: string; + found: string[]; + leaks: Record; + sourceResolved: string[]; + error?: string; +} + +export type FixtureStatus = 'error' | 'regression' | 'stale' | 'allowed' | 'clean'; + +export interface Outcome extends FixtureResult { + status: FixtureStatus; + allowed: string[]; + tolerated: string[]; + regressions: string[]; + stale: string[]; +} -const { fixtureOutputPath, relativeToWorkspace } = require('./config'); +export interface Orphan { + fixture: string; + packages: string[]; +} -/** @typedef {import('./config').Config} Config */ -/** @typedef {import('./bundle-isolation-plugin').Leak} Leak */ -/** @typedef {{configPath: string, analyze: boolean, strict: boolean, config: Config, fixturesRoot: string, packageRoot: string, workspaceRoot: string}} RuntimeOptions */ -/** @typedef {{fixture: string, found: string[], leaks: Record, sourceResolved: string[], error?: string}} FixtureResult */ -/** @typedef {'error' | 'regression' | 'stale' | 'allowed' | 'clean'} FixtureStatus */ -/** @typedef {FixtureResult & {status: FixtureStatus, allowed: string[], tolerated: string[], regressions: string[], stale: string[]}} Outcome */ -/** @typedef {{fixture: string, packages: string[]}} Orphan */ -/** @typedef {{packageName: string, options: RuntimeOptions, outcomes: Outcome[], orphans: Orphan[], totals: Totals, failed: boolean, status: 'passed' | 'passed-with-debt' | 'failed'}} Report */ -/** @typedef {{errors: number, regressions: number, stale: number, tolerated: number}} Totals */ +export interface Totals { + errors: number; + regressions: number; + stale: number; + tolerated: number; +} + +export interface Report { + packageName: string; + options: RuntimeOptions; + outcomes: Outcome[]; + orphans: Orphan[]; + totals: Totals; + failed: boolean; + status: 'passed' | 'passed-with-debt' | 'failed'; +} /** Widest badge plus its trailing gap, so every fixture line starts at the same column. */ const BADGE_WIDTH = 'REGRESSION'.length + 2; @@ -23,14 +63,20 @@ const MAX_ORIGINS = 3; const MAX_EXPORTS = 5; const MAX_IMPORTERS = 2; -/** - * @param {{packageName: string, results: FixtureResult[], fixtures: string[], options: RuntimeOptions}} input - * @returns {Report} - */ -function createReport({ packageName, results, fixtures, options }) { +export function createReport({ + packageName, + results, + fixtures, + options, +}: { + packageName: string; + results: FixtureResult[]; + fixtures: string[]; + options: RuntimeOptions; +}): Report { const outcomes = results.map(result => classify(result, options.config.allowedViolations[result.fixture] ?? [])); const orphans = orphanedAllowlistEntries(fixtures, options.config.allowedViolations); - const totals = { + const totals: Totals = { errors: outcomes.filter(outcome => outcome.status === 'error').length, regressions: sumBy(outcomes, outcome => outcome.regressions.length), stale: sumBy(outcomes, outcome => outcome.stale.length), @@ -55,18 +101,12 @@ function createReport({ packageName, results, fixtures, options }) { }; } -/** - * @param {FixtureResult} result - * @param {string[]} allowed - * @returns {Outcome} - */ -function classify(result, allowed) { +export function classify(result: FixtureResult, allowed: string[]): Outcome { const regressions = result.found.filter(name => !allowed.includes(name)); const stale = allowed.filter(name => !result.found.includes(name)); const tolerated = allowed.filter(name => result.found.includes(name)); - /** @type {FixtureStatus} */ - let status = 'clean'; + let status: FixtureStatus = 'clean'; if (result.error || result.sourceResolved.length > 0) { status = 'error'; } else if (regressions.length > 0) { @@ -80,22 +120,13 @@ function classify(result, allowed) { return { ...result, status, allowed, tolerated, regressions, stale }; } -/** - * @param {string[]} fixtures - * @param {Record} allowedViolations - * @returns {Orphan[]} - */ -function orphanedAllowlistEntries(fixtures, allowedViolations) { +export function orphanedAllowlistEntries(fixtures: string[], allowedViolations: Record): Orphan[] { return Object.entries(allowedViolations) .filter(([fixture]) => !fixtures.includes(fixture)) .map(([fixture, packages]) => ({ fixture, packages })); } -/** - * @param {Report} report - * @param {string} summaryPath - */ -function formatReport(report, summaryPath) { +export function formatReport(report: Report, summaryPath: string): string { const { options } = report; const lines = [ `Bundle isolation ยท ${report.packageName}`, @@ -120,11 +151,7 @@ function formatReport(report, summaryPath) { return lines.join('\n'); } -/** - * @param {Outcome} outcome - * @param {RuntimeOptions} options - */ -function formatFixture(outcome, options) { +function formatFixture(outcome: Outcome, options: RuntimeOptions): string[] { if (outcome.status === 'error') { return [`${badge('ERROR')}${outcome.fixture}`, ...formatError(outcome, options.workspaceRoot)]; } @@ -133,7 +160,7 @@ function formatFixture(outcome, options) { return [`${badge('CLEAN')}${outcome.fixture}`]; } - const lines = []; + const lines: string[] = []; if (outcome.regressions.length > 0) { lines.push( @@ -166,11 +193,7 @@ function formatFixture(outcome, options) { return lines; } -/** - * @param {Outcome} outcome - * @param {string} workspaceRoot - */ -function formatError(outcome, workspaceRoot) { +function formatError(outcome: Outcome, workspaceRoot: string): string[] { if (outcome.error) { return [ ' could not be bundled - is the package built?', @@ -184,13 +207,8 @@ function formatError(outcome, workspaceRoot) { ]; } -/** - * Ordered by module count so the most expensive debt to pay down is listed first. - * - * @param {Outcome} outcome - * @param {string} workspaceRoot - */ -function formatTolerated(outcome, workspaceRoot) { +/** Ordered by module count so the most expensive debt to pay down is listed first. */ +function formatTolerated(outcome: Outcome, workspaceRoot: string): string[] { const rows = outcome.tolerated .map(name => ({ name, leak: outcome.leaks[name] })) .sort((left, right) => right.leak.modules - left.leak.modules || left.name.localeCompare(right.name)); @@ -207,12 +225,7 @@ function formatTolerated(outcome, workspaceRoot) { ]); } -/** - * @param {string} name - * @param {Leak} leak - * @param {string} workspaceRoot - */ -function describeLeak(name, leak, workspaceRoot) { +function describeLeak(name: string, leak: Leak, workspaceRoot: string): string[] { const lines = [` ${name} - ${count(leak.modules, 'module')} retained`]; if (leak.exports.length === 0) { @@ -242,11 +255,7 @@ function describeLeak(name, leak, workspaceRoot) { return lines; } -/** - * @param {Leak} leak - * @param {string} workspaceRoot - */ -function originsOf(leak, workspaceRoot) { +function originsOf(leak: Leak, workspaceRoot: string): string[] { const origins = new Set( leak.exports.flatMap(({ importers }) => importers.map(importer => importer.via ?? relativeToWorkspace(importer.module, workspaceRoot)), @@ -259,8 +268,7 @@ function originsOf(leak, workspaceRoot) { return hidden > 0 ? [...listed, `+${count(hidden, 'more entry point')}`] : listed; } -/** @param {Report} report */ -function formatVerdict(report) { +function formatVerdict(report: Report): string[] { const { options, totals, orphans } = report; const fixtures = count(report.outcomes.length, 'fixture'); @@ -293,11 +301,7 @@ function formatVerdict(report) { ]; } -/** - * @param {RuntimeOptions} options - * @param {string} summaryPath - */ -function formatArtifacts(options, summaryPath) { +function formatArtifacts(options: RuntimeOptions, summaryPath: string): string[] { const analyzer = options.analyze ? `${relativeToWorkspace(fixtureOutputPath('.fixture.js', options.packageRoot), options.workspaceRoot)}/` + 'report.html + report.json' @@ -309,12 +313,10 @@ function formatArtifacts(options, summaryPath) { /** * Companion to the analyzer treemap: the same verdict, structured so it can be diffed between runs * or handed to another tool. - * - * @param {Report} report */ -function createSummary(report) { +export function createSummary(report: Report) { const { options } = report; - const toWorkspacePath = (/** @type {string} */ path) => relativeToWorkspace(path, options.workspaceRoot); + const toWorkspacePath = (path: string) => relativeToWorkspace(path, options.workspaceRoot); return { package: report.packageName, @@ -351,48 +353,22 @@ function createSummary(report) { }; } -/** - * @param {string} pattern - * @param {string} name - */ -function matchesPackagePattern(pattern, name) { +export function matchesPackagePattern(pattern: string, name: string): boolean { return pattern.endsWith('/*') ? name.startsWith(pattern.slice(0, -1)) : name === pattern; } -/** - * @param {number} value - * @param {string} singular - * @param {string} [plural] - */ -function count(value, singular, plural) { +export function count(value: number, singular: string, plural?: string): string { return `${value} ${value === 1 ? singular : plural ?? `${singular}s`}`; } -/** @param {string} label */ -function badge(label) { +function badge(label: string): string { return ` ${label.padEnd(BADGE_WIDTH)}`; } -/** @param {RuntimeOptions} options */ -function configLabel(options) { +function configLabel(options: RuntimeOptions): string { return relativeToWorkspace(options.configPath, options.workspaceRoot); } -/** - * @template TItem - * @param {TItem[]} items - * @param {(item: TItem) => number} valueOf - */ -function sumBy(items, valueOf) { +function sumBy(items: TItem[], valueOf: (item: TItem) => number): number { return items.reduce((total, item) => total + valueOf(item), 0); } - -module.exports = { - classify, - count, - createReport, - createSummary, - formatReport, - matchesPackagePattern, - orphanedAllowlistEntries, -}; diff --git a/tools/verify-bundle-isolation/tsconfig.json b/tools/verify-bundle-isolation/tsconfig.json new file mode 100644 index 00000000000000..a95e1d6f4a43cc --- /dev/null +++ b/tools/verify-bundle-isolation/tsconfig.json @@ -0,0 +1,22 @@ +{ + "extends": "@tsconfig/node20/tsconfig.json", + "compilerOptions": { + "target": "ES2019", + "pretty": true, + "noEmit": true, + "allowJs": true, + "checkJs": true, + "sourceMap": true, + "noUnusedLocals": true + }, + "include": [], + "files": [], + "references": [ + { + "path": "./tsconfig.lib.json" + }, + { + "path": "./tsconfig.spec.json" + } + ] +} diff --git a/tools/verify-bundle-isolation/tsconfig.lib.json b/tools/verify-bundle-isolation/tsconfig.lib.json new file mode 100644 index 00000000000000..8407b0a4160ae0 --- /dev/null +++ b/tools/verify-bundle-isolation/tsconfig.lib.json @@ -0,0 +1,12 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": false, + "lib": ["ES2019"], + "outDir": "../../dist/out-tsc", + "types": ["node"], + "resolveJsonModule": true + }, + "exclude": ["**/*.spec.ts", "**/*.test.ts"], + "include": ["./src/**/*.ts", "./src/**/*.js"] +} diff --git a/tools/verify-bundle-isolation/tsconfig.spec.json b/tools/verify-bundle-isolation/tsconfig.spec.json new file mode 100644 index 00000000000000..a0a0008c224b9f --- /dev/null +++ b/tools/verify-bundle-isolation/tsconfig.spec.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "module": "CommonJS", + "moduleResolution": "Node10", + "outDir": "dist", + "types": ["jest", "node"] + }, + "include": ["**/*.spec.ts", "**/*.test.ts", "**/*.d.ts"] +} diff --git a/yarn.lock b/yarn.lock index 78e9e50de716e0..b1a4fa44fbbe58 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2772,6 +2772,7 @@ __metadata: "@fluentui/react-northstar": "npm:0.66.5" "@fluentui/scripts-test-ssr": "npm:*" "@fluentui/storybook-llms-extractor": "npm:*" + "@fluentui/verify-bundle-isolation": "npm:*" "@griffel/babel-preset": "npm:1.5.8" "@griffel/eslint-plugin": "npm:^2.0.0" "@griffel/jest-serializer": "npm:1.1.24" @@ -4321,6 +4322,7 @@ __metadata: "@fluentui/react-tooltip": "npm:^9.10.4" "@fluentui/react-utilities": "npm:^9.26.5" "@fluentui/scripts-cypress": "npm:*" + "@fluentui/verify-bundle-isolation": "npm:*" "@oddbird/popover-polyfill": "npm:^0.6.1" "@swc/helpers": "npm:^0.5.1" peerDependencies: @@ -6435,6 +6437,19 @@ __metadata: languageName: unknown linkType: soft +"@fluentui/verify-bundle-isolation@npm:*, @fluentui/verify-bundle-isolation@workspace:tools/verify-bundle-isolation": + version: 0.0.0-use.local + resolution: "@fluentui/verify-bundle-isolation@workspace:tools/verify-bundle-isolation" + dependencies: + "@fluentui/eslint-plugin": "npm:*" + ajv: "npm:^8.13.0" + webpack: "npm:5.108.4" + webpack-bundle-analyzer: "npm:4.10.1" + bin: + verify-bundle-isolation: ./bin/verify-bundle-isolation.js + languageName: unknown + linkType: soft + "@fluentui/visual-regression-assert@workspace:tools/visual-regression-assert": version: 0.0.0-use.local resolution: "@fluentui/visual-regression-assert@workspace:tools/visual-regression-assert" @@ -12292,6 +12307,18 @@ __metadata: languageName: node linkType: hard +"ajv@npm:^8.13.0": + version: 8.20.0 + resolution: "ajv@npm:8.20.0" + dependencies: + fast-deep-equal: "npm:^3.1.3" + fast-uri: "npm:^3.0.1" + json-schema-traverse: "npm:^1.0.0" + require-from-string: "npm:^2.0.2" + checksum: 10c0/5df9a1c8f83863cde1bd3a9ddb426f599718f88e3dc9153616c79fb28e0be455335830d7f21d745576519f057b371352daa31047b6a33d7036fe08777d60cf2a + languageName: node + linkType: hard + "ajv@npm:~8.12.0": version: 8.12.0 resolution: "ajv@npm:8.12.0" @@ -18176,6 +18203,13 @@ __metadata: languageName: node linkType: hard +"fast-uri@npm:^3.0.1": + version: 3.1.5 + resolution: "fast-uri@npm:3.1.5" + checksum: 10c0/2bf60eb800dd610c65e17be436425dcb21c92aff3a87d442a8bccab0b7b071e88cf1a5d7d1ea946370b937e6fc0375c405c0296c10587e57de4f78be4646d1d0 + languageName: node + linkType: hard + "fastest-levenshtein@npm:^1.0.12": version: 1.0.12 resolution: "fastest-levenshtein@npm:1.0.12" From e8bd343730e2d87fbc900951f479136d67e283af Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 5 Aug 2026 13:38:30 +0200 Subject: [PATCH 10/14] fix(verify-bundle-isolation): pin webpack and fix cache invalidation The tool declared webpack with an exact version to keep it identical to the one behind the bundle-size numbers, which is the only reason its verdict means anything. Switching to a caret range let yarn resolve 5.109.2 into a nested node_modules while the rest of the repo stayed on 5.108.4, and 5.109 changed module resolution such that workspace packages resolve to their sources instead of built output - every fixture failed to bundle. Keeps the caret and pins webpack through resolutions in the root package.json instead, so a single version is enforced repo wide rather than per consumer. Also replaces the target's hand written inputs with `default` and `^default`. Overriding inputs wholesale had dropped Nx's dependency tracking, so the task was served from cache after changes to the tool and, worse, after changes to any of the packages whose built output it bundles. `^default` restores both, and makes the devDependency on the tool load bearing rather than decorative. Drops the eslint-plugin devDependency, since the lint target only covers src, and adds an export map so the schema can be resolved by package name programmatically. Editors resolve `$schema` relative to the config file without Node package resolution, so consumers still need a workspace relative path there. --- .vscode/settings.json | 7 ----- package.json | 1 + .../library/bundle-isolation.config.json | 1 + .../library/project.json | 8 +---- tools/verify-bundle-isolation/README.md | 24 +++++++++------ tools/verify-bundle-isolation/package.json | 11 +++---- yarn.lock | 29 ++++++++++++++++--- 7 files changed, 49 insertions(+), 32 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 8a899daa1fe4e9..41454c24a61a1a 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,13 +1,6 @@ { // Controls the rendering size of tabs in characters. "editor.tabSize": 2, - // Schemas for repo-local config files, so a `$schema` path is not needed in every consumer. - "json.schemas": [ - { - "fileMatch": ["**/bundle-isolation.config.json"], - "url": "./tools/verify-bundle-isolation/schema.json" - } - ], // When opening a file, `editor.tabSize` and `editor.insertSpaces` will NOT be detected based on the file contents. "editor.detectIndentation": false, "editor.formatOnSave": true, diff --git a/package.json b/package.json index 8f2ff3ee76b36c..d4ec034bee5fe9 100644 --- a/package.json +++ b/package.json @@ -381,6 +381,7 @@ "swc-loader": "0.2.6", "syncpack/minimatch": "^9.0.7", "tar-fs": "2.1.4", + "webpack": "5.108.4", "ws": "^8.21.1" }, "nx": { diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index 5b02c0467e20c7..1de6c83ea50754 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -1,4 +1,5 @@ { + "$schema": "../../../../tools/verify-bundle-isolation/schema.json", "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], diff --git a/packages/react-components/react-headless-components-preview/library/project.json b/packages/react-components/react-headless-components-preview/library/project.json index 64b4a776a4de8f..7b99dac170d309 100644 --- a/packages/react-components/react-headless-components-preview/library/project.json +++ b/packages/react-components/react-headless-components-preview/library/project.json @@ -18,13 +18,7 @@ "options": { "cwd": "{projectRoot}" }, - "inputs": [ - "{projectRoot}/bundle-isolation.config.json", - "{projectRoot}/bundle-size/**/*", - "{projectRoot}/package.json", - "{workspaceRoot}/tools/verify-bundle-isolation/**/*", - { "externalDependencies": ["ajv", "webpack"] } - ], + "inputs": ["default", "^default", { "externalDependencies": ["ajv", "webpack"] }], "outputs": ["{projectRoot}/dist/bundle-isolation"], "metadata": { "technologies": ["webpack"], diff --git a/tools/verify-bundle-isolation/README.md b/tools/verify-bundle-isolation/README.md index 831b3ac662885f..ef4f82f58b1546 100644 --- a/tools/verify-bundle-isolation/README.md +++ b/tools/verify-bundle-isolation/README.md @@ -41,13 +41,7 @@ Add the tool as a devDependency of the package to check and give it a target: "dependsOn": ["build", "^build"], "command": "yarn run -T verify-bundle-isolation", "options": { "cwd": "{projectRoot}" }, - "inputs": [ - "{projectRoot}/bundle-isolation.config.json", - "{projectRoot}/bundle-size/**/*", - "{projectRoot}/package.json", - "{workspaceRoot}/tools/verify-bundle-isolation/**/*", - { "externalDependencies": ["ajv", "webpack"] } - ], + "inputs": ["default", "^default", { "externalDependencies": ["ajv", "webpack"] }], "outputs": ["{projectRoot}/dist/bundle-isolation"] } } @@ -57,6 +51,14 @@ Add the tool as a devDependency of the package to check and give it a target: The check must run against built output, hence `dependsOn`. It reports an error if bundling resolves to package sources instead, because the verdict would not reflect what ships. +`^default` is what makes the cache correct: this task's result depends on every dependency's files, and on the tool +itself, which is a dependency by virtue of the devDependency. Replacing it with a hand-written input list silently +serves stale verdicts after a dependency changes. + +The repo pins webpack to a single version through `resolutions` in the root `package.json`. That is deliberate - the +verdict is only meaningful if it comes from the same bundler that produces the bundle-size numbers, and webpack 5.109 +changed module resolution in a way that makes these packages resolve to sources rather than built output. + | Flag | Default | Description | | ----------------- | ------------------------------ | -------------------------------------------------------------------- | | `--config ` | `bundle-isolation.config.json` | Configuration file, resolved from the working directory | @@ -94,6 +96,7 @@ summary answers _what_ leaked and _why_, while the analyzer output answers _how ```json { + "$schema": "../../../../tools/verify-bundle-isolation/schema.json", "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], @@ -110,8 +113,11 @@ All configured paths are resolved relative to the package root: - `forbiddenPackages` lists exact package names or scoped globs such as `@griffel/*`. - `allowedViolations` maps fixture paths, relative to `fixturesRoot`, to tolerated forbidden packages. -Editor completions come from the `json.schemas` mapping in `.vscode/settings.json`, so consumers do not need a `$schema` -path. Add one only if you want the file to validate outside this repo. +`$schema` has to be a workspace-relative path. Editors resolve it against the config file and do not apply Node package +resolution, so `@fluentui/verify-bundle-isolation/schema.json` will not work there despite the export map. The export +map exists for programmatic consumers, which can `require.resolve('@fluentui/verify-bundle-isolation/schema.json')`. + +Validation itself never depends on `$schema` - the CLI always loads the schema shipped alongside it. ## Fixtures diff --git a/tools/verify-bundle-isolation/package.json b/tools/verify-bundle-isolation/package.json index adc518f486a30a..eb09445ad8ed41 100644 --- a/tools/verify-bundle-isolation/package.json +++ b/tools/verify-bundle-isolation/package.json @@ -5,12 +5,13 @@ "private": true, "type": "commonjs", "bin": "./bin/verify-bundle-isolation.js", + "exports": { + "./schema.json": "./schema.json", + "./package.json": "./package.json" + }, "dependencies": { "ajv": "^8.13.0", - "webpack": "5.108.4", - "webpack-bundle-analyzer": "4.10.1" - }, - "devDependencies": { - "@fluentui/eslint-plugin": "*" + "webpack": "^5.108.4", + "webpack-bundle-analyzer": "^4.10.1" } } diff --git a/yarn.lock b/yarn.lock index b1a4fa44fbbe58..faa179eeebfce7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6441,10 +6441,9 @@ __metadata: version: 0.0.0-use.local resolution: "@fluentui/verify-bundle-isolation@workspace:tools/verify-bundle-isolation" dependencies: - "@fluentui/eslint-plugin": "npm:*" ajv: "npm:^8.13.0" - webpack: "npm:5.108.4" - webpack-bundle-analyzer: "npm:4.10.1" + webpack: "npm:^5.108.4" + webpack-bundle-analyzer: "npm:^4.10.1" bin: verify-bundle-isolation: ./bin/verify-bundle-isolation.js languageName: unknown @@ -32010,6 +32009,28 @@ __metadata: languageName: node linkType: hard +"webpack-bundle-analyzer@npm:^4.10.1": + version: 4.10.2 + resolution: "webpack-bundle-analyzer@npm:4.10.2" + dependencies: + "@discoveryjs/json-ext": "npm:0.5.7" + acorn: "npm:^8.0.4" + acorn-walk: "npm:^8.0.0" + commander: "npm:^7.2.0" + debounce: "npm:^1.2.1" + escape-string-regexp: "npm:^4.0.0" + gzip-size: "npm:^6.0.0" + html-escaper: "npm:^2.0.2" + opener: "npm:^1.5.2" + picocolors: "npm:^1.0.0" + sirv: "npm:^2.0.3" + ws: "npm:^7.3.1" + bin: + webpack-bundle-analyzer: lib/bin/analyzer.js + checksum: 10c0/00603040e244ead15b2d92981f0559fa14216381349412a30070a7358eb3994cd61a8221d34a3b3fb8202dc3d1c5ee1fbbe94c5c52da536e5b410aa1cf279a48 + languageName: node + linkType: hard + "webpack-cli@npm:5.1.4": version: 5.1.4 resolution: "webpack-cli@npm:5.1.4" @@ -32187,7 +32208,7 @@ __metadata: languageName: node linkType: hard -"webpack@npm:5, webpack@npm:5.108.4, webpack@npm:^5, webpack@npm:^5.1.0, webpack@npm:^5.106.2": +"webpack@npm:5.108.4": version: 5.108.4 resolution: "webpack@npm:5.108.4" dependencies: From 5b55d295a195dd5c1b19b8b0f2f463541fd358e0 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 5 Aug 2026 14:41:37 +0200 Subject: [PATCH 11/14] chore: yarn dedupe --- yarn.lock | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/yarn.lock b/yarn.lock index faa179eeebfce7..7b9004fa8dd480 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12294,19 +12294,7 @@ __metadata: languageName: node linkType: hard -"ajv@npm:^8.0.0, ajv@npm:^8.0.1, ajv@npm:^8.4.0, ajv@npm:^8.9.0, ajv@npm:~8.13.0": - version: 8.13.0 - resolution: "ajv@npm:8.13.0" - dependencies: - fast-deep-equal: "npm:^3.1.3" - json-schema-traverse: "npm:^1.0.0" - require-from-string: "npm:^2.0.2" - uri-js: "npm:^4.4.1" - checksum: 10c0/14c6497b6f72843986d7344175a1aa0e2c35b1e7f7475e55bc582cddb765fca7e6bf950f465dc7846f817776d9541b706f4b5b3fbedd8dfdeb5fce6f22864264 - languageName: node - linkType: hard - -"ajv@npm:^8.13.0": +"ajv@npm:^8.0.0, ajv@npm:^8.0.1, ajv@npm:^8.13.0, ajv@npm:^8.4.0, ajv@npm:^8.9.0": version: 8.20.0 resolution: "ajv@npm:8.20.0" dependencies: @@ -12330,6 +12318,18 @@ __metadata: languageName: node linkType: hard +"ajv@npm:~8.13.0": + version: 8.13.0 + resolution: "ajv@npm:8.13.0" + dependencies: + fast-deep-equal: "npm:^3.1.3" + json-schema-traverse: "npm:^1.0.0" + require-from-string: "npm:^2.0.2" + uri-js: "npm:^4.4.1" + checksum: 10c0/14c6497b6f72843986d7344175a1aa0e2c35b1e7f7475e55bc582cddb765fca7e6bf950f465dc7846f817776d9541b706f4b5b3fbedd8dfdeb5fce6f22864264 + languageName: node + linkType: hard + "anchor-markdown-header@npm:~0.5.7": version: 0.5.7 resolution: "anchor-markdown-header@npm:0.5.7" From 70250a2ac461351dc5630bbad3a4de312bc8fec6 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 5 Aug 2026 15:15:29 +0200 Subject: [PATCH 12/14] chore(react-headless-components-preview): allowlist the portal leak in TagPicker fixture #36503 merged, adding TagPicker.fixture.js. It retains @griffel/core and @griffel/react through the portal that ./tag-picker mounts, which the check correctly reports as a regression because the fixture is new and nothing allowlisted it. Records it as tracked debt so the branch is green against current master. #36512 removes the leak, and the entry has to be deleted in the same change - the allowlist is shrink-only, so a fixed leak keeps failing until its entry goes. The same merge also shrank the AllComponents icon leak: react-icons now survives only through ./teaching-popover, since the tag picker no longer ships a default icon. --- .../library/bundle-isolation.config.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index 1de6c83ea50754..d87e3f0d1ad287 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -4,6 +4,7 @@ "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], "allowedViolations": { - "AllComponents.fixture.js": ["@fluentui/react-icons", "@griffel/core", "@griffel/react"] + "AllComponents.fixture.js": ["@fluentui/react-icons", "@griffel/core", "@griffel/react"], + "TagPicker.fixture.js": ["@griffel/core", "@griffel/react"] } } From 9e558b63332a6fa8015daf660dd9a4e38cd10423 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 6 Aug 2026 10:38:56 +0200 Subject: [PATCH 13/14] fix(verify-bundle-isolation): make fixture keys cross-platform and pin down attribution Addresses review feedback on #36511. findFixtures built keys with the host separator, so on Windows a nested fixture became `nested\C.fixture.js` while the JSON config key is `nested/C.fixture.js`. That mismatch shows up as both halves of a false failure at once: the fixture reads as an unallowlisted regression, and its config entry as an orphan. Keys are now POSIX. The test asserted through the same join() as the implementation, so it could not have caught this; it now asserts the literal. Keeps `ids[0]` in externalImporters rather than matching any id, and adds the two cases that decide it. webpack emits one dependency per specifier, so `import { a, b }` is already two connections and the first id is enough; ids only grow for property reads, where `alpha.beta` yields ["alpha", "beta"]. Matching any id would report that module as an importer of `beta`, which is the same class of false attribution that previously blamed react-avatar and react-menu. Verified the new test fails under the alternative. Also rejects globs in allowedViolations at schema level. `@griffel/*` there used to produce two contradictory findings - a regression for each real package and a stale entry for the pattern - and would let a newly leaked `@griffel/anything` hide behind an entry approved for something else. Debt has to name what it is. --- tools/verify-bundle-isolation/README.md | 8 +++- tools/verify-bundle-isolation/schema.json | 6 ++- .../src/bundle-isolation-plugin.spec.ts | 40 +++++++++++++++++++ .../src/bundle-isolation-plugin.ts | 3 ++ .../src/config.spec.ts | 21 +++++++++- tools/verify-bundle-isolation/src/config.ts | 16 ++++++-- 6 files changed, 86 insertions(+), 8 deletions(-) diff --git a/tools/verify-bundle-isolation/README.md b/tools/verify-bundle-isolation/README.md index ef4f82f58b1546..9b2fc31f6dddac 100644 --- a/tools/verify-bundle-isolation/README.md +++ b/tools/verify-bundle-isolation/README.md @@ -111,7 +111,13 @@ All configured paths are resolved relative to the package root: - `fixturesRoot` is the directory containing bundle-size fixtures. - `externals` lists host-provided modules excluded from the bundle. - `forbiddenPackages` lists exact package names or scoped globs such as `@griffel/*`. -- `allowedViolations` maps fixture paths, relative to `fixturesRoot`, to tolerated forbidden packages. +- `allowedViolations` maps fixture paths, relative to `fixturesRoot` and always with forward slashes, to tolerated + forbidden packages. + +The two lists do not take the same values. `forbiddenPackages` declares intent, so it accepts globs. `allowedViolations` +records what actually leaked, so it takes **exact resolved package names** and rejects globs - `@griffel/*` there would +let a newly leaked `@griffel/anything` hide behind an entry approved for something else. The debt has to name what it +is: `@griffel/core` and `@griffel/react`, separately. `$schema` has to be a workspace-relative path. Editors resolve it against the config file and do not apply Node package resolution, so `@fluentui/verify-bundle-isolation/schema.json` will not work there despite the export map. The export diff --git a/tools/verify-bundle-isolation/schema.json b/tools/verify-bundle-isolation/schema.json index cbba697a39fc71..150dc7052fd16e 100644 --- a/tools/verify-bundle-isolation/schema.json +++ b/tools/verify-bundle-isolation/schema.json @@ -33,14 +33,16 @@ } }, "allowedViolations": { - "description": "Bundle-size fixture paths mapped to forbidden packages tolerated as tracked debt.", + "description": "Bundle-size fixture paths mapped to forbidden packages tolerated as tracked debt. Fixture paths use forward slashes and are relative to fixturesRoot.", "type": "object", "additionalProperties": { "type": "array", "uniqueItems": true, "items": { + "description": "Exact resolved package name. Globs are rejected so a tolerated leak cannot silently cover a new one.", "type": "string", - "minLength": 1 + "minLength": 1, + "pattern": "^[^*]+$" } } } diff --git a/tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts index e0823561c9e8c4..2799011b6b97b8 100644 --- a/tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts +++ b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.spec.ts @@ -114,6 +114,46 @@ describe('BundleIsolationPlugin', () => { expect(report.sourceResolved).toEqual([join(root, 'my-pkg/library/src/index.js')]); }); + + // webpack reports an import as `ids`, where only the first entry names the export. These two + // cases pin that down: matching any id instead would blame `alpha.beta` for importing `beta`. + describe('imported ids', () => { + it('credits every specifier of a multi-specifier import', async () => { + writeFiles(root, { + 'package.json': manifest('test-workspace'), + 'node_modules/forbidden-pkg/package.json': manifest('forbidden-pkg'), + 'node_modules/forbidden-pkg/index.js': `export const alpha = () => Date.now();\nexport const beta = () => Math.random();\n`, + 'my-pkg/package.json': manifest('my-pkg'), + 'my-pkg/lib/index.js': `import { alpha, beta } from 'forbidden-pkg';\nexport const use = () => alpha() + beta();\n`, + 'entry.js': `import { use } from './my-pkg/lib/index.js';\nconsole.log(use());\n`, + }); + + const report = await bundle({ root, packageRoot: join(root, 'my-pkg'), forbiddenPackages: ['forbidden-pkg'] }); + + expect(report.leaks['forbidden-pkg'].exports.map(({ name }) => name)).toEqual(['alpha', 'beta']); + }); + + it('does not treat a property read on an import as an import of that property', async () => { + writeFiles(root, { + 'package.json': manifest('test-workspace'), + 'node_modules/forbidden-pkg/package.json': manifest('forbidden-pkg'), + 'node_modules/forbidden-pkg/index.js': `export const alpha = { beta: () => Date.now() };\nexport const beta = () => Math.random();\n`, + 'my-pkg/package.json': manifest('my-pkg'), + 'my-pkg/lib/uses-beta.js': `import { beta } from 'forbidden-pkg';\nexport const viaImport = () => beta();\n`, + // Reads `.beta` off `alpha`, so its ids are ["alpha", "beta"] without importing `beta`. + 'my-pkg/lib/uses-alpha.js': `import { alpha } from 'forbidden-pkg';\nexport const viaProperty = () => alpha.beta();\n`, + 'my-pkg/lib/index.js': `export * from './uses-beta';\nexport * from './uses-alpha';\n`, + 'entry.js': `import { viaImport, viaProperty } from './my-pkg/lib/index.js';\nconsole.log(viaImport(), viaProperty());\n`, + }); + + const report = await bundle({ root, packageRoot: join(root, 'my-pkg'), forbiddenPackages: ['forbidden-pkg'] }); + const betaImporters = report.leaks['forbidden-pkg'].exports + .filter(({ name }) => name === 'beta') + .flatMap(({ importers }) => importers.map(importer => importer.module)); + + expect(betaImporters).toEqual([join(root, 'my-pkg/lib/uses-beta.js')]); + }); + }); }); function bundle({ diff --git a/tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts index eb26c42a3e1d7a..da4d93192ae52b 100644 --- a/tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts +++ b/tools/verify-bundle-isolation/src/bundle-isolation-plugin.ts @@ -159,6 +159,9 @@ function externalImporters( continue; } + // Only the first id names the import. webpack emits one dependency per specifier, so + // `import { a, b }` is already two connections, while `a.b` is a single one with ids + // ["a", "b"] - matching any id would blame that module for importing `b`. if (importedIds(connection.dependency, moduleGraph)[0] === exportName) { importers.set(origin, connection.originModule); } diff --git a/tools/verify-bundle-isolation/src/config.spec.ts b/tools/verify-bundle-isolation/src/config.spec.ts index f54dbeaac8e39f..5ca6a51f007bb9 100644 --- a/tools/verify-bundle-isolation/src/config.spec.ts +++ b/tools/verify-bundle-isolation/src/config.spec.ts @@ -44,6 +44,18 @@ describe('loadConfig', () => { expect(() => load({ ...valid, knownViolations: {} })).toThrow(/must NOT have additional properties/); }); + it('rejects a glob in allowedViolations, which would silently absorb an unrelated leak', () => { + expect(() => load({ ...valid, allowedViolations: { 'A.fixture.js': ['@griffel/*'] } })).toThrow( + /must match pattern/, + ); + }); + + it('accepts an exact package name in allowedViolations', () => { + expect(load({ ...valid, allowedViolations: { 'A.fixture.js': ['@griffel/core'] } }).allowedViolations).toEqual({ + 'A.fixture.js': ['@griffel/core'], + }); + }); + it('reports the offending path relative to the workspace', () => { expect(() => load({ ...valid, externals: 'react' })).toThrow(/bundle-isolation\.config\.json/); }); @@ -71,7 +83,8 @@ describe('findFixtures', () => { writeFileSync(join(root, 'readme.md'), ''); writeFileSync(join(root, 'nested', 'C.fixture.js'), ''); - expect(findFixtures(root)).toEqual(['A.fixture.js', 'B.fixture.js', join('nested', 'C.fixture.js')]); + // Asserted as a literal rather than via join(), because these become config keys on every platform. + expect(findFixtures(root)).toEqual(['A.fixture.js', 'B.fixture.js', 'nested/C.fixture.js']); }); }); @@ -84,6 +97,12 @@ describe('paths', () => { expect(fixtureOutputPath('A.fixture.js', '/ws/packages/thing')).toBe('/ws/packages/thing/dist/bundle-isolation/A'); }); + it('keeps a nested fixture under its own directory', () => { + expect(fixtureOutputPath('nested/C.fixture.js', '/ws/packages/thing')).toBe( + join('/ws/packages/thing/dist/bundle-isolation/nested/C'), + ); + }); + it('shortens workspace paths for display', () => { expect(relativeToWorkspace('/ws/packages/thing/index.js', '/ws')).toBe('packages/thing/index.js'); }); diff --git a/tools/verify-bundle-isolation/src/config.ts b/tools/verify-bundle-isolation/src/config.ts index cad34ab068c6e5..ee32b657ed5fb6 100644 --- a/tools/verify-bundle-isolation/src/config.ts +++ b/tools/verify-bundle-isolation/src/config.ts @@ -39,10 +39,18 @@ export function findFixtures(fixturesRoot: string): string[] { return []; } - return readdirSync(fixturesRoot, { recursive: true, withFileTypes: true }) - .filter(entry => entry.isFile() && entry.name.endsWith(FIXTURE_SUFFIX)) - .map(entry => join(entry.parentPath, entry.name).slice(fixturesRoot.length + 1)) - .sort(); + return ( + readdirSync(fixturesRoot, { recursive: true, withFileTypes: true }) + .filter(entry => entry.isFile() && entry.name.endsWith(FIXTURE_SUFFIX)) + // Fixture paths become config keys, so they stay POSIX rather than following the host separator. + .map(entry => + join(entry.parentPath, entry.name) + .slice(fixturesRoot.length + 1) + .split(sep) + .join('/'), + ) + .sort() + ); } export function findWorkspaceRoot(startDir: string): string { From 676bad1007d1288bccf510732224f4f153e10c4c Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 6 Aug 2026 11:13:45 +0200 Subject: [PATCH 14/14] chore(react-headless-components-preview): lock in the react-icons fix #36504 merged, isolating the teaching popover dismiss icon and adding a third fixture. Nothing in the headless library imports react-icons any more, so the allowlist entry claiming AllComponents still leaks it went stale and failed the check - the shrink-only rule working as intended, refusing to let a fix go unrecorded. Removes that entry. @fluentui/react-icons now reports as kept out rather than tolerated, across all three fixtures, which also locks it: any future headless component that pulls in an icon fails immediately instead of quietly rejoining the baseline. TeachingPopover.fixture.js needs no entry - it is clean. Griffel is the only forbidden runtime left, reaching every entry point through the portal mount node that ./tag-picker re-exports. #36512 removes it. --- .../library/bundle-isolation.config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index d87e3f0d1ad287..d41f176334152c 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -4,7 +4,7 @@ "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], "allowedViolations": { - "AllComponents.fixture.js": ["@fluentui/react-icons", "@griffel/core", "@griffel/react"], + "AllComponents.fixture.js": ["@griffel/core", "@griffel/react"], "TagPicker.fixture.js": ["@griffel/core", "@griffel/react"] } }