fix: import files directly from scoped npm package

[bpina]

This PR contains a:

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

Motivation / Use-Case

When a scoped NPM package import is encountered, sass-loader will prepare a handful of file variations to import. This results in several checks to import files that do not exist, which considerably hampers expected performance. This can be partially avoided by prefixing NPM packages with ~; however, when this is done inside of external packages, it is not something that can be controlled.

This is primarily driven by our use of https://github.com/material-components/material-components-web, which makes extensive use of cross-package imports. ex: https://github.com/material-components/material-components-web/blob/master/packages/mdc-textfield/mdc-text-field.scss#L23-L32

Breaking Changes

This change may effect projects where import paths follow the scoped module naming conventions @import @someNestedFolder/sassFile, where those import paths do not reside in node_modules. If these are local imports, this issue may be resolved by prepending ./ to ensure the import is handled relative to the requesting file.

Additional Info

Scoped NPM imports do still work without this change, but look-ups are not optimized for the scenario. This can result in hundreds of imports (depending on project) looking for ~6 non-existent files.

[jsf-clabot]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #679 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #679      +/-   ##
==========================================
+ Coverage   98.02%   98.06%   +0.03%     
==========================================
  Files           6        6              
  Lines         152      155       +3     
==========================================
+ Hits          149      152       +3     
  Misses          3        3

Impacted Files	Coverage Δ
lib/importsToResolve.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9162e45...c2b5623. Read the comment docs.

[alexander-akait]

using this we disable aliasing for scoped packages, it is breaking change and bad behavior

[bpina]

Thanks for checking out my PR and for sharing that information. Would it be possible to resolve the alias issue by having importsToResolve return [request, url] for this particular @import "@yada/pkg"; scenario, or is it necessary to work through full list of generated file paths?

[alexander-akait]

Here interesting moment, i think it can be solved new option resolver.
resolver: false - disable webpack resolved in favor built-in node-sass/sass resolver
resolver: true - use webpack resolver (by default)
resolver: function () { /* Logic */ } - allow to defined custom resolve instead webpack resolver (your case)

One note - node-sass/sass support importer (webpack resolver use this feature), so you can solve this in other way:

• disable webpack resolver
• implement own importer

[bpina]

@evilebottnawi I think having some configurable option for this would be great! I do have a couple of questions to see if I'm understanding the terminology correctly.

Would this resolver configuration replace webpackImporter or importsToResolve?

If you mean configuring a replacement for importsToResolve, I think webpackImporter is still useful, but providing a way to configure where to look could enhance it slightly (at least for this use case).

If you mean configuring whether webpackImporter is automatically added to the list of importers (resolver: true|false), does it make sense to simply enable/disable webpackImporter and support replacement with additional importer: [] configurations?

I would vote for option 1, configuring/replacing importsToResolve. Though this does look more complex, given how loader.js invokes webPackImporter outside of options loading. Either solution would support this use case.

[alexander-akait]

We can export importsToResolve and allow define own importers before/after defaults importers, it is allow to override logic where you need and use default logic for other

[bpina]

Closing in favor of #682