sass-loader: SassError: Can't find stylesheet to import when it contains @forward with _index file

  • Operating System: Mac OS
  • Node Version: v10.16.3
  • NPM Version: v6.13.4
  • webpack Version: v4.41.5
  • sass-loader Version: v8.0.2
  • dart-sass version: v1.25.0

Expected Behavior

Expected sass-loader to resolve the module when _index.scss files are used.

Actual Behavior

sass-loader throws following error when @import’ed Sass file contain @forward statement with _index file. (See reproducible steps below).

Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Can't find stylesheet to import.

Code

See complete example here: https://github.com/abhiomkar/sass-module-bug

How Do We Reproduce?

git clone git@github.com:abhiomkar/sass-module-bug.git
cd sass-module-bug
npm i
npx webpack

Notice the error:

[./fixture-import.scss] 434 bytes {main} [built] [failed] [1 error]

ERROR in ./fixture-import.scss
Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Can't find stylesheet to import.
  ╷
1 │ @forward "@material/base" as mdc-base-*;
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  node_modules/@material/button/_mixins.import.scss 1:1  @import
  /Users/abhiomkar/code/sass-bug-module/fixture-import.scss 1:9 

Changing webpack entry file fixture-import.scss => fixture-use.scss would fix the error.

UPDATE: See this comment for temporary workaround.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 29
  • Comments: 63 (45 by maintainers)

Commits related to this issue

Most upvoted comments

I did some investigation to see how the URLs are resolved. I’m not very familiar with how webpack importer works but seems like resolve() function in webpackImporter is resolving the url to index.js instead of index.scss or _index.scss file. (@trimox spotted this in his comment earlier about presence of index.js causing import failures).

Workaround for now is to disable webpackImporter option in sass-loader:

module.exports = {
  mode: 'development',
  entry: './fixture-import.scss',
  module: {
    rules: [
      {
        test: /\.s[ac]ss$/i,
        use: [
          'css-loader',
          {
            loader: 'sass-loader',
              options: {
+               webpackImporter: false,
                sassOptions: {
                  includePaths: ['node_modules'],
                },
                implementation: require('dart-sass'),
            },
          },
        ],
      },
    ],
  },
};

@evilebottnawi Hope this helps debug the issue 😃

I will work on that fix tomorrow

And please stop saying that there are bugs everywhere except your code, we are both developers and may make a mistake in our code, so I prefer to investigate firstly and then say where the problem is. Thanks

Anyway I found solutions, try to shipped it tomorrow

Starting working on “@import”/“@forward”/“@use” problems, sorry for the slight delay, covid 😞 approximate ETA is the end of the next week.

It will be major release:

  • fix bugs with our importer - logic for resolutions will be same as in dart-sass
  • drop supporting node@8
  • support file.import.ext
  • perf

In theory it should not break nothing, but in very very very some rare cases it can be broken due our invalid logic, so I prefer release that as a major release

I feel like things are getting a little antagonistic, so I want to take a step back and apologize for my part in that. I’m not trying to make your life difficult or blame you for stuff breaking, and it was definitely not very cool of me to list off behavioral issues in your code that you were already aware of.

I do hear you when you say that using the solution I propose will break other users, and when I push back on that it’s not that I think you’re wrong so much as that one of two things is true: I’m wrong, or I haven’t explained myself very clearly (or a combination of both as it seems is the case here). In either case I want to understand more details about exactly why it won’t work so I can either fix my suggestion or explain better.

I think both of us ultimately want to find a solution that works well for all Sass users and is as future-proof as possible. I’m sorry it’s taking so long to land on one, but I’m confident that if we work together we can get there.


From your @import "file" example, I’m thinking that I didn’t explain myself well. I was going to try to explain myself better with code in hand, but I tested the reproduction again against the latest version of this package and it looks like you already fixed the issue in aeb86f078d01460990aa8b06fce81b6fe7e3fbb6 😄.

Fixed in v9 https://github.com/webpack-contrib/sass-loader/releases/tag/v9.0.0 with some hacks, unfortunately, this can lead to loss of performance in some rare cases, we can do nothing here, hope sass (dart-sass) team improve custom resolving mechanism in future and we can avoid these problems.

The algorithm was improved and tested very well, but I’m still worried about some rare cases when we can do something wrong. So feel free to open a new issue with minimum reproducible test repo if you faced with problems, we will fix it.

@nex3 Tired 😞 , I’ve been trying to show you a design error for several months now and you’re not doing anything, I support many webpack packages integrated with many different tools, I do not want to show myself smarter than someone else, but I have enough knowledge to show all integrations problems. And you repeat the same thing several times, completely refusing to understand me. I think you need help from another sass core developer(s) here to understand the problems, can you tell who could help with this?

I deeply cannot understand why less team pretty quickly understood us and made improvements.

What is the problem to implement sass.resolve/sass.resolver/visitor api?

THE PROBLEMS

Number 1

  • Developer uses sass-loader
  • Developer setup includePaths: ['./node_modules/framework/']
  • Developer has node_modules/framework/color-utils/index.scss file
  • Developer has color-utils package with index.scss - ./node_modules/color-utils/index.scss
  • Developer write @import 'color-utils';
  • Developer run webpack

We run webpack.resolve and return ./node_modules/color-utils/index.scss to sass, but it is wrong. Because we did not take into account includePaths. Why didn’t we do it? Because webpack.resolve do not use includePaths from sass option. Why are we not doing this? Because it’s sass care. What if we take care of this? Because we have to duplicate the sass logic. Could duplication of logic be a bad idea?

Lastly you even have forImport argument here https://github.com/sass/dart-sass/blob/master/lib/src/importer/node/implementation.dart#L95.

Number 2

  • Developer uses sass-loader
  • Developer setup includePaths: ['./node_modules/framework/']
  • Developer has ./node_modules/framework/color-utils.scss file
  • Developer has alias with color-utils to ./src/utils/my-file-with-utils.scss
  • Developer write @import 'color-utils';
  • Developer run webpack

We run webpack.resolve and return ./src/utils/my-file-with-utils.scss to sass, but it is wrong. If you try to compile sass without webpack sass resolve @import ./node_modules/framework/color-utils.scss. We again have a problem with what we need to emulate sass resolution algorithm. And new improvements in sass resolutions algorithm will make us fix it again. It’s never ending.

Number 3

  • Developer uses sass-loader
  • Developer has ./custom-directory/unknown/index.scss
  • Developer write @import 'unknown';
  • Developer setup SASS_PATH equals custom-directory
  • Developer run webpack

We run webpack.resolve and spend time on looking alias/node_modules/relative paths/various extensions/other custom configured abilities (many memory and CPU) and no luck. Why did we spend this time/memory/cpu, I don’t know? By running sass.resolve we would not have to spend resources.

Why do these problems exist?

From source:

/// * The order of import precedence is as follows: /// /// 1. Filesystem imports relative to the base file. /// 2. Custom importer imports. /// 3. Filesystem imports relative to the working directory. /// 4. Filesystem imports relative to an includePaths path. /// 5. Filesystem imports relative to a SASS_PATH path.

We need to emulate logic for everything below /// 2. Custom importer imports.. If we were the last (i.e. 5) it would not be a problem. Above, I have already shown why we do it. Yes we implemented this hack/very terrible solutions https://github.com/webpack-contrib/sass-loader/blob/master/src/webpackImporter.js#L87 (we put working directory, includePaths and SASS_PATH in includePaths variable), but this is not entirely true. For example - we don’t throw an error on file.ext and _file.ext. Yes, we can improve it. But keep track of sass algorithm changes is too redundant for us.

If all you’re looking for is prioritizing plain-Sass import path resolution above custom importers, that’s something that will almost certainly be possible as part of sass/sass#2509.

What am I supposed to do here? Do you suggest me to start implementation and design?


Just say that you don’t want to fix it and save my and your time, it’s not so difficult. I’ll just embed the modified sass with fixed algorithm in sass-loader and setup automatic patching and releases. Honestly, I would have done it and no one would have had any problems.

@nex3

This is important because it means you don’t need to handle this in your importer. If you tell Sass “load package/index.ext”, Sass will know to convert that to package/index.import.ext if it’s coming from an @import rule. That’s why the current behavior already works!

You still can’t understand me 😞 I’m trying to show what the problem is and you don’t want to understand me for a long time.

I will try to explain everything again.

We emulate sass resolution algorithm before webpack resolution algorithm. We uses it because developer can setup includePaths, webpack resolver have more ability than sass resolver. Developer can setup own extensions, main files (index.ext/custom.ext/etc), main fields in package.json (sass, style, main etc) and more other abilities. He can even setup js extension, so webpack resolver return js file. Why? Because webpack is very big ecosystem. Developer can use alias, so @import 'package'; can be any resolution. But if developer set includePaths he wants to load from includePaths firstly and when try to load from webpack. Here a lot of use case, really a lot of use case.

Without emulation of sass resolution algorithm if developer setup includePaths this can lead to resolutions problems.

Here example for less-loader (no problems with resolving, because they improve it after same request) https://github.com/webpack-contrib/less-loader/blob/master/src/utils.js#L87. You can find that we first call less built-in resolver https://github.com/webpack-contrib/less-loader/blob/master/src/utils.js#L99, then if he did not resolve, then we call webpack resolver.

It is worth saying that sass is not fast, so we cached all resolver calls. Without emulation we can’t cache it.

I’ve been trying to explain this for a long time, but you either refuse to listen to me or don’t want to, honestly I don’t know.

I’d assume this is still in progress?

Looks like something broken on dart-sass side when you mixed @import and @use, our importer gets invalid URLs for resolve

@import should support _file.import.extand file.import.ext and prefer using .import files @use should not

Very strange, when we use @use our importer get those urls:

@material/button/mixins
@material/elevation/mixins
@material/base/mixins
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/theme/mixins
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/theme/variables
@material/animation/variables
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/ripple/mixins
@material/animation/functions
@material/animation/variables
@material/base/mixins
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/theme/mixins
@material/theme/functions
@material/theme/variables
@material/animation/variables
@material/theme/variables
@material/rtl/mixins
@material/theme/functions
@material/theme/mixins
@material/touch-target/mixins
@material/base/mixins
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/typography/mixins
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/shape/mixins
@material/feature-targeting/functions
@material/feature-targeting/mixins
@material/rtl/mixins
@material/shape/functions
@material/density/functions
@material/density/variables
@material/theme/variables
@material/elevation/functions

But when we use @import we have:

@material/button/mixins
@material/base <- invalid import

Investigate deeply.

@rutwick-alic, this issue and repository has nothing to do with your support request, try asking your question on the Slack channel of Stencil: https://stencil-worldwide.herokuapp.com

Also let’s run:

node_modules/.bin/dart-sass --load-path=node_modules fixture-use.scss

All work and fine.

But:

node_modules/.bin/dart-sass --load-path=node_modules fixture-import.scss

No output and no errors

Both of these invocations print the same output for me with the latest version of @abhiomkar’s repo. I think this is a legit sass-loader bug.

@nex3, @jathak would it be possible to revisit your implementation of @use in dart-sass given the issues it is causing? I think @evilebottnawi isn’t the only one who is puzzled by how this breaking change is being handled. I’ve noticed that both of you are Google employees, are you aware that the new feature is also disrupting projects maintained by Google, such as material-components-web?

For the time being, you can just return the non-import-only path and make use of the fact that file paths you return will be re-resolved for .import files by Sass if necessary.

No, I can’t:

  • It is break a module resolution in sass-loader
  • It is break PnP support
  • It is break alias resolution

Can you go into more detail about how and under what circumstances these will break? As in, walk me through an example import?

What is a problem to add new argument for importer?

I outlined a couple problems above:

  • It’s not generally a good practice to add API surface you know is going to be deleted.

  • New JS APIs have to go through a design process and, ideally, be implemented simultaneously in both Dart Sass and Node Sass. It’s not just a simple switch we can flip. If you’re interested in contributing to this design so it goes faster, you’re more than welcome to pitch in to sass/sass#2509.

In addition, the more we design the API to rely on individual importer authors to correctly implement all the intricacies of Sass’s import order, the more prone those importers will be to errors and inconsistencies like the ones being filed in this issue. Certainly they won’t be future-proof if/when the built-in import logic changes again. It may well be the case that a design that (more explicitly) calls out to the Sass implementation to do this resolution given a base file name is better than one that just throws a bunch of configuration information at the importer and trusts it to do the right thing.

Note that, looking through getPossibleRequests.js, it looks like there are already several waysthat the webpackimporter doesn’t follow standard Sass load-order conventions:

  • It doesn’t throw errors for ambiguous loads.
  • If you explicitly write @import "_foo.scss", it’ll also look for @import "__foo.scss".
  • It won’t load @use "_foo.css" (although arguably this is due to a work-around for https://github.com/sass/libsass/issues/2957).
  • It doesn’t seem to support index files at all.

Which isn’t to say this is your fault—load order is complicated and it changes with the language! It’s more to say that the webpack importer shouldn’t have to care about all this logic. Instead, more of it should be offloadable onto the Sass compiler.

Sorry guys, I can’t do something here.

I dove into the code again, and as far as I can tell the crucial missing piece doesn’t actually have anything to do with .import. It’s the last bullet point above: the lack of support for index files. If you set up the loader to look for ${request}/{_,}index.s{a,c}ss, @abhiomkar’s original reproduction starts working (although I also had to avoid using utils.urlToRequest() because it was adding an initial ./ that webpack didn’t like).

Because sass team do not want to help with the problem and ignore it, here two solutions:

  • remove webpack resolver, so no alias, no ~package requests, no other resolution abilities from webpack
  • remove includePaths from allowed sass options and do not use sass resolutions algorithm, only packages use @import 'package'; and package/index.import.ext will not work

/cc @nex3 friendly ping

I get that it feels philosophically strange, but practically speaking why is it causing problems? Isn’t it good that it will automatically detect .import files for an import?

Yes, it is strange.

For the time being, you can just return the non-import-only path and make use of the fact that file paths you return will be re-resolved for .import files by Sass if necessary.

No, I can’t:

  • It is break a module resolution in sass-loader
  • It is break PnP support
  • It is break alias resolution

What is a problem to add new argument for importer?

Changing the JS importer API is complex because it’s shared between Dart Sass and Node Sass. sass/sass#2509 is tracking a redesign, which @Awjin is actively working on.

I think it will be a breaking change so adding isImport as fourth argument is not a problem, node-sass doesn’t support @use, so for node-sass isImport will be true by default.

Without the isImport argument I can’t solve this issue if I implement .import logic it is break @use logic.

A huge part of developers use webpack and sass-loader so sometime in the past I asked you to pay attention to this problem.

I currently have a choice

  • Break module resolution (developers whom use bootstrap, foundation, bulma and other projects with @import will suffer).
  • Keep as is and material-components-web (projects use @use/@forward) will suffer.

I care too much about bugs/misleading behaviors/lacks of opportunity for 3d party dependencies. If you want the bug to be fixed, implement 4 argument as soon as possible.

Sorry guys, I can’t do something here.

I’m sorry, I didn’t mean to cast blame—I only meant to explain why you were seeing that output. @abhiomkar and I were actually initially investigating Dart Sass for a bug here before we ended up determining it was more likely a sass-loader issue and filing this.

The module system is informally documented on the Sass website. The section on imports and modules is particularly relevant here, since it describes how import-only files work.

For more detailed information, you can check out the Sass specification, particularly the sections on modules and @import.

Also let’s run:

node_modules/.bin/dart-sass --load-path=node_modules fixture-use.scss

All work and fine.

But:

node_modules/.bin/dart-sass --load-path=node_modules fixture-import.scss

No output and no errors