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 @mui/system to install docs to avoid missing peer dependency #14832

Closed
markedwards opened this issue Oct 5, 2024 · 10 comments
Closed
Labels
core Infrastructure work going on behind the scenes docs Improvements or additions to the documentation

Comments

@markedwards
Copy link

markedwards commented Oct 5, 2024

Summary

According to #11128, @mui/system is expected to be explicitly installed. However, it seems to have never been added to the installation instructions.

Is it expected to be installed? If not, why the missing peer dependency warning?

Examples

yarn install v1.22.22
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning "workspace-aggregator-9da3a97e-f777-46ee-947f-e0a57a0e83c9 > platform > @mui/[email protected]" has unmet peer dependency "@mui/system@^5.15.14 || ^6.0.0".

Motivation

No response

Search keywords: @mui/system

@markedwards markedwards added new feature New feature or request status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 5, 2024
@LukasTy
Copy link
Member

LukasTy commented Oct 7, 2024

Hello, could you clarify the use case you are referring to? 🤔
@mui/system is explicitly installed by @mui/material:
https://github.com/mui/material-ui/blob/be78d8700d0f2ce3cc5b403714756bc025402389/packages/mui-material/package.json#L45

Please provide a minimal reproduction test case with the latest version. This would help a lot 👷.
A live example would be perfect. This guide should be a good starting point. Thank you!

@LukasTy LukasTy added status: waiting for author Issue with insufficient information core Infrastructure work going on behind the scenes and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer new feature New feature or request labels Oct 7, 2024
@markedwards
Copy link
Author

I can create a minimal repro, but it's simply adding the dependencies and seeing the above warning. Is what #11128 says about moving to explicitly installing @mui/system no longer valid?

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Oct 7, 2024
@michelengelen michelengelen changed the title Add @mui/system to install docs to avoid missing peer dependency [pickers] Add @mui/system to install docs to avoid missing peer dependency Oct 7, 2024
@LukasTy
Copy link
Member

LukasTy commented Oct 7, 2024

Is what #11128 says about moving to explicitly installing @mui/system no longer valid?

It is no longer valid because there was a recent addition to the @mui/system, which forces it to be a singleton. 😉

@LukasTy LukasTy added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 7, 2024
@markedwards
Copy link
Author

markedwards commented Oct 7, 2024

So if I go to Stackblitz to make a minimal repro, and just update the package.json to look like this:

{
  "name": "parigqolwr.github",
  "version": "0.0.0",
  "private": true,
  "dependencies": {
    "@emotion/cache": "^11.11.0",
    "@emotion/react": "^11.11.4",
    "@emotion/styled": "^11.11.5",
    "@mui/icons-material": "^6.0.2",
    "@mui/material": "^6.0.0",
    "@mui/x-date-pickers": "^7.11.1",
    "react": "18.3.1",
    "react-dom": "18.3.1",
    "@types/react": "18.3.11",
    "@types/react-dom": "18.3.0"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
    "eject": "react-scripts eject"
  },
  "devDependencies": {
    "react-scripts": "latest"
  }
}

It complains about the missing @mui/system dependency:
Image

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Oct 7, 2024
@markedwards
Copy link
Author

Is there something I can do to follow this up? The above is the best repro I can offer. You see the issue when trying to even set up a repro on Stackblitz.

@LukasTy
Copy link
Member

LukasTy commented Oct 21, 2024

@markedwards Stackblitz doesn't support optional peer dependencies. 🙈

In regards to your peer dependency message, it seems setup dependent.
Using pnpm such warning is not shown as it correctly resolves that the peer dependency requirement is met by the transitive dependency from @mui/material, whereas yarn classic fails to do it. 🙈
We could add a note in the installation documentation that in case @mui/system unmet peer dependency issue is shown, such dependency can be added to the list of direct dependencies to resolve this warning.

Does it answer your question? 🤔

@markedwards
Copy link
Author

Perhaps a dumb question, but would it make sense to just have @mui/x-... depend on @mui/system? What does the peer relationship achieve?

In any case, yes my question is definitely answered, thanks.

@LukasTy
Copy link
Member

LukasTy commented Oct 21, 2024

Perhaps a dumb question, but would it make sense to just have @mui/x-... depend on @mui/system? What does the peer relationship achieve?

No questions are dumb. 🙈
Peer dependency helps us avoid the case of multiple @mui/system dependencies being installed (i.e., when Pickers are v7 and @mui/material is v6) and potentially causing issues with usages, which depend on React context or anything else that depends on a singleton. 😉

@LukasTy LukasTy added docs Improvements or additions to the documentation and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 21, 2024
@markedwards
Copy link
Author

Yeah, I figured that was the reasoning, but I wanted to confirm.

I think I will just remove the @mui/system dependency and tolerate the warning, but its good to know these details.

Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@markedwards How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

@LukasTy LukasTy changed the title [pickers] Add @mui/system to install docs to avoid missing peer dependency [core] Add @mui/system to install docs to avoid missing peer dependency Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes docs Improvements or additions to the documentation
Projects
None yet
Development

No branches or pull requests

2 participants