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

[BUG] material-ui not built correctly(?) #3004

Closed
4 tasks done
applike-ss opened this issue Mar 26, 2021 · 35 comments
Closed
4 tasks done

[BUG] material-ui not built correctly(?) #3004

applike-ss opened this issue Mar 26, 2021 · 35 comments
Assignees
Labels
bug Something isn't working contributors welcome! contributors welcome!

Comments

@applike-ss
Copy link

Bug Report Quick Checklist

  • I am on the latest version of Snowpack & all plugins.
  • I use package manager npm
  • I run Snowpack on OS Linux
  • I run Snowpack on Node.js v14.16.0

Describe the bug

I tried migrating a project from CRA to SCA, but am getting issues with material-ui.
See screenshot:
image

To Reproduce

npx create-snowpack-app material-ui-test --template @snowpack/app-template-react-typescript
cd material-ui-test
npm install @material-ui/lab @material-ui/core
echo "import { Skeleton } from '@material-ui/lab';" > src/App2.tsx
cat src/App.tsx >> src/App2.tsx
sed -ie 's/Learn React/<Skeleton\/>/' src/App2.tsx
mv src/App2.tsx src/App.tsx
npm start

Expected behavior

I would have assumed that react is not throwing an error and that snowpack would've "packed" the exported darken function from node_modules/@material-ui/core/esm/styles/colorManipulator.js:245 into the requested /_snowpack/pkg/@material-ui.core.styles.v4.11.3.js

@awburgard
Copy link

I second this issue. MUI build fails with @material-ui/lab on the latest version of snowpack

@Danzo7
Copy link
Contributor

Danzo7 commented Apr 2, 2021

downgrade to snowpack 3.0.13 will fix

@i3ima
Copy link

i3ima commented Apr 5, 2021

downgrade to snowpack 3.0.13 will fix

I hope this is a temporary solution

This also don't work for me

@Danzo7
Copy link
Contributor

Danzo7 commented Apr 5, 2021

downgrade to snowpack 3.0.13 will fix

I hope this is a temporary solution

This also don't work for me

@i3ima try run npx snowpack dev -reload,its works for me

@matiosfree
Copy link

Same issues on my end

SyntaxError: import not found: darken
 http://localhost:8080/_snowpack/pkg/@material-ui.lab.v4.0.0-alpha.43.js [:7:21] 

in code:
import {fade} from '@material-ui/core/styles';

in console:

SyntaxError: import not found: fade
 http://localhost:8080/dist/theme/marketingTheme.js [:2:8]

@awburgard
Copy link

I had to revert my version of snowpack AND change my imports to defaults instead of destructured.

@matiosfree
Copy link

@awburgard unfortunately, it's not my case. I have a pretty big application and will take a lot of time to replace destructured imports.

@dylanarmstrong
Copy link

This is working on latest master branch for me. So next release should have it fixed, I think.

@kasper573
Copy link

This is working on latest master branch for me. So next release should have it fixed, I think.

Latest works for me as well

@TheMadKow
Copy link

The same issue also happens with 5.0.0-alpha of MUI
`import * as MUI from '@material-ui/core';
..
<MUI.Box>
Hello World
</MUI.Box>

`

Capture

package.json >>
"snowpack": "3.3.0",
"@material-ui/core": "5.0.0-alpha.30",

@jalovatt
Copy link
Contributor

jalovatt commented Apr 23, 2021

Seeing this, or a similar issue, on 3.3.5 with @material-ui 4.11.x.

The file paths listed made me think it's maybe not picking up MUI's "module": "./esm/index.js", but even if I use babel-plugin-import to remap the imports to /esm it persists.

TypeError: _withWidth.default is not a function

.../_snowpack/pkg/@material-ui.core.withMobileDialog.v4.11.3.js [:61:35]

withMobileDialog_1</withMobileDialog/<@.../_snowpack/pkg/@material-ui.core.withMobileDialog.v4.11.3.js:61:35
@.../admin/components/DialogWindow/index.js:78:53

@jalovatt
Copy link
Contributor

So I've found the problem, at least in our case - Snowpack doesn't get along with Material UI's recommended import style.

https://material-ui.com/getting-started/usage/#quick-start

import React from 'react';
import ReactDOM from 'react-dom';
import Button from '@material-ui/core/Button';

function App() {
  return (
    <Button variant="contained" color="primary">
      Hello World
    </Button>
  );
}

ReactDOM.render(<App />, document.querySelector('#app'));

As of Snowpack 3.x, import Button from '@material-ui/core/Button' sidesteps any of package.json's "you can find ES modules here: ___" settings and pulls in the exact file path you gave, which is a CommonJS module and thus breaks.

I see that other posters above have slightly different errors than what we were seeing, but in case it helps anyone:

Solution 1

Add /esm to all of your direct imports:

import Typography from '@material-ui/core/Typography'
-->
import Typography from '@material-ui/core/esm/Typography'

Solution 2

Since we're in the world of ES Modules now, just use MUI's barrel exports:

import Typography from '@material-ui/core/Typography'
-->
import { Typography } from '@material-ui/core'

Solution 3?

The simplest option would be an alias in the Snowpack config, but my attempt failed:

{
  alias: {
    '@material-ui/core': '@material-ui/core/esm'
  }
}

because Snowpack picks up /esm/index.js as CommonJS:

[14:02:39] [esinstall] cjsAutoDetectExportsStatic .../node_modules/@material-ui/core/esm/index.js: Unexpected import statement in CJS module.
  at @:7:8
[14:02:39] [esinstall] cjsAutoDetectExportsRuntime error .../node_modules/@material-ui/core/esm/index.js: Command failed with exit code 1: node -p JSON.stringify(Object.keys(require('.../node_modules/@material-ui/core/esm/index.js')))
.../node_modules/@material-ui/core/esm/index.js:7
import * as colors from './colors';
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at wrapSafe (internal/modules/cjs/loader.js:979:16)
    at Module._compile (internal/modules/cjs/loader.js:1027:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at [eval]:1:28
    at Script.runInThisContext (vm.js:133:18)
    at Object.runInThisContext (vm.js:310:38)

...bunch of other logs and then...

[14:02:41] [snowpack] + @material-ui/core/esm/colors/[email protected] DONE
[14:02:41] [snowpack] The "path" argument must be of type string. Received undefined
[14:02:41] [snowpack] TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at validateString (internal/validators.js:124:11)
    at Object.join (path.js:1039:7)
    at .../node_modules/snowpack/lib/index.js:123039:54
    at async PackageSourceLocal.buildPackageImport (.../node_modules/snowpack/lib/index.js:122930:30)

@johnb8005
Copy link

for those who are interested, here is a working example: https://github.com/Nexysweb/react-snowpack

@vikigenius
Copy link

vikigenius commented May 20, 2021

Well, the esm solution suggested by @jalovatt doesn't work for me since I was trying to use the 5.0.0-alpha version and it doesn't have an esm folder

And it still shows
SyntaxError: indirect export not found: SliderMark
even when I try the barrel exports

@jp06
Copy link

jp06 commented May 24, 2021

Similar to @vikigenius, I started having this SliderMark not having a default export error when I updated Material UI to version 5. I downgraded Material UI to latest stable version (4.11.4 as of now) and now it works on current snowpack version (3.5.1). I was trying to install the new version of Material UI to get rid of this React Strict Mode error/warning regarding findDOMNode deprecation 😥.

@BenJBailey
Copy link

This seems to affect any css that is imported from npm.

@shripadk
Copy link

Screen Shot 2021-06-24 at 2 27 31 PM

Had a similar error thrown for me as well. Material UI v5.0.0-alpha.38.js.

@shripadk
Copy link

downgrade to snowpack 3.0.13 will fix

Can confirm that it works with Snowpack v3.0.13.

@shripadk
Copy link

Narrowed it down to the commit that causes this issue. Stems from this big internal rewrite of Snowpack: bea1c56

The commit prior to the one above worked fine. Will take a look at it during my free time. Not familiar with Snowpack internals.

@vikigenius
Copy link

vikigenius commented Jun 24, 2021

@shripadk great find. It seems like a huge rewrite though, I am not sure how easy it will be to figure it out. Maybe someone from the dev team can help on this. Probably has something to do with ES6 modules. MUI did change a lot of things recently regarding that like this mui/material-ui#26310. It could also explain why some of the suggested workarounds for MUI 4 don't work for MUI 5 alpha releases.

@vikigenius
Copy link

vikigenius commented Jun 30, 2021

Additional point to note is that the prod build snowpack build seems to work fine. As I mention here mui/material-ui#26568.

But the dev build npm run start i.e snowpack dev seems to still throw the same error.

@shripadk
Copy link

shripadk commented Jul 1, 2021

@vikigenius Yep can confirm! Production build works fine. Not Development server. This is definitely a bug with Snowpack and not with Material UI.

@drwpow drwpow added bug Something isn't working contributors welcome! contributors welcome! labels Jul 1, 2021
@drwpow drwpow self-assigned this Jul 1, 2021
@canozo
Copy link

canozo commented Jul 1, 2021

In our case we tried adding many aliases like the following:

// snowpack.config.js
{
  alias: {
    '@material-ui/core/Avatar': '@material-ui/core/esm/Avatar',
    '@material-ui/lab/Alert': '@material-ui/lab/esm/Alert',
  }
}

so that we didn't have to change every single Material UI import in the project (there are tons of imports!) like such:

import Typography from '@material-ui/core/Typography'
// -->
import Typography from '@material-ui/core/esm/Typography'
// or even
import { Typography } from '@material-ui/core'

It worked fine for dev builds but when creating a production build I would get the error:

lib-index-3eb408da.2558fcefa83ab187fe3a.js:1 Error: Minified React error #130;
visit https://reactjs.org/docs/error-decoder.html?invariant=130&args[]=object&args[]=
for the full message or use the non-minified dev environment for full errors and additional helpful warnings.

Here's a reproducible example for my case if it's of any help, though it seems like most big findings have already been discovered in this thread!
https://github.com/canozo/mui-snowpack-issue

git clone https://github.com/canozo/mui-snowpack-issue
cd mui-snowpack-issue
npm install
npm run build
npm start
# open http://localhost:5000/ in your browser!

@vikigenius
Copy link

I took a look at the cache that snowpack is building for the dev server.
Here's line 18, 19 from node_modules/.cache/snowpack/build/@material-ui/[email protected]/@material-ui/core.js

export * from '@material-ui/unstyled/ModalUnstyled';
export { SliderMark, SliderMarkLabel, SliderRail, SliderRoot, SliderThumb, SliderTrack, SliderValueLabel, getNativeSelectUtilityClasses, getOutlinedInputUtilityClass, getPaginationItemUtilityClass, getPaginationUtilityClass, getPaperUtilityClass, getPopoverUtilityClass, getRadioUtilityClass, getRatingUtilityClass, getScopedCssBaselineUtilityClass, getSelectUtilityClasses, getSkeletonUtilityClass, getSnackbarContentUtilityClass, getSnackbarUtilityClass, getSpeedDialActionUtilityClass, getSpeedDialIconUtilityClass, getSpeedDialUtilityClass, getStepButtonUtilityClass, getStepConnectorUtilityClass, getStepContentUtilityClass, getStepIconUtilityClass, getStepLabelUtilityClass, getStepUtilityClass, getStepperUtilityClass, getSvgIconUtilityClass, getSwitchUtilityClass, getTabScrollButtonUtilityClass, getTabUtilityClass, getTableBodyUtilityClass, getTableCellUtilityClass, getTableContainerUtilityClass, getTableFooterUtilityClass, getTableHeadUtilityClass, getTablePaginationUtilityClass, getTableRowUtilityClass, getTableSortLabelUtilityClass, getTableUtilityClass, getTabsUtilityClass, getTextFieldUtilityClass, getToggleButtonGroupUtilityClass, getToggleButtonUtilityClass, getToolbarUtilityClass, getTooltipUtilityClass, getTypographyUtilityClass, nativeSelectClasses, outlinedInputClasses, paginationClasses, paginationItemClasses, paperClasses, popoverClasses, radioClasses, ratingClasses, scopedCssBaselineClasses, selectClasses, skeletonClasses, sliderClasses, snackbarClasses, snackbarContentClasses, speedDialActionClasses, speedDialClasses, speedDialIconClasses, stepButtonClasses, stepClasses, stepConnectorClasses, stepContentClasses, stepIconClasses, stepLabelClasses, stepperClasses, svgIconClasses, switchClasses, tabClasses, tabScrollButtonClasses, tableBodyClasses, tableCellClasses, tableClasses, tableContainerClasses, tableFooterClasses, tableHeadClasses, tablePaginationClasses, tableRowClasses, tableSortLabelClasses, tabsClasses, textFieldClasses, toggleButtonClasses, toggleButtonGroupClasses, toolbarClasses, tooltipClasses, typographyClasses, useRadioGroup } from '@material-ui/unstyled/ModalUnstyled';

The second line that tries to export all these different classes is what's causing the error.
Removing that line from cache fixes the error.

Two things standout.

  1. @material-ui/unstyled/ModalUnstyled doesn't contain a lot of the exports in the 2nd line
  2. If we are reexporting everything using export * why is the 2nd line needed?

@drwpow I am hoping this helps you out.

@drwpow
Copy link
Collaborator

drwpow commented Jul 8, 2021

👋🏻 Thanks so much everyone for reporting on this issue. I think with the most recent 4.x and 5.x versions of @material-ui/*, and the newest version of Snowpack, most of the issues are resolved? I’m still going through all the issues listed but for the most part I think support for this package in Snowpack has improved pretty drastically overall.

In general @material-ui/core is in a weird spot, as it ships ESM but doesn’t yet take the extra step of declaring subpath exports. This is critical not only for Snowpack, but the larger Node ecosystem as a whole. And because Snowpack depends so much on ESM support, is why you’re seeing differences between Snowpack vs, say, webpack, which is still largely stuck in the old CommonJS ecosystem.

If someone here could open an issue on the Material UI repo to implement Node’s new ESM guidelines, I think most if not all of the issues would be resolved. Not only for Snowpack users, but for Node ESM as a whole.

@vikigenius
Copy link

@drwpow looks like the issue with SyntaxError: indirect export not found: SliderMark is still there you can reproduce it using the steps i added in mui/material-ui#26568. The issue persists even after i update snowpack to 3.8.0 (it seems to be the latest version).

The workaround I suggested with modifying the cache still works. From the discussion on mui issue page, it seems they are unwilling to do the necessary work to implement the new ESM guidelines.

What would be the action plan here? Is there a way we can work around that from snowpack itself ?

@merrifieldbrian
Copy link

@drwpow is there any update or plan regarding this issue? My team is currently trying to cut over to snowpack, but the issues with material-ui have become blockers. Thanks in advance

@tmaiaroto
Copy link

Can we skip the material-ui package from being in Snowpack cache? Would that help? Or otherwise import from an already built/bundled source of Material UI? I don't have any hopes of MUI changing things, regardless of what the ESM guidelines are, as they do very non-standard things to begin with (ie. log warnings at the error level). So I'm just wondering if it's possible to work around MUI.

@merrifieldbrian
Copy link

@tmaiaroto that seems reasonable to me. Is there any configuration way to accomplish this at the moment without breaking the node modules patterns in the project?

@drwpow
Copy link
Collaborator

drwpow commented Aug 24, 2021

This is a tough/impossible problem to fix on Snowpack’s side. To the answer of “can we skip the material-ui package from being in Snowpack cache,“ the answer is no simply because material-ui doesn’t ship browser-compatible code (ESM). They ship CommonJS. The simplest fix would be for material-ui to ship ESM code, but as @vikigenius has already pointed out, the team has said they’re unwilling to do the work.

Of course, the Snowpack project does its best to upconvert whatever CommonJS to ESM it can, but it’s an imperfect process, and while it works for many packages, it can’t always work for every package perfectly (because we’re changing the code from how it was originally designed/written). Sure, webpack goes through a similar transformation, but the difference is that a) material-ui is testing and designing for webpack’s specific transformations, and b) Snowpack takes the extra step of upconverting from CJS to ESM, which webpack does not do. And between all of these differences, material-ui hasn’t designed their package to be ESM-compatible (i.e. easily compatible with Snowpack).

Solving package issues where the original team won’t ship modern code is like a game of whack-a-mole. Changing how Snowpack handles one package could lead to breaking others, and we’re just shooting for wide support. So as much as I hate to say it, the quickest way to use material-ui is to use webpack, or give Vite a try (I don’t know if it works on Vite, but some packages work better on Snowpack and some on Vite, and this may be one of the latter).

@drwpow drwpow closed this as completed Aug 24, 2021
@Arrhont
Copy link

Arrhont commented Aug 25, 2021

But snowpack 3.0.13 somehow can handle that import system and start dev server on every material ui version i have tried. Why newer versions cant do that?

@theseyi
Copy link

theseyi commented Aug 25, 2021

But snowpack 3.0.13 somehow can handle that import system and start dev server on every material ui version i have tried. Why newer versions cant do that?

Was wondering the same, we are still running 3.0.13 as well at the moment. Seems the big refactor fixed some issues but led to big regressions that are difficult to get a handle one

@justinfarrelldev
Copy link

I wanted to also interject that for me, the solution to getting it to build with @mui/material was this:

import AppBar from '@mui/material/AppBar';

instead of using:

import { AppBar } from '@mui/material';

Hope this helps someone as lost as I was!

@jmcaree
Copy link

jmcaree commented Dec 28, 2021

I wanted to also interject that for me, the solution to getting it to build with @mui/material was this:

import AppBar from '@mui/material/AppBar';

This works for some of the MUI components, I'm having issues if I try to use e.g. Menu or Drawer

snowpack_mui.txt

Here's a working sample on CodeSandbox: https://codesandbox.io/s/reverent-lake-k8xuo?file=/src/Layout.tsx

The above does not build/run for me with Snowpack.

@jmcaree
Copy link

jmcaree commented Jan 28, 2022

In case anyone stumbles across this and is similarly stumped, the issue appears to be with circular references. There is a workaround suggested here:
#3724 (comment)

This worked in my case, using e.g. Drawer:
import Drawer from "@mui/material/Drawer/Drawer";

It looks like the MUI team are aware of the issue, also:
mui/material-ui#28628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working contributors welcome! contributors welcome!
Projects
None yet
Development

No branches or pull requests