rjsf-team / react-jsonschema-form

A React component for building Web forms from JSON Schema.
https://rjsf-team.github.io/react-jsonschema-form/
Apache License 2.0
14.18k stars 2.18k forks source link

Module not found: Error: Can't resolve '@material-ui/core' and '@material-ui/icons' #2762

Closed jakub-astrahit closed 2 years ago

jakub-astrahit commented 2 years ago

Description

When I use rjsf out-of-the-box, right after installation I get some unnecessary warnings in the console.

Steps to Reproduce

  1. Import form for MUI version 5.
  2. Observe warnings in the console.

Expected behavior

No warnings.

Actual behavior

I use rjsf as follows with Material UI version 5.

import { MuiForm5 as Form } from '@rjsf/material-ui';
  <Form schema={schema}>
    <></>
  </Form>

That gives me the following warning in the console:

WARNING in ../../node_modules/@rjsf/material-ui/dist/material-ui.esm.js 1139:21-49
Module not found: Error: Can't resolve '@material-ui/core' in '/... ... .../node_modules/@rjsf/material-ui/dist'
...
WARNING in ../../node_modules/@rjsf/material-ui/dist/material-ui.esm.js 1140:23-52
Module not found: Error: Can't resolve '@material-ui/icons' in '/... ... .../node_modules/@rjsf/material-ui/dist'

Version

From package.json:

    "@emotion/react": "^11.7.1",
    "@emotion/styled": "^11.6.0",
    "@mui/icons-material": "^5.4.4",
    "@mui/material": "^5.3.1",
    "@rjsf/core": "^4.1.0",
    "@rjsf/material-ui": "^4.1.0",
keepeek-rd commented 2 years ago

this issue should normally be fixed with version 4.1.0 :( https://github.com/rjsf-team/react-jsonschema-form/issues/2721

c0ncentus commented 2 years ago

have the same issue image "@material-ui/core": "^4.12.3", "@material-ui/icons": "^4.11.2", "@rjsf/core": "^4.1.0", "@rjsf/material-ui": "^4.1.0",

formatlos commented 2 years ago

same issue here "@mui/core": "5.0.0-alpha.54", "@mui/icons-material": "^5.4.1", "@mui/material": "^5.4.1", "@rjsf/core": "^4.1.0", "@rjsf/material-ui": "^4.1.0", "typescript": "^4.5.5"

ggmartins091 commented 2 years ago

Not to spam, but I'm having the same issue.

"@emotion/react": "^11.8.1", "@emotion/styled": "^11.8.1", "@mui/icons-material": "^5.4.4", "@mui/lab": "^5.0.0-alpha.71", "@mui/material": "^5.4.4", "@mui/styles": "^5.4.4", "@rjsf/core": "^4.1.0", "@rjsf/material-ui": "^4.1.0",

joriswitteman commented 2 years ago

Same issue when trying to use RJSF 4 with MaterialUI 4.

    "@material-ui/core": "^4.12.3",
    "@material-ui/icons": "^4.11.2",
    "@rjsf/core": "^4.1.0",
    "@rjsf/material-ui": "^4.1.0",
aiibe commented 2 years ago

Issues listed above from other users still persisted on version 4.1.1 I am using RJSF with MUIv5

heath-freenome commented 2 years ago

This is a known issue with the way Mui 4 and 5 support were built to be side-by-side. There are webpack tricks that can be used to eliminate the warnings.

Webpack 5

Add the following in your webpack.config.js to eliminate the errors for Material-UI v5 when you are only using v4:

// Have webpack 5 set the fallbacks for the resolver for Mui 5 to be false
config.resolve.fallback = { '@mui/material': false, '@mui/icons-material': false };

Add the following in your webpack.config.js to eliminate the errors for Material-UI v4 when you are only using v5:

// Have webpack 5 set the fallbacks for the resolver for Mui 4 to be false
config.resolve.fallback = { '@material-ui/core': false, '@material-ui/icons': false };

Webpack 4

Add the following in your webpack.config.js to eliminate the errors for Material-UI v5 when you are only using v4:

// Alias Material-ui 5 to be Material-ui 4
config.resolve.alias['@mui/material'] = require.resolve('@material-ui/core');
config.resolve.alias['@mui/icons-material'] = require.resolve('@material-ui/icons');

Add the following in your webpack.config.js to eliminate the errors for Material-UI v4 when you are only using v5:

// Alias Material-ui 4 to be Material-ui 5
config.resolve.alias['@material-ui/core'] = require.resolve('@mui/material');
config.resolve.alias['@material-ui/icons'] = require.resolve('@mui/icons-material');
aiibe commented 2 years ago

Sadly, React projects initiated with CRA cannot access to Webpack config without ejecting or editing webpack.config.js directly in node_modules.

epicfaace commented 2 years ago

@heath-freenome are there ways we can change the require imports in https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/Theme5/Mui5Context.tsx and https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/Theme/MaterialUIContext.tsx so that they don't give the errors by default? For example, see:

This might also be a solution -- using __webpack_is_included__ -- https://github.com/webpack/webpack/issues/8838#issuecomment-468188115 ???

szubster commented 2 years ago

Maybe the material-ui module should be splitted into three different modules? common, material v4 and material v5?

benjdlambert commented 2 years ago

This is a known issue with the way Mui 4 and 5 support were built to be side-by-side. There are webpack tricks that can be used to eliminate the warnings.

@heath-freenome is this something that is going to be addressed? Not sure that the webpack bundling hacks are going to work for larger projects.

formatlos commented 2 years ago

not to forget that not everybody is using webpack.

there's a discussion on stackoverflow how to handle optional peer dependencies, maybe some of the solutions is sufficient here as well https://stackoverflow.com/questions/54392809/how-do-i-handle-optional-peer-dependencies-when-publishing-a-typescript-package

szubster commented 2 years ago

Also, if we move out of dist directory and switch typings in package.json to just ./index.d.ts we should be able to import @rjsf/material-ui/MuiForm not whole @rjsf/material-ui/. Similar to how material-ui packages do. Are we able to do such change?

MatthewWid commented 2 years ago

This is breaking our builds when we are building with Vite for production.

We receive in the DOM:

WARNING: @mui/material or @mui/icons-material is not available

And in the console:

ReferenceError: require is not defined

This occurs whether we import the CommonJS distribution or the ESM distribution. Inspecting the ESM distribution's bundled code you can see that oddly it still includes CommonJS require statements to pull in the Material-UI components individually:

// material-ui.esm.js

try {
  // core
  Box$1 = /*#__PURE__*/require('@mui/material/Box').default;
  Button$1 = /*#__PURE__*/require('@mui/material/Button').default;
  Checkbox$1 = /*#__PURE__*/require('@mui/material/Checkbox').default;
  Divider$1 = /*#__PURE__*/require('@mui/material/Divider').default;
  FormControl$1 = /*#__PURE__*/require('@mui/material/FormControl').default;
  FormControlLabel$1 = /*#__PURE__*/require('@mui/material/FormControlLabel').default;
  FormGroup$1 = /*#__PURE__*/require('@mui/material/FormGroup').default;
  FormHelperText$1 = /*#__PURE__*/require('@mui/material/FormHelperText').default;
  FormLabel$1 = /*#__PURE__*/require('@mui/material/FormLabel').default;
  Grid$1 = /*#__PURE__*/require('@mui/material/Grid').default;
  IconButton$1 = /*#__PURE__*/require('@mui/material/IconButton').default;
  Input$1 = /*#__PURE__*/require('@mui/material/OutlinedInput').default;
  InputLabel$1 = /*#__PURE__*/require('@mui/material/InputLabel').default;
  List$1 = /*#__PURE__*/require('@mui/material/List').default;
  ListItem$1 = /*#__PURE__*/require('@mui/material/ListItem').default;
  ListItemIcon$1 = /*#__PURE__*/require('@mui/material/ListItemIcon').default;
  ListItemText$1 = /*#__PURE__*/require('@mui/material/ListItemText').default;
  MenuItem$1 = /*#__PURE__*/require('@mui/material/MenuItem').default;
  Paper$1 = /*#__PURE__*/require('@mui/material/Paper').default;
  Radio$1 = /*#__PURE__*/require('@mui/material/Radio').default;
  RadioGroup$1 = /*#__PURE__*/require('@mui/material/RadioGroup').default;
  Slider$1 = /*#__PURE__*/require('@mui/material/Slider').default;
  TextField$1 = /*#__PURE__*/require('@mui/material/TextField').default;
  Typography$1 = /*#__PURE__*/require('@mui/material/Typography').default; // icons

  AddIcon$1 = /*#__PURE__*/require('@mui/icons-material/Add').default;
  ArrowDownwardIcon$1 = /*#__PURE__*/require('@mui/icons-material/ArrowDownward').default;
  ArrowUpwardIcon$1 = /*#__PURE__*/require('@mui/icons-material/ArrowUpward').default;
  ErrorIcon$1 = /*#__PURE__*/require('@mui/icons-material/Error').default;
  RemoveIcon$1 = /*#__PURE__*/require('@mui/icons-material/Remove').default;
} catch (_unused) {// no-op
}

Which is causing errors for us as they are not transformed by our build system because it does not assume CommonJS syntax to be present.

We are using Vite v2.8.0 and RJSF /core and /material-ui v4.1.1 but this issue still seems to occur for any version past v4.0.0.

epicfaace commented 2 years ago

We'll be discussing this issue further in the next synchronous meeting: https://docs.google.com/document/d/12PjTvv21k6LIky6bNQVnsplMLLnmEuypTLQF8a-8Wss/edit

MatiasCiccone commented 2 years ago

Sadly, React projects initiated with CRA cannot access to Webpack config without ejecting or editing webpack.config.js directly in node_modules.

@szubster have you checked https://www.npmjs.com/package/react-app-rewired? I hope that helps you

aiibe commented 2 years ago

Sadly, React projects initiated with CRA cannot access to Webpack config without ejecting or editing webpack.config.js directly in node_modules.

@szubster have you checked https://www.npmjs.com/package/react-app-rewired? I hope that helps you

I had already checked that lib. But I’d nice to avoid more deps. Thanks btw.

szubster commented 2 years ago

@MatiasCiccone We are not using CRA. We use backstage.

MatiasCiccone commented 2 years ago

@MatiasCiccone We are not using CRA. We use backstage.

my bad, I was reading @aiibe message and replied to you.

Sadly, React projects initiated with CRA cannot access to Webpack config without ejecting or editing webpack.config.js directly in node_modules.

@aiibe have you checked react-app-rewired?

szubster commented 2 years ago

@epicfaace Any conclusions after synchronous meeting?

epicfaace commented 2 years ago

I'm checking with some other library authors to see if there are other, more recommended ways, to include optional dependencies that avoid these problems.

Otherwise, we will just have to split @rjsf/material-ui into two libraries.

dheeraj1447 commented 2 years ago

I'm checking with some other library authors to see if there are other, more recommended ways, to include optional dependencies that avoid these problems.

Otherwise, we will just have to split @rjsf/material-ui into two libraries.

Can this be done soon?

epicfaace commented 2 years ago

Another option is to have @rjsf/material-ui/4 and @rjsf/material-ui/5. wdyt @heath-freenome ?

epicfaace commented 2 years ago

Also could we potentially do something with imports / exports? https://github.com/panva/jose/blob/main/package.json#L56-L87

https://nodejs.org/api/packages.html#subpath-imports

benjdlambert commented 2 years ago

Another option is to have @rjsf/material-ui/4 and @rjsf/material-ui/5

Without being an expert on module exports, I still think that that it's going to cause a problem for typescript to compile, if any of the @rjsf ecosystem has a dependency on something from @material-ui or/and @mui , it's going to mean that projects are going to need to have both of the mui libraries installed in the project in order to get libCheck to pass. I'd really encourage that we move away from using the types directly from the package as type safety in the project, and rather define your own types that live inside the project that are basically a copy of the interface.

Re-exporting and/or depending on types from other packages really can get really complicated and hairy.

szubster commented 2 years ago

Modules @rjsf/material-ui-4 and @rjsf/material-ui-5 should work though. Maybe with a common module, that would have no dependencies on mui at all.

mulib commented 2 years ago

hey @heath-freenome do you need to bump npm version? i still facing the errors

"@rjsf/core": "^4.1.1",
"@rjsf/material-ui": "^4.1.1"
benjdlambert commented 2 years ago

@mulib don't think the changes have been released yet. More to follow 🤞

OliverDudgeon commented 2 years ago

I'm trying out 4.2.0 but I'm having issues getting the sub-packages to load in NextJS 12.1.6. When importing from the sub-package E.g.

import { Theme5 } from '@rjsf/material-ui/v5';

the import fails. Typescript gives a warning that the module isn't found and if I disable type checking, Theme5 is undefined.

I'm using node v16.13.1 in case that's important with the use of the exports field in the package.json.

ggmartins091 commented 2 years ago

I've tested this new 4.2.0 release and I'm still getting the same error. Screenshot from 2022-05-09 16-51-57

Using rjsf/core and rjsf/material-ui 4.2.0, I have mui-material and mui-icons-material both on 5.4.4.

Is there something I'm missing here?

heath-freenome commented 2 years ago

I'm trying out 4.2.0 but I'm having issues getting the sub-packages to load in NextJS 12.1.6. When importing from the sub-package E.g.

import { Theme5 } from '@rjsf/material-ui/v5';

the import fails. Typescript gives a warning that the module isn't found and if I disable type checking, Theme5 is undefined.

I'm using node v16.13.1 in case that's important with the use of the exports field in the package.json.

Try

import { Theme } from `@rjsf/material-ui/v5`;
OliverDudgeon commented 2 years ago

Sorry my bad. The tsconfig does work.

heath-freenome commented 2 years ago

I've tested this new 4.2.0 release and I'm still getting the same error. Screenshot from 2022-05-09 16-51-57

Using rjsf/core and rjsf/material-ui 4.2.0, I have mui-material and mui-icons-material both on 5.4.4.

Is there something I'm missing here?

What does your import of @rjsf/material-ui look like?

ggmartins091 commented 2 years ago

What does your import of @rjsf/material-ui look like?

I've tried both Form from @rjsf/material-ui/v5 and the withTheme approach aswell:

import { IChangeEvent, withTheme } from '@rjsf/core';
import { Theme } from '@rjsf/material-ui/v5';
const Form = withTheme(Theme);

Both work locally, but when using a production build I see that warning instead of the form.

heath-freenome commented 2 years ago

What does your import of @rjsf/material-ui look like?

I've tried both Form from @rjsf/material-ui/v5 and the withTheme approach aswell:

import { IChangeEvent, withTheme } from '@rjsf/core';
import { Theme } from '@rjsf/material-ui/v5';
const Form = withTheme(Theme);

Both work locally, but when using a production build I see that warning instead of the form.

That seems to indicate that your production build isn't resolving the imports for mui 5... I'd start there

ggmartins091 commented 2 years ago

That seems to indicate that your production build isn't resolving the imports for mui 5... I'd start there

What does your import of @rjsf/material-ui look like?

I've tried both Form from @rjsf/material-ui/v5 and the withTheme approach aswell:

import { IChangeEvent, withTheme } from '@rjsf/core';
import { Theme } from '@rjsf/material-ui/v5';
const Form = withTheme(Theme);

Both work locally, but when using a production build I see that warning instead of the form.

That seems to indicate that your production build isn't resolving the imports for mui 5... I'd start there

Could please you point me somewhere here? My whole application is based on mui, every component uses mui so it is for sure being resolved on production build, is this something specific do rjsf/material-ui/v5 not being able to resolve mui that I should be digging into?

Right now I'm assuming the error I'm seeing comes from here but I don't see why Mui5Context would be empty here, the stuff it imports are the same I'm using on my application.

ggmartins091 commented 2 years ago

Just for reference: I tested creating a project with vite (which is what my application use) and CRA to test this and the CRA build works perfectly, the vite production build on the other hand doesn't.

I'll dig deeper to try and understand it. I'll share anything I find, should be something to do with rollup's configuration.

tobek commented 2 years ago

@ggmartins091 your issue might be that module resolution for some other part of your environment/toolchain is not set up to use the rjsf team's workaround. See the typescript/jest configuration adjustments in the readme here: https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/README.md#material-ui-version-5-1 - you may need to do something similar

tobek commented 2 years ago

I have run into 2 issues that prevent the implemented workaround from working:

  1. The workaround is not portable. Importing from @rjsf/material-ui/v5 in typescript package A works locally (after I update tsconfig), but when I build package A and import it into app B, I get Module not found: Can't resolve '@rjsf/material-ui/v5' in ... unless I update tsconfig in app B. Package A is an SDK that will be consumed by third parties, who would then also have to update their typescript/jest/etc configurations in order to use it, which is far from ideal.
    • Maybe there's some way to configure the build such that apps consuming package A don't have to make any config changes?
  2. It doesn't seem to work with create-react-app typescript projects. When I put the tsconfig adjustments in, they get removed with the message: compilerOptions.paths must not be set (aliased imports are not supported)
viniciusalvess commented 2 years ago

I'm having the same issue using v5 and vite. When I run the project on localhost, everything works as expected. But when I build and deploy the project, I'm getting the warn.

"@emotion/react": "^11.9.0",
    "@emotion/styled": "^11.8.1",
    "@mui/icons-material": "^5.8.0",
    "@mui/material": "^5.8.1",
    "@mui/system": "^5.8.1",
    "@rjsf/core": "^4.2.0",
    "@rjsf/material-ui": "^4.2.0",

image

viniciusalvess commented 2 years ago

@ggmartins091 Any luck building this package with Vite to production?

ggmartins091 commented 2 years ago

@ggmartins091 Any luck building this package with Vite to production?

I did some other testing but with no luck. Tbh I haven't had enough time to look at it.

I don't know if this could be it, but this states that rollup (vite's bundler) doesnt support commonjs and, again, I'm not sure, but if that is true, maybe these requires won't work?

Maybe @heath-freenome could comment on this.

I'm really just shooting at the dark here.

Edit: Just confirmed, it is the require.

This expection is this: ReferenceError: require is not defined at material-ui-v5.esm.js:1198:3

MatthewWid commented 2 years ago

@viniciusalvess I resolved this by replacing the requires that @ggmartins091 mentions with import statements and then updating the ESM output bundle file to be the main entry-point in the package.json main field, then republishing it as a private package on an internal private registry for use.

I already mentioned the strange use of require in the ESM bundle earlier in this issue and I see that it indeed appears to be the issue at least for building with Vite.

ggmartins091 commented 2 years ago

What I did as a temporary fix was using patch-package I changed the requires to import, I don't know if this could cause any side effects but it works for now, until there is further discussion on this topic.

viniciusalvess commented 2 years ago

I tried to use patch-package like you @ggmartins091 mentioned, but it still not working. I can change the text of the warning to make sure my changes are being applied and I can see the changes after the deployment. But using require or import, the outcome is the same for me. This could be because I'm using yarn workspace. When I have some more time I'll continue trying.

viniciusalvess commented 2 years ago

I spent some more time on this issue and here is what worked for me. I followed ggmartins091 steps using patch-package, moved the imports to the beginning of the file and commented out from line 1196 - 1261. Here is my file. material-ui-v5.esm.zip .

heath-freenome commented 2 years ago

We are planning a v5 that will be a breaking change for many things, including splitting v4 and v5 into separate packages. This work will likely take several months to complete and we are planning a beta for a while. stay tuned

ggmartins091 commented 2 years ago

We are planning a v5 that will be a breaking change for many things, including splitting v4 and v5 into separate packages. This work will likely take several months to complete and we are planning a beta for a while. stay tuned

In the meanwhile no intentions on releasing any fixes for this case where commonjs is not supported?

heath-freenome commented 2 years ago

We are planning a v5 that will be a breaking change for many things, including splitting v4 and v5 into separate packages. This work will likely take several months to complete and we are planning a beta for a while. stay tuned

In the meanwhile no intentions on releasing any fixes for this case where commonjs is not supported?

Unfortunately most of our resources (primarily me) are focusing on the new v5 version. If you can find a way to update the current one without patch-package then I encourage you to push a PR with them. It appears that the best (and maybe only) fix requires the upcoming v5 breaking change

ggmartins091 commented 2 years ago

We are planning a v5 that will be a breaking change for many things, including splitting v4 and v5 into separate packages. This work will likely take several months to complete and we are planning a beta for a while. stay tuned

In the meanwhile no intentions on releasing any fixes for this case where commonjs is not supported?

Unfortunately most of our resources (primarily me) are focusing on the new v5 version. If you can find a way to update the current one without patch-package then I encourage you to push a PR with them. It appears that the best (and maybe only) fix requires the upcoming v5 breaking change

Maybe I'm missing something and if so let me know, please. But wouldn't changing the requires to imports on MUI5Context and MaterialUiContext fix the problem we are discussing on these last comments?

The problem for some of us is that our bundler does not support commonjs, so isn't that a simple fix? Or is there a specific reason for using require there instead of ESM?