Importers not called for all imports

[mrdziuban]

I believe this is the same issue that's been discussed in the recent comments of #9. When importing a file by its relative path, custom importers are only called if the file is not found. I've put together a simple way to reproduce this with 3 bash commands:

touch _exists.scss

# Import one file that exists and one that doesn't
echo -e "@import 'exists';\n@import 'missing';" > index.scss

node -e "\
  require('sass').renderSync({ \
    file: 'index.scss', \
    importer: u => { \
      console.log('Importer received:', u); \
      return { contents: '' }; \
    } \
  });"

The only thing logged is

Importer received: missing

when I would expect to also see the importer receive exists.

[nex3]

You're relying on a bug (or arguably a misfeature) in LibSass. This behavior wasn't replicated in Dart Sass because LibSass will eventually move away from it as well.

This is disallowed because it violates the principle of locality, which says that it should be possible to reason about a stylesheet without knowing everything about how the entire system is set up. If a user tries to import a stylesheet relative to another stylesheet, that import should always work. It shouldn't be possible for some configuration somewhere else to break it.

This is very relevant, for example, when dealing with importers that load files from load paths. If a user writes @import "variables" and we allowed absolute imports to shadow local ones, a library they use might add a _variables.scss file down the line and break them. Neither the user nor the library would have any way of knowing that was coming, nor any way of working around it other than manually namespacing everything, which sucks.

[mnahkies]

@nex3 it would be great if the dart-sass documentation could detail this. Based on experimentation I've done today, the custom importer will only receive files that dart-sass wasn't able to find on it's own - eg: I had to remove an includePath to correctly intercept the loading of a file.

There's nothing in the node-sass documentation which is linked from the dart-sass readme to indicate which files would be passed to it, implying that all files should passed.

(on a similar note I'd love more documentation about how the importer works in general, because I currently feel like I'm reverse engineering it, in particular the impact of returning {content: "..."} as this doesn't seem to be able to define variables)

EDIT: the part about content not defining variables, as actually because the correct return is {contents: "..."} and my object that had neither contents or filename was being silently skipped...

[nex3]

Based on experimentation I've done today, the custom importer will only receive files that dart-sass wasn't able to find on it's own - eg: I had to remove an includePath to correctly intercept the loading of a file.

This is a bug; Dart Sass should match Node Sass in this behavior. I've filed #681 to track this.

There's nothing in the node-sass documentation which is linked from the dart-sass readme to indicate which files would be passed to it, implying that all files should passed.

I agree that that documentation is not great. I recommend using the documentation on the Sass site instead. #680 updates the Dart Sass README to link to the Sass site documentation instead, and sass/sass-site#342 more clearly documents the order that imports are (supposed to be) resolved in.

[u-v]

Hi Natalie. I'd like to re-open this thread.

You're relying on a bug (or arguably a misfeature) in LibSass. This behavior wasn't replicated in Dart Sass because LibSass will eventually move away from it as well.

This is disallowed because it violates the principle of locality, which says that it should be possible to reason about a stylesheet without knowing everything about how the entire system is set up. If a user tries to import a stylesheet relative to another stylesheet, that import should always work. It shouldn't be possible for some configuration somewhere else to break it.

This is very relevant, for example, when dealing with importers that load files from load paths. If a user writes @import "variables" and we allowed absolute imports to shadow local ones, a library they use might add a _variables.scss file down the line and break them. Neither the user nor the library would have any way of knowing that was coming, nor any way of working around it other than manually namespacing everything, which sucks.

I do agree that it violates the principle of locality. But those who write custom importers know what they're doing (like me :) ) And I understand all pros and cons of violating some architecture principles. In fact, the main purpose of something "custom" is to be able to override things which work not the way you need.

I can bring you an example and the reason why we need to resolve imports with custom importer first:
We're working with SFCC platform which back-end architecture completely breaks the locality principle by design. We have there so-called "cartridges" which are kind of "layers" with identical folders structure. By defining the "cartridges path" we configure z-index for each cartridge. And resources from the higher cartridge will be used instead of the lower one. Thus we have to follow the platform design. And it's just one example (I'm sure it's not the craziest one).

Overall, my message that it'd be great to change the priority of import resolvers and make custom importer the very first in the queue.

Thank you for your work!

[nex3]

In the upcoming JS API refactor, we're planning to make importers work more like they do in Dart Sass (and like they originally did in Ruby Sass). This means that a relative import from a file loaded by an importer will be given to the same importer at first, which should satisfy your use case without requiring that importers universally take precedence over filesystem-relative imports.

[danielgindi]

I stumbled on this too, today.
And I have to say, the current implementation of importers is not great.
I think that an override should always be possible. Think about how rollup implements plugins.
Locality or not- that should also be overridable. Nothing should break when everyone behaves correctly, and functionality should not be reduced just because some user somewhere could act inappropriately and override everyone’s files.