search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.08k stars 1.26k forks source link

[data grid] Upgrading `@mui/x-data-grid-premium` to `v7` produces emotion error #13304

Closed Jul13nT closed 1 month ago

Jul13nT commented 3 months ago

The problem in depth

I'm trying to upgrade @mui/x-data-grid-premium from 6.20.0 to 7.5.1.

Here are my dependencies :

"@emotion/react": "^11.11.4",
"@emotion/styled": "^11.11.5",
"@mui/lab": "^5.0.0-alpha.170",
"@mui/material": "^5.15.19",
"@mui/styled-engine": "^5.15.14",
"@mui/system": "^5.15.15",
"@mui/x-data-grid-premium": "^7.5.1",
"@mui/x-license": "^7.2.1",

I'm using Yarn 4.0.2 with PnP mode, vite 4.5.0, rollup 3.29.4.

After upgrading the data-grid version, I'm getting the following error when I'm trying to build the application with vite:

RollupError: "CacheProvider" is not exported by "__vite-optional-peer-dep:@emotion/react:@mui/styled-engine", imported by "../../.yarn/__virtual__/@mui-styled-engine-virtual-568982b6a6/0/cache/@mui-styled-engine-npm-5.15.14-f2c4d6b014-0d262ea0b3.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js".

When starting the dev mode with vite, I'm getting this error:

Error: Could not resolve "@emotion/styled" imported by "@mui/styled-engine". Is it installed?
    at optional-peer-dep:__vite-optional-peer-dep:@emotion/styled:@mui/styled-engine (@mui_x-data-grid-premium.js?v=2c2b51c2:162:11)
    at __require2 (chunk-7REXU52E.js?v=1c3079f4:19:50)
    at @mui_x-data-grid-premium.js?v=2c2b51c2:174:29

Upgrading all MUI dependencies to the latest version (even @mui/x-date-pickers-pro), does not produce the problem. Only upgrading data-grid from 6 to 7 causes the error.

Do you have any idea what could be the problem ?

Your environment

`npx @mui/envinfo` ``` System: OS: Linux 6.5 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish) Binaries: Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node npm: 10.1.0 - ~/.nvm/versions/node/v20.9.0/bin/npm pnpm: Not Found Browsers: Chrome: 125.0.6422.60 ```

Search keywords: data-grid upgrade v7 migration emotion Order ID: 69724

michelengelen commented 3 months ago

@MBilalShafi did we change something on the styled engine side going from v6 to v7?

MBilalShafi commented 3 months ago

@MBilalShafi did we change something on the styled engine side going from v6 to v7?

I don't think so, since the @mui/styled-engine is coming from core, it hasn't undergone a major release yet.

@Jul13nT May I ask how you use the @mui/styled-engine package in your code, I tried to create a vite-based minimal setup but couldn't reproduce the error on upgrading the grid package from v6 to v7.

Is something unrelated to your project's grid causing the issue?

Please provide a minimal reproduction example, possibly a GitHub repo where the error reproduces on upgrading the packages. That'd help to understand more about the issue.

If you want, you can start with the minimal setup I tested with.

Thank you!

Jul13nT commented 3 months ago

@MBilalShafi Thank you for your answer. Sorry for the confusion, I don't declare @mui/styled-engine in my package.json, it was a test to see if it changes anything to the problem.

I'm not sure about what could be the problem, I will try to make a reproduction.

github-actions[bot] commented 3 months ago

The issue has been inactive for 7 days and has been automatically closed.

Jul13nT commented 1 month ago

Sorry for the late reply. I managed to make a minimal reproduction repo: https://github.com/Jul13nT/mui-x-upgrade-7-styled-error

After cloning, you need to run yarn install.

There are two different errors that you can reproduce.

The first one by running yarn build:preprod:service-openapi:

error during build:
../../.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js (4:9): "CacheProvider" is not exported by "__vite-optional-peer-dep:@emotion/react:@mui/styled-engine", imported by "../../.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js".
file: /home/julien/projects/mui-x-upgrade-7-styled-error/.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js:4:9

The second one by running yarn start:service-openapi and accessing the web app at http://localhost:8080/openapi (the error is in the browser console):

Uncaught Error: Could not resolve "@emotion/styled" imported by "@mui/styled-engine". Is it installed?
    optional-peer-dep:__vite-optional-peer-dep: @mui_x-data-grid-premium.js:266
    __require2 chunk-PLDDJCW6.js:17
    <anonymous> index.js:11
styled-engine:1:6
cherniavskii commented 1 month ago

Hi @Jul13nT Thanks for the reproduction! However, it seems to work just fine: https://stackblitz.com/~/github.com/Jul13nT/mui-x-upgrade-7-styled-error

Can you double-check your environment?

Jul13nT commented 1 month ago

Hello, thanks for the response !

You have a problematic warning warning Ignoring 'yarnPath' from '.yarnrc' because it's currently not supported.. It show yarn install v1.22.19. You need to run the commands in an environment where Yarn Berry is supported. With Yarn 4.3.1 I have the errors.

cherniavskii commented 1 month ago

Thanks, I managed to reproduce it in GitHub Codespaces:

$ yarn build:preprod:service-openapi
vite v5.3.3 building for production...
✓ 1820 modules transformed.
x Build failed in 9.71s
error during build:
../../.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js (4:9): "CacheProvider" is not exported by "__vite-optional-peer-dep:@emotion/react:@mui/styled-engine", imported by "../../.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js".
file: /workspaces/mui-x-upgrade-7-styled-error/.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js:4:9

2: 
3: import * as React from 'react';
4: import PropTypes from 'prop-types';
            ^
5: import { CacheProvider } from '@emotion/react';
6: import createCache from '@emotion/cache';

    at getRollupError (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/parseAst.js:392:41)
    at error (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/parseAst.js:388:42)
    at Module.error (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:13827:16)
    at Module.traceVariable (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:14275:29)
    at ModuleScope.findVariable (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:11990:39)
    at FunctionScope.findVariable (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)
    at FunctionBodyScope.findVariable (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:7432:38)
    at Identifier.bind (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:6908:40)
    at CallExpression.bind (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:4771:28)
    at CallExpression.bind (file:///workspaces/mui-x-upgrade-7-styled-error/.yarn/cache/rollup-npm-4.18.1-ff252ce141-c3c73252fd.zip/node_modules/rollup/dist/es/shared/node-entry.js:9135:15)

I'm not sure it's an MUI X issue though, it seems like an issue on the package manager side. I'll look into it.

cherniavskii commented 1 month ago

What I can see from the errors above:

1.

    ../../.yarn/__virtual__/@mui-styled-engine-virtual-619ee54b8b/0/cache/@mui-styled-engine-npm-5.16.4-494372f892-5dbd656aed.zip/node_modules/@mui/styled-engine/StyledEngineProvider/StyledEngineProvider.js (4:9): "CacheProvider" is not exported by "__vite-optional-peer-dep:@emotion/react:@mui/styled-engine"
It's very hard to debug package resolutions without `node_modules`, but it says that `@emotion/react` does not export `CacheProvider`.
`@emotion/react` is a [peer dependency of `@mui/styled-engine`](https://unpkg.com/browse/@mui/styled-engine@5.16.4/package.json).
`@emotion/react` is installed in `service-openapi/front`'s `package.json` and is resolved to version 11.11.4: https://github.com/Jul13nT/mui-x-upgrade-7-styled-error/blob/5577fb0344c710ed5a5a25b97fc1999a8514ae92/yarn.lock#L508-L509
`CacheProvider` is exported in https://unpkg.com/browse/@emotion/react@11.11.4/dist/emotion-react.esm.js

I don't see anything wrong with dependencies here, which leaves me with the conclusion that it's probably an issue on the `yarn` side.
  1. Uncaught Error: Could not resolve "@emotion/styled" imported by "@mui/styled-engine". Is it installed?
    optional-peer-dep:__vite-optional-peer-dep: @mui_x-data-grid-premium.js:266

    This one comes from vite, but I don't understand it. @emotion/styled is clearly declared in dependencies, so it should be installed. If it's not – then again, it might be an issue with yarn.

Jul13nT commented 1 month ago

Thanks for your investigation. Since I don't have the problem with MUI-X 6, I assumed it was a MUI-X issue. I will open an issue in Yarn repo.

cherniavskii commented 1 month ago

Thanks @Jul13nT I'll close the issue for now, but feel free to post an update when you have something to share.

github-actions[bot] commented 1 month ago

:warning: 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.

@Jul13nT: 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.

Jul13nT commented 1 month ago

I have news!

This comment https://github.com/yarnpkg/berry/issues/6425#issuecomment-2263232456 says that there is a problem with dependencies declaration in @mui/styled-engine.

Adding the following to .yarnrc.yml to override the package.json of @mui/styled-engine fixes the issue:

packageExtensions: {
  "@mui/styled-engine@*": {
    dependencies: {
      "@emotion/react": "^11.4.1",
      "@emotion/styled": "^11.3.0",
    }
  },
}

I don't know if the right solution for MUI is to declare these in dependencies instead of peerDependencies like I did. I will let you be the judge of that 😄

clemyan commented 1 month ago

Looks to me like the peer dependencies on @emotion/react and @emotion/styled should at least not be declared optional. (I don't know MUI so it is very possible I'm mistaken)

To quote myself:

Yarn would have warned you about this, but since @mui/styled-engine declares the peer to be optional the warning is suppressed. I don't know MUI but it looks like @mui/styled-engine needs @emotion/react and @emotion/styled to work, in which case declaring the peer to be optional is incorrect.

After that is fixed, Yarn would properly warn about @mui/x-data-grid-premium not declaring peer dependencies on @emotion/react and @emotion/styled.

cherniavskii commented 1 month ago

Hi @clemyan Thanks for looking into this!

  1. Regarding @mui/styled-engine declaring @emotion/react and @emotion/styled as optional – I think this is intentional. @mui/styled-engine is a dependency of @mui/system, which can optionally be used with @mui/styled-engine-sc that uses styled-components and doesn't use Emotion. In that case, @mui/styled-engine is still installed (as a direct dependency), but it's not used and Emotion isn't installed.

  2. This is because @mui/x-data-grid-premium does not declare a peer dependency on @emotion/react and @emotion/styled

    @mui/x-data-grid-premium does not import Emotion directly, so declaring Emotion as a peer dependency feels counter-intuitive and doesn't make a lot of sense to me.

    when the instance of @mui/styled-engine under @mui/x-data-grid-premium tries to import @emotion/react, it will fail to resolve.

    I'm not sure I understand this one. Are you saying that any peer dependency in the dependency tree should be propagated to the top and declared in each parent package, otherwise it won't be resolved?

clemyan commented 1 month ago

Regarding @mui/styled-engine declaring @emotion/react and @emotion/styled as optional – I think this is intentional. @mui/styled-engine is a dependency of @mui/system, which can optionally be used with @mui/styled-engine-sc that uses styled-components and doesn't use Emotion. In that case, @mui/styled-engine is still installed (as a direct dependency), but it's not used and Emotion isn't installed.

So @mui/styled-engine-sc is intended to be aliased. Hmm that's tricky indeed. I don't think I have a clean solution for this case.

There are ways to be "more technically correct" if designed for from the start, but to do it that way now would be quite cumbersome and break many end users' setup, so let's just say that ship has sailed.

For the issue at hand, declaring that @mui/x-data-grid-premium has (optional) peer dependencies on @emotion/react and @emotion/styled should be suffice, though you may want to also review other packages for similar problems.

I'm not sure I understand this one. Are you saying that any peer dependency in the dependency tree should be propagated to the top and declared in each parent package, otherwise it won't be resolved?

In PnP, yes. See this article by our lead maintainer.

More generally, package managers should ideally enforce that:

Unfortunately, npm (and by extension Yarn's node-modules linker because that is designed as a compatibility layer) does the former without the latter. In cases like this, it will allow you to do something (e.g. accessing transitive dependencies, implicitly inheriting peer dependencies, etc.) the vast majority of the time, but then "breaks" when it hits an edge case and cause issues. That's because allowing you to do that thing in the first place is just a coincidence, not an enforced feature.

PnP uses a different approach -- it enforces both. So while sometimes it seems unnecessarily strict, the stuff it disallows are often things you shouldn't rely on in the first place; you just didn't hit the edge case where doing that would be problematic.

Jul13nT commented 1 month ago

For the issue at hand, declaring that @mui/x-data-grid-premium has (optional) peer dependencies on @emotion/react and @emotion/styled should be suffice, though you may want to also review other packages for similar problems.

I can confirm that doing this in .yarnrc.yml also fixes the issue:

packageExtensions: {
  "@mui/x-data-grid-premium@*": {
    peerDependencies: {
      "@emotion/react": "*",
      "@emotion/styled": "*",
    },
    peerDependenciesMeta: {
      "@emotion/react": {
        optional: true,
      },
      "@emotion/styled": {
        optional: true,
      },
    }
  },
  "@mui/x-data-grid-pro@*": {
    peerDependencies: {
      "@emotion/react": "*",
      "@emotion/styled": "*",
    },
    peerDependenciesMeta: {
      "@emotion/react": {
        optional: true,
      },
      "@emotion/styled": {
        optional: true,
      },
    }
  },
  "@mui/x-data-grid@*": {
    peerDependencies: {
      "@emotion/react": "*",
      "@emotion/styled": "*",
    },
    peerDependenciesMeta: {
      "@emotion/react": {
        optional: true,
      },
      "@emotion/styled": {
        optional: true,
      },
    }
  },
}
cherniavskii commented 1 month ago

In PnP, yes. See this article by our lead maintainer.

@clemyan Thanks for the link! I didn't know that and I've never used yarn PnP.

Redeclaring peer dependencies of dependencies kinda sucks. I don't mind adding it once, but should we also keep the version in sync? If we don't, can it lead to multiple package versions being installed and used?

github-actions[bot] commented 1 month ago

:warning: 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.

@Jul13nT: 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.