fix: remove unused react-dom peerDependencies from react-components packages - #36559
Draft
Martin Hochel (Hotell) wants to merge 3 commits into
Draft
Conversation
All v9 packages depend on `@fluentui/react-jsx-runtime`, which imports `react/jsx-runtime` directly. That entry point was only backported to the 16.x line in react@16.14.0 (react@16.13.1 ships no jsx-runtime files), so `react >=16.14.0` is a hard floor. 12 packages advertised a `>=16.8.0` lower bound on some subset of react/react-dom/@types/react/@types/react-dom, which is unsatisfiable in practice. The skew originated in microsoft#31937, which reverted a bad release that wrote `"react": "^8.119.0"` into peerDependencies but only restored the `react` key, leaving the other three behind. microsoft#35145 then swept `<19.0.0` -> `<20.0.0` while preserving the existing lower bounds, freezing the skew in place. All publishable `packages/react-components/**` packages now use: "@types/react": ">=16.14.0 <20.0.0" "@types/react-dom": ">=16.9.0 <20.0.0" "react": ">=16.14.0 <20.0.0" "react-dom": ">=16.14.0 <20.0.0" `@types/react-dom` stays at `>=16.9.0` because no 16.14.x was ever published for it (16.x ends at 16.9.25) - see microsoft#30259. Also updates the `react-library` generator template, which still emitted `<19.0.0` because microsoft#35145 did not touch `tools/`, so every newly scaffolded package started life with a stale upper bound. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Yarn records workspace peerDependencies in the lockfile, so changing them in package.json makes yarn.lock stale and `yarn install --immutable` fails with YN0028 in CI. Regenerated via `yarn install --mode=update-lockfile`. The diff is 35 lines, all of them react/react-dom/@types peer ranges on the 12 realigned workspace entries - no resolution, version or checksum changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ackages Follow-up to microsoft#36551, which aligned the *ranges* of the four react peers but did not question whether they should be declared at all. 76 packages declared `react-dom` and/or `@types/react-dom` peers. Only 7 import `react-dom` in shipped code (verified against both source imports and built `lib/` output; `react-positioning` and `react-storybook-addon-export-to-sandbox` match only in a comment URL and in sandbox template strings respectively). `@types/react-dom` is dropped everywhere: it appears in 0 of the 290 checked-in `api.md` reports and 0 of the 150 `dist/` rollups, so it never forms part of the published type contract. The peer set is part of a package's virtual instance key, so declaring a peer that is never imported forks the virtual store on a dimension the package does not care about. Measured with the published react-tabster tarball under `nodeLinker: pnp` + `pnpMode: strict`, across two workspaces with identical `react@18.3.1` differing only in react-dom (18.3.1 vs 18.2.0): peers as shipped -> 2 virtual instances react-dom removed -> 1 virtual instance Removal is also strictly safer under strict resolution: the current manifests emit `YN0002 doesn't provide react-dom` / `@types/react-dom` for a module that is never required. Repo-wide, `yarn install` peer warnings drop from 533 to 425. `@fluentui/react-components` keeps its `react-dom` peer: it re-exports Portal, so the transitive runtime requirement is real. `@fluentui/react-conformance-griffel` had the inverse bug - it declared `@types/react-dom` but not `react-dom`, while doing `await import('react-dom')` and `await import('react-dom/client')` at runtime. Those are swapped. Also updates the `react-library` generator template so newly scaffolded packages no longer start life with the unused peers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📊 Bundle size report✅ No changes found |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
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 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default - RTL.submenus open.chromium.png | 599 | Changed |
| vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default.submenus open.chromium.png | 605 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 847 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 723 | Changed |
vr-tests-react-components/ProgressBar converged 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 165 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png | 63 | Changed |
vr-tests-react-components/TagPicker 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - High Contrast.chromium.png | 1319 | Changed |
| vr-tests-react-components/TagPicker.disabled - RTL.disabled input hover.chromium.png | 635 | Changed |
| vr-tests-react-components/TagPicker.disabled.disabled input hover.chromium.png | 677 | Changed |
vr-tests-web-components/Accordion 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Accordion. - Dark Mode.normal.chromium_1.png | 3398 | Changed |
vr-tests-web-components/Badge 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Badge. - Dark Mode.normal.chromium.png | 443 | 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 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-LineChart.Events.default.chromium.png | 1 | Changed |
vr-tests/react-charting-SankeyChart 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-SankeyChart.PlaceHolder - RTL.default.chromium.png | 77 | Changed |
| vr-tests/react-charting-SankeyChart.PlaceHolder.default.chromium.png | 78 | 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 3 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.
Important
Stacked on #36551 — please merge that one first. Until it does, the diff here
also contains its 2 commits. The commit unique to this PR is
fix: remove unused react-dom peerDependencies from react-components packages.GitHub can't target the base branch directly because it lives in the fork, hence
master.Previous Behavior
76 packages under
packages/react-components/**declaredreact-domand/or@types/react-dominpeerDependencies. Only 7 of them actually importreact-dom.#36551 aligned the ranges of those peers, but deliberately did not question whether
they should be declared at all. This PR does.
Two concrete costs:
key, so declaring a peer that is never imported forks the virtual store on a
dimension the package doesn't care about. This is the reported issue under
yarn/midgard strict.
YN0002 doesn't provide react-domfor a module that is never required.New Behavior
@types/react-dompeer: 76 → 0react-dompeer: 75 → 8@types/react-domis removed everywhere. It appears in 0 of the 290 checked-inapi.mdreports and 0 of the 150dist/rollups, so it has never been part of thepublished type contract.
react-domis kept only where it is genuinely imported at runtime — verified againstboth source imports and built
lib/output:react-portalReactDOM.createPortalreact-comboboxReactDOM.unstable_batchedUpdatesreact-tag-pickerReactDOM.unstable_batchedUpdatesreact-treeReactDOM.unstable_batchedUpdatesdeprecated/react-virtualizerflushSynctheme-designer(private)react-dom/clientreact-componentsPortal, so the transitive runtime requirement is realreact-conformance-griffelawait import('react-dom')+react-dom/clientTwo matches were confirmed as false positives and correctly get no peer:
react-positioning(areact-dompath inside a comment URL) andreact-storybook-addon-export-to-sandbox(react-dominside generated sandboxtemplate strings).
Bug fixed along the way
@fluentui/react-conformance-griffelhad the inverse problem — it declared@types/react-dombut notreact-dom, while doingawait import('react-dom')andawait import('react-dom/client')at runtime. Those are now swapped.Generator template
tools/workspace-plugin/src/generators/react-library/files/package.json__tmpl__nolonger emits the unused peers, so newly scaffolded packages don't reintroduce them.
Evidence
Measured with the published
@fluentui/react-tabster@9.26.16tarball — identical code,only the manifest differing — published to a local registry and installed under
nodeLinker: pnp+pnpMode: strict. Two workspaces with identicalreact@18.3.1,differing only in
react-dom(18.3.1 vs 18.2.0):Removal is also strictly safer. Variant with peers removed, installed with no
react-dompresent at all, then required:The current shipped manifest in the same app instead emits:
Repo-wide,
yarn installpeer warnings drop from 533 → 425 (baseline measured bystashing and reinstalling).
Validation
nx run-many -t testforreact-provider, react-motion, react-table, react-portal—492 passed / 25 suites. These are the packages that import
react-dom/serverandreact-dom/clientin*-node.test/*-hydrate.test; they resolve via root hoisting,consistent with the repo convention that no package declares react devDeps (0 of 207).
nx run-many -t buildfor 7 projects + 65 dependencies — cleanapi.mddiffs — public API surface unchangedyarn syncpack list-mismatches— exit 0yarn check:change— exit 0Change files:
patchfor 67 publishable packages,type: nonefor the 4 frozen ones(
global-context,deprecated/react-alert,deprecated/react-infobutton,deprecated/react-virtualizer) which disallowpatch. The 5 private packages get none.Out of scope
scripts/perf-test-flamegrillalso carries unusedreact-dompeers (plus stale<19.0.0bounds), but it's a private script package outsidepackages/react-components/**— the same scope boundary #36551 drew. Worth a separate cleanup.
Related Issue(s)