[code-infra] Optimize dependency definition#22006
Conversation
LukasTy
added
dependencies
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Deploy preview: https://deploy-preview-22006--material-ui-x.netlify.app/ Bundle size report
|
LukasTy
force-pushed
the
core/optimize-dep-graph-renovate
branch
from
30c2e9e to
2eef463
Compare
github-actions
bot
removed
the
PR: out-of-date
LukasTy
force-pushed
the
core/optimize-dep-graph-renovate
branch
4 times, most recently
from
43ac0c5 to
52f05d8
Compare
Follow-up on mui#21999. Reduces Renovate's per-PR memory pressure and tightens the dependency graph. - renovate.json: add `ignorePaths: ["**/bug-reproductions/**"]`. The mui-public preset already ignores `examples/`, but `bug-reproductions/` packages pin every dep to `"latest"`, so there is nothing for Renovate to do there. - pnpm-workspace.yaml: hoist `@atlaskit/pragmatic-drag-and-drop`, `@base-ui/react`, `@mui/types`, `execa`, and `jsdom` into the catalog. Each was duplicated with the same pinned version across 2-4 package.json files; moving them to the catalog removes the duplication and cuts per-package Renovate PRs. - Drop `chai-dom` and `@types/chai-dom` from root devDeps. Both are already direct deps of `@mui/internal-test-utils`, so the matchers and type augmentation keep flowing in transitively. Also drop the stale `"chai-dom"` entry from `packages/x-virtualizer/tsconfig.json` (x-virtualizer has no tests at all; the entry was leftover cruft). - Relax the `reactMajor < 19` assertions in `layout.DataGrid`, `slots.DataGrid`, and `TreeViewItemsPlugin` to a substring prefix. `@vitejs/plugin-react` 5.2+ changes JSX scope handling so the inner named function expression no longer gets a `2` suffix in the `<ForwardRef(DataGrid2)>` frame. The prefix match is forward compatible with both the current `ForwardRef(DataGrid2)` form and the future `ForwardRef(DataGrid)` form, so a future `vite` / `vitest` / `@vitejs/plugin-react` catalog bump won't need to touch these tests again.
LukasTy
force-pushed
the
core/optimize-dep-graph-renovate
branch
from
52f05d8 to
6385529
Compare
There was a problem hiding this comment.
Pull request overview
This PR is a code-infra follow-up aimed at reducing Renovate noise and tightening dependency version management across the monorepo by centralizing repeated versions in the pnpm workspace catalog, plus small test expectation tweaks for forward compatibility.
Changes:
- Update
renovate.jsonto ignorebug-reproductions/**so Renovate doesn’t open pointless PRs for “latest”-pinned fixtures. - Hoist several duplicated dependency pins into
pnpm-workspace.yamlcatalog:and convert package.json entries tocatalog:references (plus corresponding lockfile updates). - Remove root
chai-domdeps and clean up a stalechai-domTStypesentry; relax a few<ForwardRef(...)>-name assertions to be resilient across@vitejs/plugin-reactversions.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
renovate.json |
Ignores bug-reproductions/** to reduce Renovate churn. |
pnpm-workspace.yaml |
Adds catalog entries for several previously duplicated pinned deps. |
pnpm-lock.yaml |
Reflects catalog hoists and root dependency cleanup. |
package.json |
Removes chai-dom / @types/chai-dom from root; switches execa/jsdom to catalog:. |
docs/package.json |
Uses catalog for @atlaskit/pragmatic-drag-and-drop. |
packages/x-charts-vendor/package.json |
Uses catalog for execa. |
test/performance-charts/package.json |
Uses catalog for jsdom. |
packages/x-data-grid/package.json |
Uses catalog for @mui/types. |
packages/x-virtualizer/package.json |
Uses catalog for @mui/types. |
packages/x-virtualizer/tsconfig.json |
Removes stale chai-dom from compilerOptions.types. |
packages/x-scheduler/package.json |
Uses catalog for @base-ui/react. |
packages/x-scheduler-premium/package.json |
Uses catalog for @base-ui/react. |
packages/x-scheduler-headless/package.json |
Uses catalog for @atlaskit/pragmatic-drag-and-drop and @base-ui/react. |
packages/x-scheduler-headless-premium/package.json |
Uses catalog for @base-ui/react. |
packages/x-data-grid/src/tests/layout.DataGrid.test.tsx |
Relaxes <ForwardRef(...)> assertion to a prefix match for plugin-react compatibility. |
packages/x-data-grid/src/tests/slots.DataGrid.test.tsx |
Same prefix-match relaxation for <ForwardRef(...)>. |
packages/x-tree-view/src/internals/plugins/items/TreeViewItemsPlugin.test.tsx |
Same prefix-match relaxation for <ForwardRef(...)> across plugin-react versions. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LukasTy
requested review from
JCQuintas,
alexfauquette and
romgrk
as code owners
LukasTy
changed the title
| 'The Data Grid component requires all rows to have a unique `id` property', | ||
| reactMajor < 19 && 'The above error occurred in the <ForwardRef(DataGrid2)> component', | ||
| // `ForwardRef(DataGrid)` on newer `@vitejs/plugin-react`, `ForwardRef(DataGrid2)` on older ones. | ||
| reactMajor < 19 && 'The above error occurred in the <ForwardRef(DataGrid', |
There was a problem hiding this comment.
Adding a displayName to the grid should stabilize this
Janpot
left a comment
Janpot
left a comment
There was a problem hiding this comment.
🙂 Another proof that catalogs are the wrong solution to the problem. There's nothing enforcing them to be used. You now need a linter to verify al these dependencies have the catalog: value. At which point you may as well just verify they have the same value and get rid of the catalog.
| reactMajor >= 19 && | ||
| 'The Data Grid component requires all rows to have a unique `id` property', | ||
| reactMajor < 19 && 'The above error occurred in the <ForwardRef(DataGrid2)> component', | ||
| // `ForwardRef(DataGrid)` on newer `@vitejs/plugin-react`, `ForwardRef(DataGrid2)` on older ones. |
There was a problem hiding this comment.
Do we ever use older plugins? Can't we just use the value without the 2 at the end now? <ForwardRef(DataGrid)>
There was a problem hiding this comment.
We currently use the older plugin, which builds the name with 2.
After upgrading to the latest plugin, it's to without the 2.
So, this prepares for the plugin bump, when other packages are read and don't break when running browser tests.
Are you suggesting ditching the comment altogether?
There was a problem hiding this comment.
No, I was under the impression we were using the newer plugin. It is fine then until we change.
Agreed. There isn't too much benefit to it. In any case, you need diligence or linting to ensure the same version is used in the repo. 🙈
|
|
Not sure if it's temporary, but Renovate was able to run after this got merged: https://developer.mend.io/github/mui/mui-x/-/job/b1cbddb4-6bba-4176-8f4b-ee1fcd33b887. |
Follow-up on #21999.
Further attempt to make life for
Renovateeasier and tighten the dependency graph along the way.Changes
Renovate
renovate.json— added"ignorePaths": ["**/bug-reproductions/**"]. Bothbug-reproductions/x-charts/package.jsonandbug-reproductions/x-data-grid/package.jsonpin every dep to"latest", so there is nothing for Renovate to update there. The mui-public preset already ignores**/examples/**, but notbug-reproductions/**.Catalog hoists
Hoisted into the
pnpm-workspace.yamlcatalog:to remove duplicated pinned versions and shrink Renovate's per-PR diff churn:@atlaskit/pragmatic-drag-and-drop^1.7.7(was duplicated indocs/andpackages/x-scheduler-headless)@base-ui/react^1.3.0(was duplicated across all fourx-scheduler*packages)@mui/types7.4.12(was duplicated inx-data-gridandx-virtualizer)execa9.6.1(was duplicated in root andx-charts-vendor)jsdom26.1.0(was duplicated in root andtest/performance-charts)Cleanup
package.json— droppedchai-domand@types/chai-domfrom root devDependencies. Both are already direct deps of@mui/internal-test-utils, so the runtime matchers and type augmentation keep flowing in transitively.packages/x-virtualizer/tsconfig.json— removed the stale"chai-dom"entry fromcompilerOptions.types.x-virtualizerhas no test files of its own; the entry was leftover cruft from when this code lived inx-data-grid.Robust
ForwardRef(...)test expectationsRelaxed the
reactMajor < 19assertions inlayout.DataGrid.test.tsx,slots.DataGrid.test.tsx, andTreeViewItemsPlugin.test.tsxto match only the component-name prefix instead of the exact<ForwardRef(DataGrid2)> componentstring. See the "vite / vitest catalog bump — investigated, deferred" section below for why.vite/vitestcatalog bump — investigated, deferredThe initial plan for this PR also bumped
vite ^7.3.1 → ^8.0.2,vitest ^4.0.18 → ^4.1.3,@vitest/* → ^4.1.3,@vitejs/plugin-react ^5.1.4 → ^5.2.0, and@vitejs/plugin-react-swc ^4.2.3 → ^4.3.0to align the catalog with@mui/internal-bundle-size-checker, which already pullsvite@^8into the tree. That bump was reverted before merge after CI feedback.Why it was attempted
On master,
@mui/internal-bundle-size-checkerhavingvite@^8as a direct dep means pnpm materializes bothvite@7.3.1(for the catalog) andvite@8.0.2(for bundle-size-checker) in the tree. Any edit to any workspacepackage.jsonthen causespnpm installto re-resolve peers and cascades thevite@8instantiation across ~36 peer-resolved sites, producing an ~80-line lockfile churn and these install-time warnings:Bumping the catalog to
vite@8was meant to consolidate everything on a single major and stop the cascade, making future Renovate runs and PRs much cheaper.What the bump did break
@vitejs/plugin-react@5.1.4 → 5.2.0changes JSX scope handling such that inner named function expressions insideReact.forwardRef(function DataGrid(...) {...})no longer get the2suffix that previously disambiguated them from the outerconst DataGrid. FivereactMajor < 19tests inlayout.DataGrid.test.tsx,slots.DataGrid.test.tsx, andTreeViewItemsPlugin.test.tsxasserted on the exact<ForwardRef(DataGrid2)> componentstring. Fixed by relaxing them to substring-prefix matches — kept in this PR for forward compatibility.vitest@4.1.3+vite@8+ chromium headless browser mode crashed pages repeatedly during the React 19 browser CI job intest/e2eandtest/regressions, with the runner eventually dying on:describeValidation.MobileDateRangePicker.test.tsx, which is one of the larger describe suites we have (~40+ test cases sharing a single long-lived browser page underisolate: false+maxWorkers: 1). React 18 browser CI was unaffected.Why that could not be stabilized in this PR
The vitest/vite upgrade chain is tightly coupled by peer ranges:
@vitest/mocker@4.0.18requiresvite: ^6 || ^7.0.0-0→ forcesvitest@4.0.18onto vite@7@vitest/mocker@4.1.3requiresvite: ^6 || ^7 || ^8→ happy on vite@8So we can't cherry-pick "vite@8 but keep vitest@4.0.18", or vice versa, without overrides. And the root cause of the chromium page crash looks like an existing vitest browser-mode memory accumulation issue that is not specific to this repo:
vite@8beta support in v4.1.1 (the stable vite@8 line is supported but still rocky in combination with chromium browser mode)Workarounds discussed in the vitest tracker (
isolate: false,fileParallelism: false,maxWorkers: 1) are already all enabled invitest.shared.mts, so there was nothing left to tune from our side.Decision
vite/vitest/@vitest/*/@vitejs/plugin-react*catalog bumps. All of those catalog entries stay at their current master values.<ForwardRef(DataGrid2)>naming), so they're a no-op today, but they make the next@vitejs/plugin-reactbump a drop-in with no test changes required.chai-domcleanup, which were the core goal of this PR.Follow-up for when we revisit the vite bump:
@mui/internal-test-utils/@mui/internal-bundle-size-checkerrev that moves everything to a single vite major.vitestbrowser-mode release with better page-lifecycle memory behaviour (the open issues above are being tracked by the vitest team).pnpm-workspace.yamlcatalog entries will need touching.Lockfile impact
pnpm-lock.yaml: +46 lines vs master (+28 / −18),vite@8footprint unchanged at 2 entries (same as master — the 38-entry cascade only shows up mid-install andpnpm dedupecollapses it again).Test plan
pnpm install— clean, only the pre-existing@eslint/js@10vseslint@9andreact-runner@1.0.5vsreact@19peer warnings that are already on masterpnpm typescript— all 23 projects passpnpm eslint— exit 0pnpm prettier— cleanpnpm deduplicate— no lockfile changespnpm dlx --package=renovate renovate-config-validator renovate.json— config validpnpm test:unit --project x-data-grid --project x-tree-view --project x-scheduler --runon React 19 — 1594 passed<ForwardRef(X2)>tests (layout.DataGrid,slots.DataGrid,TreeViewItemsPlugin) pass on both React 18 and React 19