import prefer `js` over `css/sass/scss`

[larpo]

It appears that 7.0 breaks sass modules we are using from npm. I guess it is related to https://github.com/webpack-contrib/sass-loader/pull/479

For example foundation-sites that we are using imports it's internal modules like this:

// Sass utilities
@import 'util/util';

Now, unfortunately 'util/util' is a valid file inside node_modules (from 'node-util' pkg), and the build fails with errror like this:

ERROR in ./src/style.scss (./node_modules/css-loader!./node_modules/sass-loader/lib/loader.js!./src/style.scss)
Module build failed: 
// Copyright Joyent, Inc. and other Node contributors.
^
      Invalid CSS after "...N THE SOFTWARE.": expected 1 selector or at-rule, was "var formatRegExp = "
      in ###/webpack-sass-broken-imports/node_modules/util/util.js (line 1, column 1)
 @ ./src/style.scss 2:14-117
 @ ./src/index.js

repro repo: https://github.com/larpo/webpack-sass-broken-module-imports

Is there a way to revert to the old behaviour? I don't see any mentions on READMe about how to configure the aliasing behaviour.

[alexander-akait]

@larpo Confirm, wip

[alexander-akait]

/cc @jhnns looks like regression

[jhnns]

Yep, fix is one the way. Thanks for the test case 👍

[alexander-akait]

@jhnns can you take care?

[larpo]

Good to know it is a regression and not the intended behaviour :)

[jhnns]

Shipped as 7.0.1 🚀.

That's why reproducible test cases are so good 👍

[alexander-akait]

@jhnns looks like we still have problem :confused:

[alexander-akait]

@jhnns found problem: I have in directory two file with same name component.js and component.scss , also have app.scss with code

@import "component";

[alexander-akait]

@jhnns looks like problem here https://github.com/webpack-contrib/sass-loader/blob/master/lib/loader.js#L61, if you have two file with difference extensions, resolve prefer .js :confused:

[julkue]

Hello guys, I'm coming here since I'm also experiencing issues in this PR about updating to v7: https://github.com/julmot/form-components/pull/14#issuecomment-381153653 The build fails now with an error that looks like an issue of the loader to me: https://travis-ci.org/julmot/form-components/builds/366135805#L4146

[dak]

Just confirming I'm seeing the same issue as @evilebottnawi

[alexander-akait]

@julmot @dak I have PR https://github.com/webpack-contrib/sass-loader/pull/558, Can you test this and confirm what is fixed your problem

[julkue]

@evilebottnawi Yes, the build passes with this branch. @dak npm install git://github.com/webpack-contrib/sass-loader.git#fix-resolve-conflict --save-dev

[dpikt]

@evilebottnawi PR #558 fixes the issue for me as well.

[ioanungurean]

@larpo the loader works now for me too. I had errors with v7.0.0 and v7.0.1.

Use: npm install git://github.com/webpack-contrib/sass-loader.git#fix-resolve-conflict --save-dev as @julmot pointed out.

Thank you for the PR @evilebottnawi!

[julkue]

Please let us know when the PR is merged.

[ioanungurean]

What it's taking so long to be reviewed? The PR was open 10 day ago. :cry:

[phahn]

This is also causing issues for me in facebook/create-react-app#3815

[alexander-akait]

/cc @jhnns looks we should search solution for this asap

[jhnns]

We need to go a different route (see here). I'm thinking about making webpack's resolver opt-in and moving back to Sass' regular behavior.

Until then you should use webpack's resolve.modules option instead of node-sass' includePaths option if you experience any problems.

[julkue]

Ping guys

[ioanungurean]

I did not upgrade to ^7.0.0 because my build breaks. I'm staying with 6.0.7 for now.

[julkue]

I do too, but I'd rather like to have a solution for this issue (not a workaround) instead of keeping the old version.

[alexander-akait]

In near future i will send PR with right fix, sorry guys a lot of work right now

[ghost]

I'm setting up a new site, and bumped into this issue again.

Are folks here still sticking to v6.0.7 to avoid breakage?

[jsg2021]

yes, at least i am.

[ioanungurean]

Yes, unfortunately. :disappointed:

[alexander-akait]

WIP on this, but it is really hard to solve keeping aliases and support includePaths

[alexander-akait]

Workaround: always using .sass/.scss extension on any import inside sass/scss files. Example:

@import "path/to/import.scss";

a {
    color: red;
}

[emilio-martinez]

Bumped into this issue. In my case, I was using the node-sass includePaths option rather than the webpack resolution (we have other use cases that don't involve building with webpack). I was able to resolve by including the corresponding extension when referencing the problematic .sass/.scss files.

[alexander-akait]

FMI: hack how we can solve problem, but with limitation, aliasing for css/scss/sass only support alias with extension css/sass/scss, but we can add this to readme

sokra [12 minutes ago]
`this._compilation.resolverFactory.get("normal", { ...this._module.resolveOptions, extensions: [...] })`

[jsg2021]

@jhnns I was under the impression the normal sass/css imports happened by default, and node_module's needed the import with ~ prefix to use webpack's resolver... is this not the case anymore?

[jhnns]

I was under the impression the normal sass/css imports happened by default, and node_module's needed the import with ~ prefix to use webpack's resolver... is this not the case anymore?

No. This was never the case. We used to just do Sass' native resolving until people wanted to alias packages. Then we switched to webpack's resolver which also enabled people to apply loaders on Sass imports. This, however, came with problems. node-sass seems to have a threadpool issue if node's fs module is used to load the Sass files (which webpack does and libsass does not). This leads to stuck compilations. As a result, we needed to introduce a queue which leaves one thread available for other fs tasks. My assumption is that this is the cause for slow compilations in big projects.

You see that we're in a kind of complicated situation now 😁

My current plan is:

• Switch back to regular Sass imports by default. Which means that you can use includePaths but cannot alias paths and cannot use loaders. This is just regular Sass and I think this should be fine for most people
• We'll still support webpack's resolver (for now), but then you can't use includePaths because the same can be achieved by using resolve.modules inside your webpack.config.js.

What do you think?

[alexander-akait]

@jhnns we can fix this using advice from @sokra :

this._compilation.resolverFactory.get("normal", { ...this._module.resolveOptions, extensions: [...] })

And write in docs what aliasing working only with css/sass/scss. We have a lot of project where use aliases and using includePaths to allow working some legacy sass libraries and frameworks

[emilio-martinez]

Perhaps something relevant to this conversation if Webpack is not doing the module resolution:

As of node-sass@4.9.0

@importing of .css files will produce a deprecation warning. This behaviour will be removed in node-sass@5.0.0.

This behaviour violates the Sass language specification. Additionally it makes it impossible to output to the input directory via the CLI or watcher.

[julkue]

@jhnns I can not confirm that this is working:

I still get the following with the new major version (only):

Is there a solution that I can use to finally merge https://github.com/julmot/form-components/pull/14?

[alexander-akait]

@larpo because it is works only for modules, i.e. ~module/path/to/file.scss

[julkue]

So, what would be the workaround then?

[alexander-akait]

@julmot https://github.com/webpack-contrib/sass-loader/issues/556#issuecomment-394019055

[julkue]

@evilebottnawi Okay, thanks. If this is the only workaround – modify the source code (import) in all imports – for this bug I think I'm going to prefer to keep the old version. Please keep us posted when this is resolved without modifying the source.

[alexander-akait]

@julmot it is in my todo, i think it will be solve in this week :+1:

[alexander-akait]

@julmot in your example you forget resolve.extensions: [".scss", ".sass", ".css", "list-of-your-extensions-or-standard-js-and-json"]

[JakeChampion]

@evilebottnawi Is there anything we can do to help with this issue?

[alexander-akait]

@JakeChampion don't have time right now, feel free to search solution, you can try

this.resolver = this._compilation.resolverFactory.get("normal", { ...this._module.resolveOptions, extensions: ['scss', 'sass, 'css'] })

change inside loader and try it

[julkue]

I still can't upgrade to sass-loader v7 due to this bug.

[alexander-akait]

@julmot can you create minimum reproducible test repo?

[joshdcomp]

In case it can help someone else, this worked for me:

In your Webpack config's module.rules[], move your rules for non-js assets to be evaluated before the js, so:

module: {
    rules: [
      {
        test: /\.js$/,
        exclude: /node_modules/,
        use: {
          loader: 'babel-loader'
        }
      },
      {
        test: /\.(ttf|eot|woff|woff2|svg)$/,
        use: {
          loader: "file-loader",
          options: {
            name: "[name].[ext]",
          },
        },
      },
      {
        test: /\.scss$/,
        use: ExtractTextPlugin.extract({
          fallback: "style-loader",
          use: ['css-loader', 'sass-loader']
        })
      }
    ]
  },

becomes:

module: {
    rules: [
      {
        test: /\.(ttf|eot|woff|woff2|svg)$/,
        use: {
          loader: "file-loader",
          options: {
            name: "[name].[ext]",
          },
        },
      },
      {
        test: /\.scss$/,
        use: ExtractTextPlugin.extract({
          fallback: "style-loader",
          use: ['css-loader', 'sass-loader']
        })
      },
      {
        test: /\.js$/,
        exclude: /node_modules/,
        use: {
          loader: 'babel-loader'
        }
      },
    ], 
  },

[tarjei]

@evilebottnawi any chance that the fix above[1] will be merged into the main package?

https://github.com/bencergazda/sass-loader/commit/3787121b45ff473bece63ba66f5b3a381d0ebd8e

[alexander-akait]

/cc @jhnns looks we need solve this bug, solution above looks good, other solution is add support custom options (fields, extensions and etc) to (resolver.resolve) (i can send a PR with this features), what do you think?

[antonellil]

experiencing the same JS precendence issue as well, currently unable to upgrade to 7.*.* from 6.0.7. hitting the error below when webpack is compiling *.scss that @import 'colors.scss'