feat: add fixture that fails to resolve to _index.import.scss

[bgotink]

This PR contains a:

• bugfix
• new feature
• code refactor
• test update
• typo fix
• metadata update

Motivation / Use-Case

The resolution of .import.scss files in sass-loader differs from the resolution of sass itself. (#904)

While creating the reproduction I noticed everything works fine if I add a _index.scss file next to the _index.import.scss file.
That being said, in our case we have files we explicitly only want to expose to @import and not to @use, so we have files that only exist with the .import.scss extension. This works with relative imports in sass but not when using ~ imports via sass-loader.

Adding an extra file without the .import in the filename—e.g., a file that contains an @error to fail when loaded—is a pretty clean workaround. We'll use that, though it would of course be great if we could skip that.

The error while running the tests:

 FAIL  test/loader.test.js (40.018 s)
  ● loader › should import .import.scss files without .scss file next to it (dart-sass) (scss)

    evalmachine.<anonymous>:13
    throw new Error("Module build failed (from ../src/cjs.js):\nSassError: Can't find stylesheet to import.\n  ╷\n1 │ @import '~only-import';\n  │         ^^^^^^^^^^^^^^\n  ╵\n  test/scss/only-import.scss 1:9  root stylesheet");
    ^

    Error: Module build failed (from ../src/cjs.js):
    SassError: Can't find stylesheet to import.
      ╷
    1 │ @import '~only-import';
      │         ^^^^^^^^^^^^^^
      ╵
      test/scss/only-import.scss 1:9  root stylesheet

      at Object../scss/only-import.scss (evalmachine.<anonymous>:13:7)
      at __webpack_require__ (evalmachine.<anonymous>:36:41)
      at evalmachine.<anonymous>:46:18
      at evalmachine.<anonymous>:47:12
      at getCodeFromBundle (test/helpers/getCodeFromBundle.js:21:21)
      at Object.<anonymous> (test/loader.test.js:1465:34)

[alexander-akait]

I see, give me some time to debug sass resolution, it is easy to fix

[alexander-akait]

I am afraid we can't fix it, because sass (dart-sass) doesn't provide API for this, we should load index.import only for @import, but we can't determinate used at-rule (it is @import or @use)

/cc @nex3 We returned again to the old problem, which you absolutely do not want to fix

[alexander-akait]

@bgotink My recommendation don't use ~ and use includePaths with path to node_modules

[nex3]

@alexander-akait If you return "test/scss/only-import.scss" to Dart Sass, it will automatically load the import-only file if applicable (even if the physical test/scss/only-import.scss file doesn't exist).

[alexander-akait]

@nex3 yep, looks good like workaround

[alexander-akait]

@nex3 oh, no, I can't, developer can write @use 'index-import/_index.import.sass' or @import 'index-import/_index.import.sass' or @import 'index-import' or @use 'index-import' and I can't remove .import because I don't know context, what is the problem provide is it @use or @import?

I really can't understand - no new API for resolving and no actions on current problems and bugs

[alexander-akait]

Just note - webpack resolve works like require.resolve() with more abilities, but I can't get context of resolved source, so I don't know it was resolved like index file in directory, or as absolute/relative path with extensions or something else

[nex3]

If you just resolve the directory and use the same base filename that the user passed in, the Sass compiler will automatically do the rest of the resolution for you. You don't need to care about whether the path has .import or not or whether the file is being loaded via @import or @use.

[alexander-akait]

@nex3 I can't do it

If you just resolve the directory and use the same base filename that the user passed in, the Sass compiler will automatically do the rest of the resolution for you. You don't need to care about whether the path has .import or not or whether the file is being loaded via @import or @use.

Because when I run require.resolve('foo') it can be foo.scss file and can be foo/index.scss and can be foo/_index.scss and etc, I don't know context of resolving, if i insert additional interrupts with check we reduce sass-loader performance ~50-100%

[alexander-akait]

@nex3 I'm already ready to beg you to listen to us, currently sass-loader code it is hacky code with ton of workarounds, just to let the resolving system work (webpack + sass), all i need to reduce complex and improve performance - it is @use or @import used (just small variables with simple value) and can even implement file based cache which will increase the performance of most projects at least 2-3x, and forever solve the problems with resolving with which I suffer the last 6-12 month, i just beg you

[nex3]

Because when I run require.resolve('foo') it can be foo.scss file and can be foo/index.scss and can be foo/_index.scss and etc, I don't know context of resolving, if i insert additional interrupts with check we reduce sass-loader performance ~50-100%

All of that is fine—just return foo/index.scss or foo/_index.scss, whatever require.resolve('foo') returns, and Dart Sass will automatically turn it into foo/index.import.scss if necessary. Why wouldn't that work?

[alexander-akait]

@nex3

Why wouldn't that work?

It will work but it require a lot of extra logic and it reduces perf very, if you want to have best perf, we need information about it was @import or @use

[nex3]

What extra logic does it require? I'm essentially suggesting that you ignore the existence of .import.scss files and let Dart Sass handle it.

[alexander-akait]

@nex3 we need simulate sass logic, because webpack has aliases/fallbacks/etc mechanisms, also developer can write @import 'file.import.scss', so if we remove it, it will not work

[nex3]

All of those aliases and fallbacks should happen prior to resolving the import-only file. If the user aliases foo to bar and you return bar.scss, then Sass should look for bar.import.scss. It doesn't matter if foo.import.scss also exists; it's just ignored.

Writing @import 'file.import.scss' should also just work. You find a file with that name the way you normally would, return it to Sass, and Sass knows not to look for file.import.import.scss.

[alexander-akait]

@nex3 But we don't know real filename, developers can use @import 'alias';, but alias can be dir/index.scss, dir/index.import.scss, index.import.scss and etc, it can be any

[nex3]

After you pass alias to require.resolve(), it returns some SCSS file that Webpack found, right? Let's say it's dir/index.scss. Then you pass this back to Sass. Sass checks if dir/index.import.scss exists if necessary, without needing you to do anything special.

[alexander-akait]

What about if alias will be dir/index.import.scss, when we have in source code @import 'alias'; I don't know what developer want to load, it can be any - dir/index.scss or dir/index.import.scss or something else, so I don't know what I need to return, because I don't know what was used @import or @use

[nex3]

If someone explicitly aliases 'alias' to dir/index.import.scss, then it should load the import-only stylesheet regardless of whether it's through @use or through @import—same as if someone wrote @use 'dir/index.import.scss' directly.

[alexander-akait]

@nex3 it is the problem, because I don't know what is user provide, resolver can be configured in different ways, it supports plugins too, above you told me that I should just delete .index and allow sass do job instead of sass-loader, but I can't remove .index in this case, because I don't know should I remove it or not

[nex3]

I'm not saying you should ever remove .import. I'm just saying you don't have to add it. Just ignore the fact that it might exist.

[alexander-akait]

@nex3 I am not adding anything, this value from importer, this is not the first time I repeat that for seamless integration I need to know is this @import or @use, what else do I need to explain in more depth?

1. our importer get value from @import or @use
2. we try to resolve it, but we don't known what is this, it can be with .import, without .import, it can be any file, it even can be loaded from network
3. when I try to resolve it in node_modules or in other modules directory I need to know should I resolve .import files or not, if I will always resolve without .import it can be broken for aliases, if I will resolve without .import we will get problem as here

[nex3]

3. when I try to resolve it in node_modules or in other modules directory I need to know should I resolve .import files or not, if I will always resolve without .import it can be broken for aliases, if I will resolve without .import we will get problem as here

This is the part I don't understand. An importer that returns file paths should never need to manually look for .import files. I believe I explained above that even with aliases, you just return the filename without .import and the compiler will add .import if necessary. What precisely is the problem you run into?

[alexander-akait]

An importer that returns file paths should never need to manually look for .import files.

We do not do this and we have this problem (described above). We have @import '~only-import' (i.e. node_modules/only-import/index.[ext] or node_modules/only-import/index.import.[ext]), but we don't look at .import files, so resolver throw an error (can't resolve, because we have node_modules/only-import/index.import.[ext] ) and we return path as is to sass, but sass can't resolve @import '~only-import' (even @import 'only-import' sass still can't resolve, because it is inside node_modules). If we allow resolve .index files we will get problem described above (problem with aliases and etc). This is why it is so important to know is it @import or @use, this will allow me to setup resolver in right way.

[nex3]

Okay, let me see if I'm understanding correctly:

• One user writes @import "~foo" and another writes @use "~foo".
• Webpack's internal black-box logic resolves foo first to some_pkg/foo1 and then to some_pkg/foo2.
• some_pkg/foo1.import.scss exists, but some_pkg/foo1.scss does not. However, some_pkg/foo2.scss does exist.
• So you don't know whether to return some_pkg/foo1.scss or some_pkg/foo2.scss without knowing whether you're resolving an @import or a @use. Is that right?

[alexander-akait]

@nex3 and other case:

• One user writes @import "~foo" and another writes @use "~foo".
• foo/index.import.scss exists, but foo/index.scss does not. In other case foo/index.scss doesn't exists, but foo/index.scss exists, in the third case both foo/index.import.scss and foo/index.scss existing, foo can be directory or package
• So you don't know whether to return foo/index.import.scss or foo/index.scss without knowing whether you're resolving an @import or a @use. Is that right?

[alexander-akait]

@nex3 friendly ping

[nex3]

Finally found time to circle back to this. I'm planning to add a fromImport field to the importer's this context. Will that work for you?

[alexander-akait]

Yes, it will help 👍 Maybe we can do it better fromType: 'import' | 'use' | 'something-else', so if we will add more at-rules we just add a new type without new public property

[nex3]

We don't plan to add any new modes of loading files that don't re-use module semantics for a long, long time if ever. Also, I think listing "use" as a term for "module-system semantics" could be confusing since @forward and meta.load-module() also use those semantics.

[alexander-akait]

It was just an idea, maybe using fromType: 'import' | 'use' | 'forward' | 'other-value' is not bad, custom resolvers will be able to implement any logic, based on at-rule name, I can't give many useful examples right now, but I feel that it can be useful, in any case, the last word is yours

[alexander-akait]

@bgotink sorry for delay, fixed by #906