Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[test] Ensure compatibility with React 18 RC 1 #31263

Closed
wants to merge 6 commits into from
Closed

[test] Ensure compatibility with React 18 RC 1 #31263

wants to merge 6 commits into from

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Mar 1, 2022 •
edited
Loading

@eps1lon eps1lon added the test label Mar 1, 2022
@eps1lon eps1lon added this to the React 18 milestone Mar 1, 2022
@mui-bot
Copy link

mui-bot commented Mar 1, 2022 •
edited
Loading

Details of bundle changes

@material-ui/core: parsed: -0.01% 😍, gzip: -0.08% 😍
@material-ui/lab: parsed: -0.01% 😍, gzip: -0.09% 😍
@material-ui/styles: parsed: -0.09% 😍, gzip: -0.76% 😍
@material-ui/system: parsed: -0.08% 😍, gzip: -0.52% 😍
@material-ui/utils: parsed: -1.24% 😍, gzip: -3.21% 😍
@mui/material-next: parsed: -0.04% 😍, gzip: -0.28% 😍

Generated by 🚫 dangerJS against 031f99d

eps1lon
eps1lon commented Mar 1, 2022
View reviewed changes
packages/mui-material/src/Avatar/Avatar.test.js Outdated
Comment on lines 44 to 47
expect(img).to.have.attribute('src', '/fake.png');
const expectedSrc = /jsdom/.test(window.navigator.userAgent)
? '/fake.png'
: new URL('/fake.png', window.origin).toString();
expect(img).to.have.property('src', expectedSrc);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably caused by facebook/react#23316. Not clear to me if both values are interchangeable and if JSDOM is not correct here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting, will fix this in React

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix will be included in the next RC and is available on main

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Will try it out to confirm.

@eps1lon eps1lon marked this pull request as ready for review March 1, 2022 20:54
@gnoff gnoff mentioned this pull request Mar 1, 2022
Merged
@eps1lon eps1lon added the on hold There is a blocker, we need to wait label Mar 2, 2022
@eps1lon
Copy link
Member Author

eps1lon commented Mar 2, 2022

PR should not be merged yet until #31263 (comment) is resolved

@eps1lon
Copy link
Member Author

eps1lon commented Mar 2, 2022

React 18 pipeline is now green without any changes to runtime or tests 💪🏻

@eps1lon eps1lon closed this Mar 2, 2022
@eps1lon eps1lon deleted the test/react-18 branch March 2, 2022 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnoff gnoff gnoff left review comments

Assignees
No one assigned
Labels
on hold There is a blocker, we need to wait test
Projects
None yet
Milestone
Leverage React 18
Development

Successfully merging this pull request may close these issues.
3 participants
@eps1lon @mui-bot @gnoff