FredKSchott / snowpack

ESM-powered frontend build tool. Instant, lightweight, unbundled development. ✌️
https://www.snowpack.dev
MIT License
19.48k stars 922 forks source link

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

Closed applike-ss closed 3 years ago

applike-ss commented 3 years ago

Bug Report Quick Checklist

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 commented 3 years ago

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

Danzo7 commented 3 years ago

downgrade to snowpack 3.0.13 will fix

i3ima commented 3 years ago

downgrade to snowpack 3.0.13 will fix

I hope this is a temporary solution

This also don't work for me

Danzo7 commented 3 years ago

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 commented 3 years ago

Same issues on my end

  1. SyntaxError: import not found: darken
    http://localhost:8080/_snowpack/pkg/@material-ui.lab.v4.0.0-alpha.43.js [:7:21] 
  2. 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 commented 3 years ago

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

matiosfree commented 3 years ago

@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 commented 3 years ago

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

kasper573 commented 3 years ago

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 commented 3 years ago

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

Hello World ` ![Capture](https://user-images.githubusercontent.com/5797896/114740416-8047cb80-9d52-11eb-920c-feca05967915.PNG) package.json >> "snowpack": "3.3.0", "@material-ui/core": "5.0.0-alpha.30",
jalovatt commented 3 years ago

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 commented 3 years ago

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/blue@4.11.3 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 commented 3 years ago

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

vikigenius commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

shripadk commented 3 years ago
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 commented 3 years ago

downgrade to snowpack 3.0.13 will fix

Can confirm that it works with Snowpack v3.0.13.

shripadk commented 3 years ago

Narrowed it down to the commit that causes this issue. Stems from this big internal rewrite of Snowpack: https://github.com/snowpackjs/snowpack/commit/bea1c56ce93a7a073cb0fabeb068f742e7095151

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 commented 3 years ago

@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 https://github.com/mui-org/material-ui/pull/26310. It could also explain why some of the suggested workarounds for MUI 4 don't work for MUI 5 alpha releases.

vikigenius commented 3 years ago

Additional point to note is that the prod build snowpack build seems to work fine. As I mention here https://github.com/mui-org/material-ui/issues/26568.

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

shripadk commented 3 years ago

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

canozo commented 3 years ago

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 commented 3 years ago

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/core@5.0.0-alpha.38/@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 commented 3 years ago

👋🏻 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 commented 3 years ago

@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 https://github.com/mui-org/material-ui/issues/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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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).

Arrhont commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

In case anyone stumbles across this and is similarly stumped, the issue appears to be with circular references. There is a workaround suggested here: https://github.com/withastro/snowpack/issues/3724#issuecomment-944884374

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: https://github.com/mui-org/material-ui/issues/28628