chore(workspace-plugin): harden verify-packaging with export map check and gate attw in CI - #36617
Draft
Martin Hochel (Hotell) wants to merge 3 commits into
Draft
Conversation
Adds an Nx sync generator that keeps package.json entry point fields (main, module, typings) and the exports map in sync with each project's declared entry points, for non-private libraries tagged vNext + platform:web. Motivation: microsoft#36606 had to hand-fix 6 headless subpaths that shipped with the legacy flat shape ({types, node, import, require}, .js CJS paths, no .d.cts) mixed into an otherwise migrated map. The exports map is also the source of truth for generate-api, so a missing subpath silently drops a dts rollup. Entry points cannot be inferred from the file layout, so multi entry projects declare them via project.json#metadata.exportMap. Projects without the declaration default to a single root entry, which is what the other 79 packages already have. Also fixes react-headless-components-preview, whose main/module/typings were dead because its exports map had no "." key.
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.
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -51,6 +51,7 @@ jobs: | |||
| - name: Workspace lint | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Menu Converged - submenuIndicator slotted content 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default - RTL.submenus open.chromium.png | 404 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 745 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 732 | Changed |
vr-tests-react-components/ProgressBar converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 64 | Changed |
vr-tests-react-components/Skeleton converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Skeleton converged.Translucent Skeleton with circle - High Contrast.default.chromium.png | 1 | Changed |
vr-tests-react-components/TagPicker 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - Dark Mode.chromium.png | 658 | Changed |
| vr-tests-react-components/TagPicker.disabled - High Contrast.disabled input hover.chromium.png | 1319 | Changed |
vr-tests-web-components/Avatar 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Avatar. - Dark Mode.normal.chromium_1.png | 298 | Changed |
vr-tests-web-components/TextInput 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/TextInput. - Dark Mode.normal.chromium_1.png | 288 | Changed |
vr-tests/react-charting-LineChart 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-LineChart.Multiple - RTL.default.chromium.png | 200 | Changed |
| vr-tests/react-charting-LineChart.Multiple.default.chromium.png | 192 | Changed |
| vr-tests/react-charting-LineChart.Multiple - Dark Mode.default.chromium.png | 181 | Changed |
vr-tests/react-charting-VerticalBarChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-VerticalBarChart.Basic - Secondary Y Axis.default.chromium.png | 3 | Changed |
There were 2 duplicate changes discarded. Check the build logs for more information.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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|map).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-plugin317 tests / 42 suites 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.