Repository navigation
chore(workspace-plugin): harden verify-packaging with export map check and gate attw in CI - #36617
Open
Martin Hochel (Hotell) wants to merge 5 commits into
Open
Martin Hochel (Hotell) wants to merge 5 commits into
Martin Hochel (Hotell) wants to merge 5 commits into
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Generates bundle-size fixtures from source so they cannot silently fall behind what a package actually exports. Only fixtures declared in project.json#metadata.bundleSizeFixtures are generated; hand written monosize fixtures in the same folder are left alone. entryPoints kind namespace imports every non-root export subpath, resolved from metadata.exportMap rather than package.json#exports so a drifted export map cannot hide behind a matching drifted fixture. This immediately caught AllComponents.fixture.js missing menu-button, positioning and utils. baseHooks kind named imports every use*Base_unstable hook exported by the project's workspace dependencies, enumerated by walking each dependency's src/index.ts with the TypeScript parser. Source is used rather than the etc/*.api.md rollups because api.md is generated by generate-api, which itself reads package.json#exports - deriving fixtures from it would reintroduce the coupling this harness exists to break. Regenerating BaseHooks.fixture.js reproduces the hand maintained file byte for byte.
Adds an export map cross-check to verify-packaging: every file referenced by any condition in package.json#exports must appear in the npm pack file list. The existing assertions only matched broad globs and never looked at the export map, so an entry pointing at a file that was never built passed unnoticed while resolving to nothing at runtime. Paths under src/ are skipped because dev only conditions (eg. ./__dev) resolve to source, which is already asserted as never shipped. Widens verify-packaging from a hardcoded two project include list to every non private v9 library - 6 projects to 91, all passing. Fixes the ships-cjs assertion, which used `lib-commonjs/**/*.(js|map)` and so never matched a `.cjs` file - after the native ESM migration it was only ever passing because sourcemaps ship alongside. Scopes the attw target to esm first packages and adds it to the affected CI gate. CommonJS-first packages always report inherent `CJS default export` interop findings under the node16 profile, so gating them would be permanently red. All 80 esm first packages pass. Also fixes tag leakage in the verify-packaging spec, which pushed onto a shared context mock so tags accumulated across tests.
Martin Hochel (Hotell)
force-pushed
the
chore/verify-packaging-export-map
branch
from
October 9, 2026 09:12
7856edc to
f945329
Compare
Martin Hochel (Hotell)
marked this pull request as ready for review
October 9, 2026 11:56
Tudor Popa (tudorpopams)
requested a review
from Paul Mardling (PaulGMardling)
October 12, 2026 06:49
Paul Mardling (PaulGMardling)
approved these changes
Oct 12, 2026
Paul Mardling (PaulGMardling)
left a comment
Paul Mardling (PaulGMardling)
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me!
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previous Behavior
verify-packagingasserted on broad globs (lib/**/*.js,dist/*, …) but never cross-referenced the export map. An entry declaring a file that was never built passed unnoticed while resolving to nothing at runtime.It also only ran for 2 projects (
react-text,react-components), andattw— already inferred by the workspace plugin — was explicitly not a CI gate.Two latent defects surfaced while working on this:
ships cjsassertion has been vacuous since the native ESM migration. It usedlib-commonjs/**/*.(js|map), which never matches a.cjsfile. It only passed because sourcemaps (index.cjs.map) ship alongside — so it was effectively asserting "ships cjs sourcemaps". Any package built without maps would have failed spuriously.executor.spec.tspushed tags onto a shared context mock, sov8/ships-amd/ships-bundleaccumulated across tests.New Behavior
Export map cross-check
Every file referenced by any condition in
package.json#exportsmust appear in thenpm pack --dry-runfile list.Paths under
src/are skipped: dev-only conditions (e.g../__devinbabel-preset-storybook-full-source) resolve to source, which the executor already asserts is never shipped — checking them would contradict an existing rule.This is the complement to the sync generators in #36615 / #36616, which cannot cover it: those are pure Tree operations that run before the build, while this needs built output and
npm pack. Sync answers "is the declaration correct?"; this answers "did the declared files actually get built and shipped?".Wider, cheaper scope
verifyPackaging.includenow covers the critical packages rather than 2:react-components,react-headless-components-preview,react-utilities,react-button,react-chartsverify-packagingalso moves from thedefaultnamed input toproduction, so test-only PRs no longer re-pack.productionisdefaultminus test files, so this is a strict narrowing.attwis now a CI gatebuildAttwTargetis gated onpackageJSON.type === 'module'. CommonJS-first packages always report inherent🤨 CJS default exportinterop findings under thenode16profile, so gating them would be permanently red. This self-maintains — a package becomes gated the moment it migrates to ESM-first.attwis added to the affected target list inpr.yml. 80 ESM-first packages, all green.Also fixed
ships cjspattern →lib-commonjs/**/*.(js|cjs)(sourcemaps alone no longer satisfy the assertion).executor.spec.ts(assigns instead of pushes).Verification
src/conditions.babel-preset-storybook-full-source, the./__devcase) — now handled; all covered projects pass.productioninput verified both directions: touchingButton.test.tsx→ full cache hit (15/15); touchingButton.tsx→ correctly invalidated (13/15).nx run-many -t attw→ 80/80 green.workspace-pluginrun: 317 tests / 42 suites green. Final combined-stack run: 385 tests / 44 suites / 225 snapshots green.Performance
npm pack --dry-run, per packagenpm packcost is dominated by subprocess startup rather than package size, which is why the project count matters more than package size.Related Issue(s)
Note
Third of a 3 PR stack, branched off #36616 — earlier commits will show here until those merge.
Review Readiness Update
Updated head:
1c17895cdc4ad9cdf848e15bf40fdd0e1bb32d12.master.noneBeachball metadata fixes without duplicating them.workspace-plugin:type-check, build/lint/graph/formatting,nx sync:check, Beachball, and production-input cache isolation checks. The final combined stack passed all five critical packaging targets, all 80 affectedattwtargets, and the full plugin suite (385 tests / 44 suites / 225 snapshots).CI follow-up: the initial restacked run exposed TS2353 in the new unmatched-wildcard regression fixture. The one-line repair uses the existing typed
importcondition;workspace-plugin:type-checkand all 13 executor tests passed afterward. The earlier React 17 SwatchPicker focus timeout was in unchanged source; React-major integration and E2E both passed naturally on the final head. No gate was bypassed.Reports For Owner Review
react-headless-components-previewentire-library increases of 340 B minified / 139 B gzip. Its content was verified against themonosize-reportartifact from the successful exact-head bundle-size run. This combined-stack delta remains visible for owner review and has not been independently attributed to the packaging change.Ready for review, not merged. #36616 must still land before this stack's final PR.