fix: respect webpack aliases (resolve.alias)

[alexander-akait]

Fixes #410

[codecov]

Codecov Report

Merging #479 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #479   +/-   ##
=======================================
  Coverage   97.58%   97.58%           
=======================================
  Files           6        6           
  Lines         124      124           
=======================================
  Hits          121      121           
  Misses          3        3

Impacted Files	Coverage Δ
lib/webpackImporter.js	100% <ø> (ø)	⬆️
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 6439cef...44f1d8c. Read the comment docs.

[michael-ciniawsky]

👍

[michael-ciniawsky]

fix or better feature ? :)

[alexander-akait]

@michael-ciniawsky more fix, than feature, webpack by default support aliases, need sombody who know wepback and sass to be sure that all good

[joshwiens]

@evilebottnawi - @jhnns has been busy but he's still available when needed. Just ping him on slack like you would Tobias if you need him to look at a PR.

[jhnns]

Thanks for your PR.

The code looks good, but I'm not sure about the semantics. Webpack aliases can only alias module requests, which are usually requests to node_modules. In JS, these requests must not start with a . or / or [a-z]:/. In CSS, and all CSS-like languages like Sass and Less, module requests must start with a ~ to distinguish them from relative requests. I'm not happy about the ~ in general, but that's how we currently handle it in the webpack ecosystem.

This change makes it possible to alias regular relative requests which is not possible in JS. If you wanted to alias a module request, you needed to write @import "~import-foo-as-alias" and I think this already works. In fact, I tried out your test without the lib modification and aliasing `"~import-foo-as-alias" works.

So, personally, I'd rather keep the current behavior because it is consistent with the other CSS loaders.

However, I would like to merge this PR without the changes to lib. If you're ok with it, I would just add my local modifications, a small note in the README and merge it. What do you think?

[jhnns]

Thanks for spotting this. Seems like it slipped through with some of the last commits...

[alexander-akait]

@jhnns big bad problem with aliases is nonsupporting this inside node_modules, because we can't add ~ inside other libraries/frameworks. To be honest, I do not know how to solve this, in theory we can add option where developers will add own aliases, anywhere here https://github.com/webpack-contrib/sass-loader/blob/master/lib/importsToResolve.js#L16. Example:

{
                loader: "sass-loader",
                options: {
                    includePaths: ["absolute/path/a", "absolute/path/b"],
                    aliases: { /regex/: 'path/to/something' } // as in webpack
                }
            }

I agree this look strange, but we can explain in README why we use this solution. I can rewrite this here if we will accept this. Thanks!

[alexander-akait]

/cc @jhnns friendly ping, what do your think about options.alias?

[jhnns]

@evilebottnawi Sorry for being unresponsive.

I won't add any additional custom aliasing to the sass-loader since it is already complicated enough. If webpack's module.alias option would be more powerful, this could be solved by anyone.

However, I think your use case should be solvable today by writing a custom importer for this particular file:

const nodeSass = require("node-sass");

// webpack.config.js
module.exports = {
    ...
    {
        loader: "sass-loader",
        options: {
            importer: [
                // url will be the string passed to @import
                // prev is the file where the import was encountered
                (url, prev) => (
                    shouldBeAliased(url) ?
                        { file: require.resolve("path/to/alias.sass") } :
                        // pass file to the next importer
                        // the last importer is the webpack importer inserted by the sass-loader
                        nodeSass.types.Null()
                )
            ]
        }
    }
};

I haven't tested this, but this should work. Please let me know if it actually did 😁. Or try out the node-sass-magic-importer which may solve this for you already...

You can also write a custom resolver for webpack, but this is more complicated:

class ResolverPlugin {
    constructor(options) {
        this.options = options;
    }
    apply(resolver) {
        resolver.plugin("described-resolve", (request, callback) => {
            // request is an object that contains meta information about the resolve request
            // request.request is a string that has been passed to the resolver, similar to url in the custom importer
            // request.context.issuer is an absolute path to the file that wants to import the request

            if (shouldBeAliased(request.request) === false) {
                callback();

                return;
            }

            resolver.doResolve(
                "resolve",
                Object.assign({}, request, {
                    request: require.resolve("path/to/alias.sass")
                }),
                "aliased sass file",
                callback
            );
        });
    }
}

module.exports = class AliasSassFilePlugin {
    constructor(options) {
        this.options = options;
    }

    apply(compiler) {
        compiler.plugin("after-resolvers", () => {
            compiler.resolvers.normal.apply(new ResolverPlugin(this.options));
        });
    }
}

[mischkl]

@evilebottnawi @jhnns As I mentioned in #466 this means webpack is essentially incompatible with a large segment of third-party scss modules on NPM, including material components from Google. For anyone who wants to use such packages to need to build in this kind of hack seems like a poor solution. Especially considering that users of e.g. @angular/cli have no such ability to configure their build in this way.

What we really need IMHO is for sass-loader to process imports that occur within third-party modules differently than those that occur in the webpack project itself. For reference, everything that https://github.com/sass-eyeglass/eyeglass supports should ideally be able to be resolved by sass-loader, for the purpose of long-term interoperability of the third-party scss module ecosystem.

[alexander-akait]

@mischkl Be great if your do PR, @jhnns now is very busy and only do review PR, i am not familiar with eyeglass and for me difficult understand what need to add

[michael-ciniawsky]

What's the status here ? :)

[alexander-akait]

@michael-ciniawsky need answer from @jhnns