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.

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

[core] Add react-is dependency #18551

Merged
merged 4 commits into from
Nov 25, 2019
Merged

[core] Add react-is dependency #18551

merged 4 commits into from
Nov 25, 2019

Conversation

HeadFox
Copy link
Contributor

@HeadFox HeadFox commented Nov 25, 2019

This dependency is called in Menu.js but is not listed in package.json

I detected this problem when I runned a project with yarn berry (v2) using pnp.
The Menu.js component is using 'isFragment' function from the react-is library.
The only package.json that have react-is is in @material-ui/utils.

This change is also related to the #17317

@oliviertassinari oliviertassinari changed the title Add react-is dependency [core] Add react-is dependency Nov 25, 2019
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Same problem with the lab.

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Thanks! You need to run yarn again in the monorepo. yarn v1 has sometimes issues with yarn workspace * add.

Can't wait for yarn v2 which catches these issues at runtime.

packages/material-ui/package.json Outdated Show resolved Hide resolved
HeadFox added 2 commits November 25, 2019 11:31
This dependency is called in Menu.js but is not listed in package.json
@mui-pr-bot
Copy link

mui-pr-bot commented Nov 25, 2019

No bundle size changes comparing 73fb52a...7311379

Generated by 🚫 dangerJS against 7311379

@HeadFox HeadFox requested a review from eps1lon November 25, 2019 11:17
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

packages/material-ui-lab/package.json needs it as well

@HeadFox
Copy link
Contributor Author

HeadFox commented Nov 25, 2019

packages/material-ui-lab/package.json needs it as well

I also noticed that material-ui-utils is using react 16.8.0 and react-utils 16.8.6. Is this wanted or it's an issue ? @eps1lon

@oliviertassinari
Copy link
Member

@HeadFox It's a great first pull request on Material-UI 👌🏻. Thank you for working on it!

@HeadFox
Copy link
Contributor Author

HeadFox commented Nov 25, 2019

I'm using material-ui since the v1 !
It helped me a lot into improving my react skill with a lot of good practices to follow
I'm so happy to be able to contribute now even with a small fix 😄

@oliviertassinari
Copy link
Member

@HeadFox awesome.

If you could change, add, or remove one thing in the framework, what would it be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants