webpack-contrib / sass-loader

Compiles Sass to CSS
MIT License
3.91k stars 428 forks source link

Option to use the DartVM executable rather than the pure-js version of dart-sass. #774

Closed jrood closed 6 months ago

jrood commented 4 years ago

Feature Proposal

Currently, the only options for implementation are node-sass and the pure-js version of dart-sass. There should be an option to use the fast DartVM executable version of dart-sass.

The pure JS version is slower than the stand-alone executable (https://sass-lang.com/dart-sass)

Feature Use Case

For projects that have a large codebase of sass files that need to be compiled and performance is important.

alexander-akait commented 4 years ago

Make sense, feel free to send a PR

rowanbeentje commented 4 years ago

https://github.com/sass/embedded-host-node, currently in Alpha, may be the easiest way to achieve this!

alexander-akait commented 4 years ago

Yep, but right now it is not easy

strarsis commented 3 years ago

+1!

denizsokullu commented 2 years ago

@alexander-akait https://github.com/sass/embedded-host-node is getting close to a usable beta that includes custom importers and many other features that the sass-loader relies on. What do you think?

alexander-akait commented 2 years ago

Unfortunately, I have not looked at this for a long time, if you have any ideas how we can integrate this now, you can send it, I will be glad any help

alexander-akait commented 2 years ago

It will be easy if they have docs :smile: @nex3 Can we try it to integrate and test or is it still very young?

nex3 commented 2 years ago

As soon as https://github.com/sass/embedded-host-node/pull/94 lands, the embedded host will fully support the new JS API. I'll be cutting a beta release at that point that you can test against.

rowanbeentje commented 2 years ago

Just to mention that @nex3 has now merged that PR and cut the beta release: https://github.com/sass/embedded-host-node/releases/tag/1.0.0-beta.8 . I'm excited about this!

alexander-akait commented 2 years ago

@nex3 Hi, where we can find docs about usage? Thanks

nex3 commented 2 years ago

It's all documented at https://sass-lang.com/documentation/js-api.

Edit: I realized you probably meant docs specifically about using embedded-host-node. We don't have official docs yet, but it's really just a drop-in replacement for the sass package, so you should be able to add it to your package.json and go from there.

alexander-akait commented 2 years ago

I tried to adopt https://github.com/sass/embedded-host-node locally, and it works good for me, I'll try it here and maybe make a release, perf is almost the same

alexander-akait commented 2 years ago

https://github.com/webpack-contrib/sass-loader/pull/1011

alexander-akait commented 2 years ago

@nex3 Small feedback:

  1. Function is not work:

Example (if you change sass-embedded on sass/node-sass, all works fine):

var sass = require("sass-embedded");

sass.render(
  {
    data: "#{headings(2,5)} { color: #08c; }\n",
    functions: {
      "headings($from: 0, $to: 6)": (from, to) => {
        const f = from.getValue();
        const t = to.getValue();
        const list = new sass.types.List(t - f + 1);
        let i;

        for (i = f; i <= t; i++) {
          list.setValue(i - f, new sass.types.String(`h${i}`));
        }

        return list;
      },
    },
  },
  function (err, result) {
    if (err) {
      // `err - Error: expected selector.` for `sass-embedded`
      console.log(err);

      return;
    }
    console.log(result.css.toString());
  }
);
  1. Importers broken, I tried old format (i.e. just function) and new format (i.e. { canonicalize(url) {}, load(canonicalUrl) {} } ), both is not working, code is simple:
    @import import-with-custom-logic

We still use render function to keep compatibility with node-sass/sass (dart), maybe problem with this, I don't found docs about it, but in types they are exists

  1. includePaths doesn't work, we have very simple test where we provide includePaths to sass and just use @import "import-from-included-path";
denizsokullu commented 2 years ago

https://github.com/sass/embedded-host-node/blob/main/lib/src/legacy.ts does not seem to have some of the specification included as it masks most of the options before calling the new api compile. @alexander-akait you could try to drop the legacy render method in favor of compileAsync?

alexander-akait commented 2 years ago

Yep, we can do it, just interesting why it doesn't work with legacy, because types exist

alexander-akait commented 2 years ago

hm, I was wrong, if we change render on compileStringAsync old legacy importers will not work, it is breaking change for us, because many developers still use them...

alexander-akait commented 2 years ago

I tried to switch on modern API, but docs are very bad and no examples (spent a lot of time to understand how new API works), so it requires more time than I think...

alexander-akait commented 2 years ago

Spent two days on this stuff and no luck, the new importer api is not usable and docs are very bad :disappointed: , https://sass-lang.com/documentation/js-api/interfaces/Importer,

The problem - we don't have prev in new importer API (as we have in old), so we don't know where should we resolve your import/use (you can have @import "variables"; in different files and we should know where it was requested from), we can use the url option, but it will work only for top file and nested imports is no possible to resolve

alexander-akait commented 2 years ago

@nex3 Why prev was removed from importers https://sass-lang.com/documentation/js-api/modules#LegacySyncImporter?

Without file where it was requested we can't perform resolving, we don't know context so we don't have where we should start to resolve...

nex3 commented 2 years ago

Yep, we can do it, just interesting why it doesn't work with legacy, because types exist

We implemented the new API for sass-embedded first. Note that it's currently only a beta release, so it doesn't have everything we plan to add. I'm currently working on adding support for the legacy JS API.

hm, I was wrong, if we change render on compileStringAsync old legacy importers will not work, it is breaking change for us, because many developers still use them...

Correct, old importers won't work with the new JS API and vice versa. Old importers are actually quite difficult to make efficient because they don't play nicely with the concept of "canonical URLs", which was one of the motivations of moving to a new API in the first place.

I tried to switch on modern API, but docs are very bad and no examples (spent a lot of time to understand how new API works), so it requires more time than I think...

I wrote all those docs, so if you can provide specific ways they could be more helpful or additional examples you'd like to see I'd be happy to improve them.

The problem - we don't have prev in new importer API (as we have in old), so we don't know where should we resolve your import/use (you can have @import "variables"; in different files and we should know where it was requested from), we can use the url option, but it will work only for top file and nested imports is no possible to resolve

This would have been a good thing to bring up when we asked for feedback. In this case, though, there's a good reason for the behavior. There are a few invariants we want to make sure all importers abide by, in order to allow the Sass compiler to make important optimizations (like only loading each stylesheet once) and to ensure that users can consistently reason about loads:

  1. There must be a one-to-one mapping between (absolute) canonical URLs and stylesheets. This means that even when a user loads a stylesheet using a relative URL, that stylesheet must have an absolute canonical URL associated with it and loading that canonical URL must return the same stylesheet. This means that any stylesheet can always be loaded using its canonical URL.

  2. Relative URLs are resolved like paths, so for example within scheme:a/b/c.scss the URL ../d should always be resolved to scheme:a/d.

  3. Loads relative to the current stylesheet always take precedence over loads from the load path (including virtual load paths exposed by importers), so if scheme:a/b/x.scss exists then @use "x" within scheme:a/b/c.scss will always load.

The old importer API didn't guarantee any of these invariants. An importer could use the prev parameter to add stylesheets that could only be imported as relative URLs, thus violating (1). Or it could ignore the prev parameter completely and break relative URLs, violating (2) and (3).

Rather than pushing all the complexity of handling these invariants onto importer authors like the old API, the new API is designed so that these invariants can't be violated. To quote the documentation:

Relative loads in stylesheets loaded from an importer are handled by resolving the loaded URL relative to the canonical URL of the stylesheet that contains it, and passing that URL back to the importer's canonicalize method. For example, suppose the "Resolving a Load" example above returned a stylesheet that contained @use "mixins":

  • The compiler resolves the URL mixins relative to the current stylesheet's canonical URL db:foo/bar/baz/_index.scss to get db:foo/bar/baz/mixins.
  • It calls canonicalize with "db:foo/bar/baz/mixins".
  • canonicalize returns new URL("db:foo/bar/baz/_mixins.scss").

Because of this, canonicalize must return a meaningful result when called with a URL relative to one returned by an earlier call to canonicalize.

For your use-case, it's probably easiest to use a FileImporter: this allows you to redirect a Webpack-style load to a specific file, but then automatically handles any relative loads from there without you needing to intervene at all. This is also more efficient, since it saves a round-trip from the compiler for each relative loads.

alexander-akait commented 2 years ago

@nex3

We implemented the new API for sass-embedded first. Note that it's currently only a beta release, so it doesn't have everything we plan to add. I'm currently working on adding support for the legacy JS API.

:+1:

Correct, old importers won't work with the new JS API and vice versa. Old importers are actually quite difficult to make efficient because they don't play nicely with the concept of "canonical URLs", which was one of the motivations of moving to a new API in the first place.

Yep, I noticed this when debug problems, my bad

For your use-case, it's probably easiest to use a FileImporter: this allows you to redirect a Webpack-style load to a specific file, but then automatically handles any relative loads from there without you needing to intervene at all. This is also more efficient, since it saves a round-trip from the compiler for each relative loads.

I am afraid we can't use FileImporter, because we support loading by http/https/file/data and ability to integrate any schema, there are a lot of other places where it can be problem, resolving in webpack fully based context (i.e. prev), without this we can't resolve (like Node.js built-in resolver, ts resolver and etc).

I understand why was it designed this way, but unfortunately this does not give us any opportunity to implement the importer, I want to say it is critical blocker for us.

Yep, I missed it in feedback on new API because a lot of task :disappointed: and to be honest, I didn't even notice that because this is usually the base in any resolving logic...

Unfortunately, all these mean that we cannot integrate new API, to be honestly no one bundler can do it... because the logic in such a place is almost the same for webpack/rollup/vite/etc

alexander-akait commented 2 years ago

And FileImporter also doesn't have context (i.e. prev), so resolving is impossible in this case too...

nex3 commented 2 years ago

Can you give me an example of how webpack's resolution logic uses the file context?

alexander-akait commented 2 years ago

@nex3 Yep, we use it for resolving - https://github.com/webpack/enhanced-resolve, it is supporting all new JS features like exports/imports, type, also plugable and have many built-in plugins, under the hood we use this package here:

simplified logic:

const context = path.dirname(prev);

resolve(context, url, (err, result) => {
  result;
});

Just for information - we have more than one resolver for sass, list of them - https://github.com/webpack-contrib/sass-loader/blob/master/src/utils.js#L451

nex3 commented 2 years ago

What information is needed from the context object that's not available globally? In other words, from a user's perspective, how might @import "~foo" be resolved differently depending on which file it comes from?

alexander-akait commented 2 years ago

@nex3

@import "~foo"

Firstly, aliases - they provided by users, so it can be path on any place, anyway after resolving we have resolved URL, so we can return to sass real path with file:///, there are some note - developer can create plugin which resolve different URLs based on any condition, but it kill webpack cache too

Second, versions - if developer have @import "~foo" inside package inside node_modules, it will be resolved relative to package, because in other package can be @import "~foo" with other version of foo

Thirdly, path can be different based on package manager, yarn/pnpm uses versions in path, i.e. node_modules/.store/package@v1.3.0/style.css (simplified), also yarn supports resoltuions, so requests can be rewritten on package manager level.

Anyway, after resolving we always have path to real file and can convert it to file:///, in most of cases it's deterministic, yep, there are some exceptions, but most often this is problem with configuration.

nex3 commented 2 years ago

What if we included the canonical URL of the previous file in the options parameter to canonicalize(), but only for non-absolute URLs? That way you could make ~-style loads context-dependent without breaking guarantees for relative loads.

alexander-akait commented 2 years ago

@nex3

What if we included the canonical URL of the previous file in the options parameter to canonicalize(), but only for non-absolute URLs? That way you could make ~-style loads context-dependent without breaking guarantees for relative loads.

Is previous file is file where we have @import/@use?, i.e. I have **style.scss and @use 'variables';, so in importer I need url - variables and /absolute/path/to/style.scss - prev/origin (it can be with file:///)

alexander-akait commented 2 years ago

@nex3 Found small problem in modern API, when I have @import "file"; and in file we have broken syntax (or any other errors, for example: a {{}), sass throws an error, in modern API we don't have the file property on Error, in old API we have it, will be great to fix it too, otherwise watching will be broken if developer accidentally do mistake in code

denizsokullu commented 2 years ago

@alexander-akait There is a file attached to the Error but it's not necessarily always what causes the error.(https://github.com/sass/embedded-host-node/blob/main/lib/src/legacy.ts#L227) Adding that as a dependency does not actually trigger a compilation in the example below:

// example_mixin.scss
mixin example_mixin($arg1, $arg2) {
...
}
// main.scss
@import "example_mixin";
@include example_mixin($arg1: 1, $arg2: 2);

Let's say you edit example_mixin.scss and change the argument name $arg1 to $arg3. The compiler is going to generate an error with the message:

SassError: No argument named $arg1.
┌──> main.scss

The file attached in the error will be main.scss so even if you change back example_mixin to what it was, since it was not added as a dependency after the error was returned, the watcher won't recompile the sass. And interestingly, this is not a embedded-host-node specific bug. As long as we are not using the webpackImporter: true in sass-loader, this issue can be observed. I'm guessing the example_mixin.scss is added as a dependency as webpack handles the importing resolutions.

nex3 commented 2 years ago

Is previous file is file where we have @import/@use?, i.e. I have **style.scss and @use 'variables';, so in importer I need url - variables and /absolute/path/to/style.scss - prev/origin (it can be with file:///)

That's right. It would be the canonical URL of the file in which the load appeared, so in this case file:///absolute/path/to/style.scss.

Found small problem in modern API, when I have @import "file"; and in file we have broken syntax (or any other errors, for example: a {{}), sass throws an error, in modern API we don't have the file property on Error, in old API we have it, will be great to fix it too, otherwise watching will be broken if developer accidentally do mistake in code

The location of the error can be retrieved from the span field. Specifically, error.span.url contains the canonical URL of the file in which the error occurred.

As far as watcher support is concerned, https://github.com/sass/sass/issues/2950 is tracking a way to retrieve all files that were accessed when compilation failed.

alexander-akait commented 2 years ago

@nex3

That's right. It would be the canonical URL of the file in which the load appeared, so in this case file:///absolute/path/to/style.scss.

Sounds good to me :+1:

As far as watcher support is concerned, sass/sass#2950 is tracking a way to retrieve all files that were accessed when compilation failed.

It will be good improvement :+1:

@denizsokullu

The file attached in the error will be main.scss so even if you change back example_mixin to what it was, since it was not added as a dependency after the error was returned, the watcher won't recompile the sass. And interestingly, this is not a embedded-host-node specific bug. As long as we are not using the webpackImporter: true in sass-loader, this issue can be observed. I'm guessing the example_mixin.scss is added as a dependency as webpack handles the importing resolutions.

Yes, it is truth, because we resolve and add dependency before sass compilation, anyway without file we will have problems in some cases, when resolver failed too, there are edge cases...

nex3 commented 2 years ago

I've filed https://github.com/sass/sass/issues/3247 to track this feature on the Sass end. There are still some design questions to iron out, so I'd appreciate any feedback or suggestions you have on that issue.

denizsokullu commented 2 years ago

Hi @alexander-akait, https://github.com/sass/embedded-host-node/releases/tag/1.0.0-rc.1 is out now with the legacy importer support

alexander-akait commented 2 years ago

@denizsokullu I will finish https://github.com/webpack-contrib/sass-loader/pull/1012 on this weekend, in theory we can support new API too, just without built-in importer and fix it late

alexander-akait commented 2 years ago

Almost all is fine, but some bugs still exists https://github.com/webpack-contrib/sass-loader/pull/1012#issuecomment-1025156034, in theory I can just skip broken tests and do release, but I am afraid source maps is very good for dev env, and we should fix it

alexander-akait commented 2 years ago

I want to do release today, there are some bugs with source maps and @use (edge case) in sass-embedded (I will ping nex3 here and report about them here, we can't fix them in sass-loader), also I implement api: "modern" | "legacy", but it is experimental and currently modern API doesn't support built-in webpack resolver, i.e. all styles from node_modules will be not loaded, but you can use loadPaths if you have simple project, if you rely on the sass field in package.json/aliases or somethings more complex it will not work with modern API, unfortunately to solve it we need to improve some things on sass side.

Benchmarks (locally, bootstrap 5 latest):

node-sass#render x 2.70 ops/sec ±1.36% (18 runs sampled)
dart-sass#render x 0.41 ops/sec ±12.03% (7 runs sampled)
sass-embedded#render x 3.95 ops/sec ±0.74% (24 runs sampled)
Fastest is sass-embedded#render

I think words are unnecessary here

alexander-akait commented 2 years ago

Released: https://github.com/webpack-contrib/sass-loader/releases/tag/v12.5.0

@nex3 We finished integration, but there are some bugs in sass-embedded side (for render API, i.e. legacy):

  1. Source map generation is broken with custom importer, reproducible example:
    
    const fs = require("fs");
    const dartSass = require("sass");
    const sassEmbedded = require("sass-embedded");

(async () => { await new Promise((resolve) => { // Works fine dartSass.render( { file: "node_modules/bootstrap-v5/scss/bootstrap.scss", data: fs .readFileSync("node_modules/bootstrap-v5/scss/bootstrap.scss") .toString(), sourceMap: true, outFile: "out.css", importer: function () { return null; }, }, (error, result) => { if (error) { throw error; }

    console.log(JSON.parse(result.map).sources);

    resolve();
  }
);

});

await new Promise((resolve) => { // Broken sassEmbedded.render( { file: "node_modules/bootstrap-v5/scss/bootstrap.scss", data: fs .readFileSync("node_modules/bootstrap-v5/scss/bootstrap.scss") .toString(), sourceMap: true, outFile: "out.css", // Comment this and source map generation is fine importer: function () { return null; }, }, (error, result) => { if (error) { throw error; }

    console.log(JSON.parse(result.map).sources);

    resolve();
  }
);

}); })();


Output:

- `sass` (i.e. `dart-sass`)

[ 'file:///home/evilebottnawi/IdeaProjects/sass-loader/node_modules/bootstrap-v5/scss/bootstrap.scss', 'file:///home/evilebottnawi/IdeaProjects/sass-loader/node_modules/bootstrap-v5/scss/_root.scss', ... ]


- `sass-embedded`

[ 'node_modules/bootstrap-v5/scss/bootstrap.scss', 'stdin', 'stdin', // Only stdin ... ]


If you comment the `importer` option, source map generation is working fine

2. `sass-embedded` doesn't support `@import`/`@use` with `file:///path/to/file.scss` and `/path/to/file.scss` on windows (on linux and macos it works fine).

Example:

@import "file:///path/to/file.sass"


But it works fine with `sass` (`dart-sass`).

3. Doesn't work with `@use "~scss/underscore" as underscore3;` and custom importer

Output:
Error: Module build failed (from ../src/cjs.js):
SassError: TypeError: Cannot read property 'url' of undefined
  ╷
9 │ @use "~sass/underscore" as underscore3
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  test/sass/uses.sass 9:1  root stylesheet

Full error report:

Error: Error: TypeError: Cannot read property 'url' of undefined ╷ 9 │ @use "~sass/underscore" as underscore3 │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ╵ test/sass/uses.sass 9:1 root stylesheet at handleCompileResponse (/home/evilebottnawi/IdeaProjects/sass-loader/node_modules/sass-embedded/lib/src/compile.ts:304:11) at compileRequestAsync (/home/evilebottnawi/IdeaProjects/sass-loader/node_modules/sass-embedded/lib/src/compile.ts:160:12) at processTicksAndRejections (internal/process/task_queues.js:97:5) { status: 1, formatted: "Error: TypeError: Cannot read property 'url' of undefined\n" + ' ╷\n' + '9 │ @use "~sass/underscore" as underscore3\n' + ' │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n' + ' ╵\n' + ' test/sass/uses.sass 9:1 root stylesheet', toString: [Function: toString], line: 9, column: 1, file: '/home/evilebottnawi/IdeaProjects/sass-loader/test/sass/uses.sass' }


As you can see it happens in `sass-embedded`, I go to this line and found in dist:

if (options === null || options === void 0 ? void 0 : options.url) input.setUrl(options.url.toString());



If I comment it no problems, but I found the `url` option only for modern API, but looks like used for legacy API too.

If you need more information or context of the problem, feel free to feedback
nex3 commented 2 years ago
  1. I've got a fix for this at https://github.com/sass/embedded-host-node/pull/113.

  2. I'm not sure what the URL file:///path/to/file.sass would mean on Windows, since it lacks either a drive letter or a server name. I'm not sure what Dart Sass does with it, but Node's url.fileURLToPath() function considers it invalid on Windows.

  3. Can you give me a stand-alone reproduction of this?

alexander-akait commented 2 years ago

I'm not sure what the URL file:///path/to/file.sass would mean on Windows, since it lacks either a drive letter or a server name. I'm not sure what Dart Sass does with it, but Node's url.fileURLToPath() function considers it invalid on Windows.

I agree, that is interesting, because it works using dart-sass, but it is edge case. so I think we can mark it as expected behavior

Can you give me a stand-alone reproduction of this?

There are two examples:

 const sassEmbedded = require("sass-embedded");

(async () => {
  try {
    await new Promise((resolve, reject) => {
      sassEmbedded.render(
        {
          // file: "node_modules/bootstrap-v5/scss/bootstrap.scss",
          data: `@use "util";
@use "@org/style";`,
          file: "/test/test.scss",
          // Due empty array
          importer: [

          ],
        },
        (error, result) => {
          if (error) {
            reject(error);

            return;
          }

          resolve();
        }
      );
    });
  } catch (error) {
    console.log(error);
  }
})();

Output:

Error: Error: TypeError: Cannot read property 'call' of undefined

Original problem:

const sassEmbedded = require("sass-embedded");

(async () => {
  try {
    await new Promise((resolve, reject) => {
      sassEmbedded.render(
        {
          data:
            '@use "util";\n' +
            '/* @use "~module"; */\n' +
            '//@use "~module" as module2;\n' +
            '/* @use "~another"; */\n' +
            '//@use "~another" as another;\n' +
            '/* @use "~@org/style"; */\n' +
            "// Should prefer `scss`\n" +
            '@use "@org/style";\n',
          file: "/home/evilebottnawi/IdeaProjects/sass-loader/test/scss/uses.scss",
          importer: [
            function (prev, url, done) {
              console.log("HERE");

              done({ file: url });
            },
          ],
        },
        (error, result) => {
          if (error) {
            reject(error);

            return;
          }

          console.log(result);

          resolve();
        }
      );
    });
  } catch (error) {
    console.log(error);
  }
})();

You can see that we don't even call importer. If I remove file all works fine, looks like sass-embedded wait URL here, but in legacy API it is path.

alexander-akait commented 2 years ago

@nex3 Can you look at the latest my report, because we have reports from other developers too - https://github.com/webpack-contrib/sass-loader/issues/1028, more repro - https://github.com/francescocaveglia/sass-loader-bug

nex3 commented 2 years ago

Working on a fix. In the meantime, you should be able to work around it by passing importers: undefined instead of importers: [].

nex3 commented 2 years ago

https://github.com/sass/embedded-host-node/pull/114

alexander-akait commented 2 years ago

@nex3 Can you look at second example https://github.com/webpack-contrib/sass-loader/issues/774#issuecomment-1041493056 (the same problem - https://github.com/francescocaveglia/sass-loader-bug), it is more important, because developers can't compile bootstrap 5, thanks

alexander-akait commented 2 years ago

Confirmed - source map generation fixed https://github.com/webpack-contrib/sass-loader/pull/1030

Hope @nex3 will find time in near future to fix problems with custom importer - https://github.com/webpack-contrib/sass-loader/issues/774#issuecomment-1046014498 and second example in https://github.com/webpack-contrib/sass-loader/issues/774#issuecomment-1041493056

After this I want to say it it safe to migrate on sass-embedded and get perf improvement

nex3 commented 2 years ago

I have a PR out: https://github.com/sass/embedded-host-node/pull/114

alexander-akait commented 2 years ago

@nex3 I'm afraid I've confused you a bit, could you run, that is another problem:

Can you run locally:

const sassEmbedded = require("sass-embedded");

(async () => {
  try {
    await new Promise((resolve, reject) => {
      sassEmbedded.render(
          {
            data:
                '@use "util";\n' +
                '/* @use "~module"; */\n' +
                '//@use "~module" as module2;\n' +
                '/* @use "~another"; */\n' +
                '//@use "~another" as another;\n' +
                '/* @use "~@org/style"; */\n' +
                "// Should prefer `scss`\n" +
                '@use "@org/style";\n',
            file: "/home/evilebottnawi/IdeaProjects/sass-loader/test/scss/uses.scss",
            importer: [
              function (prev, url, done) {
                console.log("HERE");

                done({ file: url });
              },
            ],
          },
          (error, result) => {
            if (error) {
              reject(error);

              return;
            }

            console.log(result);

            resolve();
          }
      );
    });
  } catch (error) {
    console.log(error);
  }
})();

Output:

Error: Error: TypeError: Cannot read property 'url' of undefined
  ╷
8 │ @use "@org/style";
  │ ^^^^^^^^^^^^^^^^^
  ╵
  test/scss/uses.scss 8:1  root stylesheet
    at handleCompileResponse (/path/to/sass-loader/node_modules/sass-embedded/dist/lib/src/compile.js:213:15)
    at compileRequestAsync (/path/to/sass-loader/node_modules/sass-embedded/dist/lib/src/compile.js:101:16)
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  status: 1,
  formatted: "Error: TypeError: Cannot read property 'url' of undefined\n" +
    '\u001b[34m  ╷\u001b[0m\n' +
    '\u001b[34m8 │\u001b[0m \u001b[31m@use "@org/style"\u001b[0m;\n' +
    '\u001b[34m  │\u001b[0m \u001b[31m^^^^^^^^^^^^^^^^^\u001b[0m\n' +
    '\u001b[34m  ╵\u001b[0m\n' +
    '  test/scss/uses.scss 8:1  root stylesheet',
  toString: [Function: toString],
  line: 8,
  column: 1,
  file: '/path/to/sass-loader/test/scss/uses.scss'
}

As you can see we have another problem here:

Error: Error: TypeError: Cannot read property 'url' of undefined

If you try to compile webpack 5 using sass-loader you will faced with it. It can be critical for developers.

Even more it doesn't run importer.

Thanks for help!

nex3 commented 2 years ago

@alexander-akait with https://github.com/sass/embedded-host-node/pull/114 patched in I see:

HERE
Error: Error: Can't find stylesheet to import.
  ╷
1 │ @use "util";
  │ ^^^^^^^^^^^
  ╵
  /home/evilebottnawi/IdeaProjects/sass-loader/test/scss/uses.scss 1:1  root stylesheet
    at handleCompileResponse (/home/nweiz/goog/sass/host/dist/lib/src/compile.js:213:15)
    at compileRequestAsync (/home/nweiz/goog/sass/host/dist/lib/src/compile.js:101:16)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  status: 1,
  formatted: "Error: Can't find stylesheet to import.\n" +
    '\x1B[34m  ╷\x1B[0m\n' +
    '\x1B[34m1 │\x1B[0m \x1B[31m@use "util"\x1B[0m;\n' +
    '\x1B[34m  │\x1B[0m \x1B[31m^^^^^^^^^^^\x1B[0m\n' +
    '\x1B[34m  ╵\x1B[0m\n' +
    '  /home/evilebottnawi/IdeaProjects/sass-loader/test/scss/uses.scss 1:1  root stylesheet',
  toString: [Function: toString],
  line: 1,
  column: 1,
  file: '/home/evilebottnawi/IdeaProjects/sass-loader/test/scss/uses.scss'
}
alexander-akait commented 2 years ago

@nex3 thanks, I will test it tomorrow, because it is strange, it means something different in resolving (maybe bug?), I will check and provide report