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

Sass loader cannot resolve modules from Yarn v2 PNP #15753

Closed
nicholaschiang opened this issue Jul 31, 2020 · 18 comments
Closed

Sass loader cannot resolve modules from Yarn v2 PNP #15753

nicholaschiang opened this issue Jul 31, 2020 · 18 comments

Comments

@nicholaschiang
Copy link
Contributor

Bug report

Describe the bug

Next.js does not resolve SCSS that is imported by my dependencies:

  1. styles/home.module.scss uses @material/elevation (this works fine).
  2. @material/elevation uses @material/animation (this fails).

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. First, clone this repository and CD inside:
$ git clone https://github.com/nicholaschiang/with-sass
$ cd with-sass/
  1. Then, make sure you have Yarn installed then run:
$ yarn
  1. Finally, start the development server by running:
$ yarn dev

Error

You'll notice that when you run yarn dev the Next.js server starts up properly
but the sass-loader is unable to resolve my dependency's imports.

It seems to import the @material/elevation package just fine:

// styles/home.module.scss
@use '@material/elevation

But it doesn't seem to be able to import @material/animation for the
@material/elevation package:

error - ./styles/home.module.scss (./.yarn/$$virtual/css-loader-virtual-393ee7c517/0/cache/css-loader-npm-3.5.3-0f886851e6-910936f0ac.zip/node_modules/css-loader/dist/cjs.js??ref--5-oneOf-3-1!./.yarn/$$virtual/next-virtual-4df8b3b97e/0/cache/next-npm-9.5.1-e14f31e6e9-effa9056b8.zip/node_modules/next/dist/compiled/postcss-loader??__nextjs_postcss!./.yarn/cache/resolve-url-loader-npm-3.1.1-cf1a268137-7b113ac9e6.zip/node_modules/resolve-url-loader??ref--5-oneOf-3-3!./.yarn/$$virtual/sass-loader-virtual-dbafe3ebab/0/cache/sass-loader-npm-8.0.2-f0d209ad64-e23d9b308f.zip/node_modules/sass-loader/dist/cjs.js??ref--5-oneOf-3-4!./styles/home.module.scss)
SassError: Can't find stylesheet to import.
   ╷
23 │ @use "@material/animation/variables";
   │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ╵
  .yarn/cache/@material-elevation-npm-7.0.0-aaa8862010-5ea26d080e.zip/node_modules/@material/elevation/_variables.scss 23:1  @forward
  .yarn/cache/@material-elevation-npm-7.0.0-aaa8862010-5ea26d080e.zip/node_modules/@material/elevation/_index.scss 1:1       @use
  /home/nchiang/repos/with-sass/styles/home.module.scss 1:1                                                                                                                  root stylesheet

I've had this error before when using Next.js with
MWC but was
able to fix it by specifying an includePaths configuration in next.config.js
like so:

// next.config.js
const path = require('path');

module.exports = {
  sassOptions: {
    includePaths: [
      path.resolve(__dirname, 'node_modules'),
    ],
  },
};

Now, I'm not sure how to tell sass-loader to use Yarn PNP's module resolution.

Expected behavior

Next.js should be able to resolve the Yarn PNP modules.

System information

  • OS: Ubuntu 18.04.2
  • Version of Next.js: 9.5.1
  • Version of Node.js: 14.7.0
@nicholaschiang
Copy link
Contributor Author

Seems like #6049 was closed rather prematurely.

@nicholaschiang
Copy link
Contributor Author

Perhaps this could be fixed by using a custom importer: https://github.com/sass/node-sass#importer--v200---experimental

@merceyz
Copy link
Contributor

merceyz commented Jul 31, 2020

The issue is that sass-loader tries to load ./@material/animation/_variables.scss from @material/elevation/_variables.scss which doesn't work.
The relative path is produced here in 9.0.0 https://github.com/webpack-contrib/sass-loader/blob/c6d56e48728eb8d65258e6ee7606507187e0b457/src/utils.js#L223 and here in 8.0.2 https://github.com/webpack-contrib/sass-loader/blob/03773152760434a2dd845008c504a09c0eb3fd91/src/importsToResolve.js#L24

So it tries to look for these

[
  './@material/animation/_variables.scss',
  './@material/animation/_variables.sass',
  './@material/animation/_variables.css',
  './@material/animation/variables.scss',
  './@material/animation/variables.sass',
  './@material/animation/variables.css',
  './@material/animation/variables',
  '@material/animation/variables'
]

from C:\with-sass\.yarn\cache\@material-elevation-npm-7.0.0-aaa8862010-5ea26d080e.zip\node_modules\@material\elevation

@evilebottnawi is the module import @material/animation/variables supposed to become relative?

@nicholaschiang
Copy link
Contributor Author

nicholaschiang commented Jul 31, 2020

It seems as if the issue originates from loader-utils assuming that @material/animation/variables is a relative URL:

const url = "@material/animation/variables";
const request = loaderUtils.urlToRequest(url); // "./@material/animation/variables"

This is not the case. So either MWC changes all of their imports to prepend a ~ (which I don't think they'll do), or loader-utils has to stop assuming that all paths are by default relative (and instead assume that all paths are by default modules unless ./ is preprended).

@alexander-akait
Copy link
Contributor

alexander-akait commented Jul 31, 2020

Please do not use Yarn v2 PnP with webpack@4, only webpack@5 support PnP, also my strong recommendation - do not use includePaths and PNP, it is compatibility, because sass do not use require.resolve for @import and can't load virtual files 😄

You can try https://github.com/arcanis/pnp-webpack-plugin, but I don't know real compatibility

@alexander-akait
Copy link
Contributor

@merceyz and yes and not https://github.com/webpack-contrib/sass-loader/blob/c6d56e48728eb8d65258e6ee7606507187e0b457/src/utils.js#L234, contains ['./@material/animation/variables', '@material/animation/variables'], because you can have a directory with @ character in name

@nicholaschiang
Copy link
Contributor Author

@evilebottnawi I switched to Webpack v5 and I got the same error.

nicholaschiang added a commit to tutorbookapp/tutorbook that referenced this issue Jul 31, 2020
Uses the temporary fix proposed by @4cm4k1 in vercel/next.js#15026,
Timer/cssnano-preset-simple#6, and cssnano/cssnano#927.

Also temporarily fixes Sass module resolution by using the Yarn v2
nodeLinker option (until it's fixed upstream; see vercel/next.js#15753).
@merceyz
Copy link
Contributor

merceyz commented Jul 31, 2020

@evilebottnawi The PnP plugin is already installed, the import @material/animation/variables causes sass-loader to try to load this list:

[
  './@material/animation/_variables.scss',
  './@material/animation/_variables.sass',
  './@material/animation/_variables.css',
  './@material/animation/variables.scss',
  './@material/animation/variables.sass',
  './@material/animation/variables.css',
  './@material/animation/variables',
  '@material/animation/variables'
]

Would it make sense to also try these?

  '@material/animation/_variables.scss',
  '@material/animation/_variables.sass',
  '@material/animation/_variables.css',
  '@material/animation/variables.scss',
  '@material/animation/variables.sass',
  '@material/animation/variables.css',

Or is this behavior expected and therefor it's an issue with how @material/elevation/_variables.scss declares its import and @material/animation/variables should be changed to @material/animation/_variables.scss?

@nicholaschiang
Copy link
Contributor Author

@merceyz that suggestion looks great! According to the Sass lang spec, the @material/animation/variables is definitely the correct way to import that partial (plus, that MWC package is maintained by Google, so I'm pretty sure they got their stuff right haha).

@alexander-akait
Copy link
Contributor

alexander-akait commented Aug 1, 2020

@nicholaschiang Are you sure what you uses latest version? Where you got list?

Anyway adding @material/animation/_variables make sense, anyway we need reproducible test repo with error? Can you open a new issue with minimum reproducible test repo with error?

@nicholaschiang
Copy link
Contributor Author

@evilebottnawi there's already a repro linked in the original issue description (it's the default Next.js template with a SCSS import added).

And I was using Webpack 5.0.0-beta.22 originally (I can add it to the repro if you want too) by specifying Yarn resolutions (as per the instructions in #13341) when I first encountered this issue.

@alexander-akait
Copy link
Contributor

Thanks, I will look at this in near future

@alexander-akait
Copy link
Contributor

alexander-akait commented Aug 4, 2020

Just note - next.js uses sass-loader@8, but latest is sass-loader@9

@alexander-akait
Copy link
Contributor

Confirmed, it is bug, reproduced with sass-loader@9, WIP

@alexander-akait
Copy link
Contributor

Resolution in sass is terrible 😞

@alexander-akait
Copy link
Contributor

Fixed, ETA is tomorrow, I found other strange resolution bug (very rare case), anyway you need to update sass-loader to latest, it will not work with sass-loader@8

@merceyz
Copy link
Contributor

merceyz commented Sep 11, 2020

Fixed in #16970

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants