-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Bump Vite & Vitest to ^4.0.6 (major) #20102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Deploy preview: https://deploy-preview-20102--material-ui-x.netlify.app/ Bundle size report
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
765e2cc to
8970abc
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
8970abc to
1f1d928
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
152b001 to
dee8247
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
d8c5142 to
a82f2c9
Compare
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
package.json
Outdated
| "@types/babel__core": "^7.20.5", | ||
| "@types/babel__traverse": "^7.28.0", | ||
| "@types/chai-dom": "^1.11.3", | ||
| "@types/mocha": "^10.0.10", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our tsconfig depends on the monorepo's, which references types: ["mocha"], so we need to add them as a dev dependency even if we don't use them.
The build was failing because of it.
Building types for /tmp/mui/packages/x-internal-gestures/tsconfig.build.json in /tmp/code-infra-build-tsc-z0IZk8
error TS2688: Cannot find type definition file for 'mocha'.
The file is in the program because:
Entry point of type library 'mocha' specified in compilerOptions
code-infra build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't, just override types in tsconfig.json to types: ["node", "react"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, makes sense 👍
I've added an empty array instead to let the consumers of this tsconfig define the types they need and avoid too many changes.
2373a74 to
7afb818
Compare
| launchOptions: { | ||
| // Required for tests which use scrollbars. | ||
| ignoreDefaultArgs: ['--hide-scrollbars'], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to move this to the shared config now I believe, which would simplify this file 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't used in all files tests, so I didn't want to move it there if we don't use it. Would the other packages benefit from this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably just make the sharedConfig a func that accepts arguments, so we can simplify this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In CI the --hide-scrollbars is always ignored because it is defined here: https://github.com/JCQuintas/mui-x/blob/f8b4a73db5df88af3e443a3176d0af625492b736/scripts/playwrightLaunchServer.mjs#L16
So we probably can drop it here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we run the tests locally? In that case we don't call playwrightLaunchServer, right? In that case, I suppose we would like the tests to be similar to CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is my point, o CI it always runs with that ignore flag, so we should probably move it to the shared config
Yeah I guess that not running them in isolation mode is sharing the same "runner", if each project has a different config, it might causes issues. Though this is theory crafting 🤷
Why would it be specially brittle? Having tests out of isolation is the real problem I guess, but we can't solve that. The vitest@v3 behaviour was to run tests in sequence, so it only makes sense that we define the sequence ourselves. I went all in and added a order for every project(since i suspect this might solve some issues we have had with browser tests), but it is very possible there are only a few offenders that need to be ran on their own. |
I think it's brittle because we each config to define its own order with a number that is global. A collision in numbers could cause tests to fail. It would be nice if we could set the projects order from the root config instead. However, it'll probably only cause issues when adding new projects and it'd probably be caught by CI, so not a blocker for me.
Yeah, but I don't see any mention of that change in the changelog. I'll open an issue in Vitest's repo to understand if this is intentional and left out of the migration guide accidentally or if it's actually a bug. |
|
Vitest bug confirmed: vitest-dev/vitest#8894 (comment) Will subscribe to be warned when it's fixed so we can revive this PR. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
987c1f7 to
9124bfe
Compare
|
The remaining tests are failing because Excerpt of successful tests (3914ec5) |
|
|
| ); | ||
|
|
||
| // Fixes act warning | ||
| await act(() => Promise.resolve()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried using await act(() => clock.runAll()) and await act(() => vi.runAllTimers()) but they didn't work. This was the only solution that I could make work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the timers wouldn't really work here.
I suspect this works because it turns the act flag on until the next loop
b42e537 to
7d4585e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, thanks for taking care of this.
Left some suggestions for improvements/questions
| export default defineConfig({ | ||
| test: { | ||
| environment: 'node', | ||
| maxWorkers: 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this here? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We need maxWorkers to be consistent across all projects, otherwise we'll get an error:
Error: Projects "docs" and "eslint-plugin-mui-x" have different 'maxWorkers' but same 'sequence.groupOrder'.
Provide unique 'sequence.groupOrder' for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok, this is not importing the sharedConfig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly
| 'The above error occurred in the <UseSvgRef> component', | ||
| ]; | ||
| const expectedError = reactMajor < 19 ? errorMessages : errorMessages.slice(0, 2).join('\n'); | ||
| const expectedError = ['The above error occurred in the <UseSvgRef> component']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened to these errors? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the way errors are reported now has changed. I don't understand why. It also seems to have changed in different ways in React 18 vs React 19. I suspect it was the JSDOM integration that changed slightly in v4 and it has caused changes in the way these errors are logged.
This is indeed weird because these errors are being logged as "uncaught" by JSDOM (search here for "Error: Uncaught [Error: MUI X Charts: Could not find the Chart context."), but they're being caught by the ErrorBoundary otherwise the test would fail because it's asserting on caught errors. It seems something has changed in Vitest's integration with JSDOM and now only the final error in the cause chain is being logged.
| ); | ||
|
|
||
| // Fixes act warning | ||
| await act(() => Promise.resolve()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the timers wouldn't really work here.
I suspect this works because it turns the act flag on until the next loop
| launchOptions: { | ||
| // Required for tests which use scrollbars. | ||
| ignoreDefaultArgs: ['--hide-scrollbars'], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In CI the --hide-scrollbars is always ignored because it is defined here: https://github.com/JCQuintas/mui-x/blob/f8b4a73db5df88af3e443a3176d0af625492b736/scripts/playwrightLaunchServer.mjs#L16
So we probably can drop it here too
|
think we are good to go |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
7b184f9 to
c343d7d
Compare
c343d7d to
7c23db6
Compare
5205191 to
fe250c6
Compare
1f23372 to
776f446
Compare
This PR contains the following updates:
^3.2.4->^4.0.6^3.2.4->^4.0.6^3.2.4->^4.0.63.2.4->4.0.6Release Notes
vitest-dev/vitest (@vitest/browser)
v4.0.6Compare Source
🐞 Bug Fixes
isolateandfileParallelismare false - by @sheremet-va in #8889 (31706)FormDatatoRequest- by @sheremet-va in #8880 (197ca)View changes on GitHub
v4.0.5Compare Source
🐞 Bug Fixes
ssr.noExternalwhen externalizing dependencies, fix Svelte and Astro - by @sheremet-va in #8862 (a4f86)Locatortype in selectOptions element parameter - by @rzzf and @sheremet-va in #8848 (7ee28)getBuiltinsunconditionally - by @sapphi-red in #8863 (0e858)groupIdtogroupOrderin error message - by @Yohannfra in #8856 (b9aab)🏎 Performance
--no-isolate --maxWorkers=1- by @AriPerkkio in #8835 (584aa)View changes on GitHub
v4.0.4Compare Source
🐞 Bug Fixes
node:prefix - by @sheremet-va in #8829 (06208)MaxListenersExceededWarning- by @AriPerkkio in #8820 (d1bff)stdioto logger - by @AriPerkkio in #8809 (fb95f)vi.mockedutility - by @sheremet-va in #8839 (f8756)isolate: false- by @AriPerkkio in #8821 (573dc)🏎 Performance
View changes on GitHub
v4.0.3Compare Source
🐞 Bug Fixes
View changes on GitHub
v4.0.2Compare Source
🐞 Bug Fixes
length- by @sheremet-va in #8778 (d4c2b)restoreMocksandmockResetare set in the config - by @sheremet-va in #8781 (2eedb)View changes on GitHub
v4.0.1Compare Source
🐞 Bug Fixes
getBuiltinscheck - by @sheremet-va in #8765 (81000)View changes on GitHub
v4.0.0Compare Source
Vitest 4.0 is out!
To stay updated, read our blog post and check the migration guide.
🚨 Breaking Changes
'basic'reporter - by @AriPerkkio in #7884 (82fcf)vitest/nodeexports - by @sheremet-va in #8197 (dc848)vitest/nodeinstead - by @sheremet-va in #8200 (1e60c)workspaceoption in favor ofprojects- by @sheremet-va in #8218 (76fb7)--standalonewhen CLI filename filter is used - by @AriPerkkio in #8262 (013bf)minWorkersand set it automatically to 0 in non watch mode - by @sheremet-va in #8454 (2c2d1)treereporter - by @sheremet-va and @AriPerkkio in #8500 (25fd3)tinypool- by @AriPerkkio and @sheremet-va in #8705 (4822d)todoif no function is passed down totestordescribe- by @sheremet-va in #8346 (1a81c)🚀 Features
onUnhandledErrorcallback - by @sheremet-va in #8162 (924cb)expect.assertfor type narrowing - by @sheremet-va in #8695 (fe589)displayAnnotationsoption togithub-options- by @sheremet-va in #8706 (4a66d)experimental_parseSpecifications- by @sheremet-va in #8408 (fdeb2)enableCoverageanddisableCoveragemethods - by @sheremet-va and @AriPerkkio in #8412 (61eb7)getGlobalTestNamePatternmethod - by @sheremet-va in #8438 (bdb70)relativeModuleIdtoTestModule- by @sheremet-va in #8505 (3be09)getSeedmethod - by @sheremet-va in #8592 (438c4)toBeInViewportutility method to assert element is in viewport or not - by @Shinyaigeek in #8234 (ceed5)vitest initcli command - by @thejackshelton in #8330 (1638b)toMatchScreenshotfor Visual Regression Testing - by @macarie in #8041 (d45f9)trackUnhandledErrorsoption - by @sheremet-va in #8386 (c0ec0)lengthproperty to locators,toHaveLengthnow accepts locators - by @sheremet-va in #8512 (2308c)optionsonBrowserProviderOption- by @sheremet-va in #8609 (0d0e5)--inspectoption in webdriverio - by @sheremet-va in #8613 (38adc)autoUpdateto support percentage formatting - by @Battjmo and @AriPerkkio in #8456 (99e01)toBeNullableexpect function to check provided value is nullish - by @Shinyaigeek and @sheremet-va in #8294 (eeec5)automockerentry - by @sheremet-va in #8301 (e9c92)🐞 Bug Fixes
test.extend- by @AriPerkkio in #8278 (43977)--changedflag support tovitest listcommand - by @haakonjackfloat in #8270 and #8272 (e71a5)browser- by @sheremet-va in #8334 (0417a)oxcinstead ofesbuildonrolldown-vite- by @hi-ogawa in #8378 (e922e)stacksproperty in Node.js context - by @sheremet-va in #8392 (b825e)import.meta.resolveon Vite 7 - by @hi-ogawa in #8493 (549d3)expect.pollassertion fails - by @sheremet-va in #8483 (fb450)useFakeTimersis called multiple times - by @sheremet-va in #8504 (ed7e3)expect.extendmatchers - by @lzl0304 in #8520 (96945)globalSetupfiles - by @sheremet-va in #8534 (8978a)$and%formatting totest.for/eachtitle - by @hi-ogawa in #8557 (ea6d7)"./*"with specific files in vitest package - by @hi-ogawa in #8560 (ce746)optimizeDeps.includefor browser mode - by @jake-danton in #8570 (cdcf7)enginesfield to drop Node 18 support - by @sheremet-va in #8608 (9a0bf)baseoption doesn't crash vitest - by @sheremet-va in #8760 (9f0ec)locator.element()returnsHTMLElementorSVGElement- by @sheremet-va in #8440 (c1ac1)vitedirectly - by @sheremet-va in #8541 (d7fca)not.toBeInTheDocument()- by @sheremet-va in #8751 (f5d06)objectContainingexpect utility to have more compatibility to jest's one - by @Shinyaigeek in #8241 (480be)--projectfilter - by @gtbuchanan in #7885 (761be)vitest:coverage-transformplugin - by @AriPerkkio in #8477 (ff517)coverage.exclude- by @sheremet-va in #8731 (c9c30)regexpHoistableto allow whitespace before parentheses - by @cszhjh in #8231 (a0f9a)resolvedSourcescorrectly - by @sheremet-va in #8736 (8fc52)vitest --standalone- by @AriPerkkio in #8248 (37cc2)config.includeoption withconfig.browser.instances[].includeoption if it is specified - by @Shinyaigeek in #8260 (010fc)🏎 Performance
process.title- by @sheremet-va in #8453 (0a766)@vitest/expect- by @sheremet-va in #8461 (cc98c)workerIdfrom a global object - by @sheremet-va in #8507 (46b13)meta.resolveflag instead of a custom loader - by @sheremet-va in #8567 (2e063)View changes on GitHub
Configuration
📅 Schedule: Branch creation - Between 12:00 AM and 04:59 AM, only on Monday ( * 0-4 * * 1 ) in timezone UTC, Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about these updates again.
This PR was generated by Mend Renovate. View the repository job log.