Skip to content

[core] Extend StrictMode compatiblity #16283

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
Merged
merged 5 commits into from
Jun 21, 2019
Merged

[core] Extend StrictMode compatiblity #16283

merged 5 commits into from
Jun 21, 2019

Conversation

@eps1lon
Copy link
Member

@eps1lon eps1lon commented Jun 18, 2019 •
edited by oliviertassinari
Loading

Tested with locally linked https://github.com/mui-org/react-transition-group/pull/2

TODO:

  • lab
  • publish and use @material-ui/react-transition-group

Closes 13394
Closes #16273

Future work:

@eps1lon eps1lon added package: lab Specific to the lab. package: core labels Jun 18, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Jun 18, 2019 •
edited
Loading

@material-ui/core: parsed: +0.15% , gzip: +0.11%

Details of bundle changes.

Comparing: bd65310...c12974f

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.15% 🔺 +0.11% 🔺 320,020 320,513 88,360 88,461
@material-ui/core/Paper 0.00% 0.00% 68,278 68,278 20,363 20,363
@material-ui/core/Paper.esm 0.00% 0.00% 61,573 61,573 19,161 19,161
@material-ui/core/Popper 0.00% -0.04% 28,945 28,945 10,395 10,391
@material-ui/core/Textarea 0.00% +0.08% 🔺 5,513 5,513 2,374 2,376
@material-ui/core/TrapFocus 0.00% 0.00% 3,753 3,753 1,578 1,578
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,009 16,009 5,791 5,791
@material-ui/core/useMediaQuery 0.00% 0.00% 2,597 2,597 1,102 1,102
@material-ui/lab +0.21% 🔺 +0.19% 🔺 140,001 140,293 43,388 43,470
@material-ui/styles 0.00% 0.00% 51,698 51,698 15,348 15,348
@material-ui/system 0.00% 0.00% 15,420 15,420 4,391 4,391
Button +0.16% 🔺 +0.18% 🔺 84,296 84,430 25,712 25,758
Modal 0.00% -0.02% 14,427 14,427 5,087 5,086
Portal 0.00% -0.06% 3,473 3,473 1,572 1,571
Slider 0.00% -0.02% 74,701 74,701 23,248 23,243
colorManipulator 0.00% 0.00% 3,904 3,904 1,544 1,544
docs.landing 0.00% 0.00% 55,119 55,119 13,940 13,940
docs.main +0.09% 🔺 +0.14% 🔺 648,452 649,040 204,510 204,806
packages/material-ui/build/umd/material-ui.production.min.js +0.15% 🔺 +0.16% 🔺 293,225 293,675 84,237 84,370

Generated by 🚫 dangerJS against c12974f

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 18, 2019 •
edited
Loading

100% strict mode compliance or do we still miss a few components (not taking the styles package into account 🙃)? Let me look at that :D.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 18, 2019

Ok, I was getting too excited for no reason. We still have a few problems. For instance, the enzyme 's .simulate() API seems to break it. It stops the Slider test to run with strict: true.

@eps1lon eps1lon mentioned this pull request Jun 19, 2019
Closed
1 task
@eps1lon eps1lon added this to the 16.8 React.StrictMode milestone Jun 19, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 21, 2019 •
edited
Loading

Closes #13394

Would it be better to address the tests issues in a different pull request? Solving #16273 is already a significant step :).

@eps1lon
Copy link
Member Author

eps1lon commented Jun 21, 2019

Closes #13394

Would it be better to address the tests issues in a different pull request? Solving #16273 is already a significant step :).

Yeah the a11y issues have priority for now which will eventually get rid of simulate and other problematic enzyme methods but this is good to go.

@eps1lon eps1lon marked this pull request as ready for review June 21, 2019 07:31
eps1lon
eps1lon commented Jun 21, 2019
View reviewed changes
packages/material-ui/src/Backdrop/Backdrop.test.js
skip: ['componentProp'],
}));

it('should render a backdrop div', () => {
Copy link
Member Author

@eps1lon eps1lon Jun 21, 2019

Choose a reason for hiding this comment

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

Already tested in describeConformance

@oliviertassinari oliviertassinari merged commit 6957c91 into mui:master Jun 21, 2019
@eps1lon eps1lon deleted the feat/rtg-strict branch June 21, 2019 08:24
This was referenced Jun 24, 2019
Closed
Merged
@oliviertassinari oliviertassinari added internal Behind-the-scenes enhancement. Formerly called “core”. and removed package: core package: lab Specific to the lab. labels Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees

No one assigned

Labels

internal Behind-the-scenes enhancement. Formerly called “core”.

Projects

None yet

Milestone

React.StrictMode

Development

Successfully merging this pull request may close these issues.

Uncaught Invariant Violation: Unable to find node on an unmounted component error occurred in the <Transition> component Warnings in strict mode

3 participants

@eps1lon @mui-pr-bot @oliviertassinari