Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS API: Support custom importers #9

Closed
nex3 opened this issue Nov 1, 2016 · 22 comments
Closed

JS API: Support custom importers #9

nex3 opened this issue Nov 1, 2016 · 22 comments
Labels
enhancement JavaScript Issues particular to the Node.js distribution
Milestone

Comments

@nex3
Copy link
Contributor

nex3 commented Nov 1, 2016

We need to support the node-sass API's importer option, which specifies custom importers that locate and load source files. Note that this may require the use of something like synchronize.js, since Dart Sass is entirely synchronous.

@nex3 nex3 added enhancement JavaScript Issues particular to the Node.js distribution labels Nov 1, 2016
@nex3 nex3 added this to the 1.0.0 milestone Nov 1, 2016
@nex3 nex3 mentioned this issue Nov 1, 2016
6 tasks
@zoechi
Copy link

zoechi commented Nov 1, 2016

Is this also necessary for using it in Dart projects without packages directory for scss files imported using packages paths, or are there other ways?

@nex3
Copy link
Contributor Author

nex3 commented Nov 1, 2016

package: URI support is planned. I just filed #19 to track it.

@graingert
Copy link

@nex3
Copy link
Contributor Author

nex3 commented Oct 6, 2017

Some notes on the (largely undocumented) behavior of node-sass's importer API:

  • The done() parameter is only provided to the importer function when using render(), nor renderSync().
  • For renderSync(), returning null or undefined indicates that the file wasn't found.
    • For render(), null indicates that the file wasn't found, while undefined indicates that Sass should wait until done() is called.
  • node-sass doesn't seem to handle throws from importers gracefully. In render(), it top-levels the errors; in renderSync(), it segfaults.
    • It can, however, return an Error object that will be handled gracefully.
    • It also allows Error subclasses, although it doesn't seem to preserve the original class information.
  • prev seems to be passed regardless of whether the same importer was used to import the current file.
    • For imports in the root stylesheet, if that stylesheet was passed using data:, prev is always "stdin".
    • For imports from the filesystem, prev is an absolute path (not a URL).
  • this refers to an object with an options field based on the options passed to render() or renderSync(). However:
    • Some default values are filled in:
      • includePaths, precision, indentWidth, indentType, and linefeed
      • includePaths defaults to the current working directory. It's a single colon-separated string rather than a list.
      • indentType is an enum. 0 indicates spaces, 1 indicates tabs.
      • linefeed is the actual line feed character, not the symbolic name.
      • It has a style field that's an enum of output styles. The number 1 corresponds to the expanded style.
    • It has two fields that aren't options:
      • this.options.context is a circular reference to the same object as this
      • this.options.results is a partially-constructed result object that looks like {stats: {entry: ..., start: ...}}.
  • includedFiles contains absolute paths for files imported via the filesystem, and the literal contents of the @import directives for files imported via importers.
    • It also contains absolute paths for files imported via importers that return {file: ...}.
  • If the importer returns an object without file or contents keys, it's treated as successfully importing an empty file.
    • If it returns any other value, it's treated the same as null.
  • Filesystem imports are path-based, not URL-based. For example, paths may be backslash-separated on Windows, which will pose some challenges at parse-time.
  • An importer takes precedence over a relative path. See Custom importers shouldn't shadow relative imports libsass#2483.
    • When a new import is found, every importer is tried in order, instead of Ruby Sass's behavior of trying a relative import in the current importer first.

The final bullet point means we probably can't model node-sass importers the same way we do the Dart API importers introduced in #179. We'll likely need to give the internal compile*() functions a special nodeImporters parameter that takes precedence over all other importers, including relative imports.

nex3 added a commit that referenced this issue Nov 3, 2017
nex3 added a commit that referenced this issue Nov 3, 2017
nex3 added a commit that referenced this issue Nov 17, 2017
This allows us to support asynchronous importers and, eventually,
functions without breaking synchronous support. The copies were made
manually, but the eventual plan is to auto-generate the synchronous
versions by stripping all asynchrony from the async versions.

See #9
nex3 added a commit that referenced this issue Nov 18, 2017
This allows us to support asynchronous importers and, eventually,
functions without breaking synchronous support. The copies were made
manually, but the eventual plan is to auto-generate the synchronous
versions by stripping all asynchrony from the async versions.

See #9
nex3 added a commit that referenced this issue Nov 18, 2017
This allows us to support asynchronous importers and, eventually,
functions without breaking synchronous support. The copies were made
manually, but the eventual plan is to auto-generate the synchronous
versions by stripping all asynchrony from the async versions.

See #9
nex3 added a commit that referenced this issue Nov 18, 2017
nex3 added a commit that referenced this issue Nov 18, 2017
nex3 added a commit that referenced this issue Nov 20, 2017
nex3 added a commit that referenced this issue Dec 1, 2017
This allows us to support asynchronous importers and, eventually,
functions without breaking synchronous support. The copies were made
manually, but the eventual plan is to auto-generate the synchronous
versions by stripping all asynchrony from the async versions.

See #9
nex3 added a commit that referenced this issue Dec 1, 2017
@nex3 nex3 closed this as completed in 8272724 Dec 2, 2017
@ryanelian
Copy link

ryanelian commented May 25, 2018

In node-sass, when importer function callback is invoked with .css file as file parameter, it converts the import request into URL:

@import "bootstrap";

Becomes:

@import url("E:\myproject\node_modules\bootstrap\dist\css\bootstrap.css");

When using my custom importer / resolver:

importer: (request, source, done) => {
    // request: bootstrap
    // source: E:\myproject\client\css\index.scss
    this.sassImport(source, request).then(result => {
        // result: E:\myproject\node_modules\bootstrap\dist\css\bootstrap.css
        done({
            file: result
        });
    }).catch(error => {
        done(error);
    });
}

When using dart-sass Node API, the import request is not being converted...

Sass CompileError: Can't find stylesheet to import.
@import "bootstrap";
        ^^^^^^^^^^^
  client\css\index.scss 1:9  root stylesheet

Is this by-design?

Also, is it possible to use importer with the sync API?

@nex3
Copy link
Contributor Author

nex3 commented May 25, 2018

@ryanelian I wasn't aware of that aspect of Node Sass's importer support, but it's not something I think we should support. An importer shouldn't be able to transform a dynamic import into a static import. So I guess the answer is yes, the current behavior is by design.

Also, is it possible to use importer with the sync API?

Yes, as long as you return the result rather than passing it to the done() callback.

@ryanelian
Copy link

ryanelian commented May 25, 2018

An importer shouldn't be able to transform a dynamic import into a static import. So I guess the answer is yes, the current behavior is by design.

I don't agree with this stance.

I mean, what's the point of enabling consumers to use *CUSTOM* importer if they can't fully customize the result of the import?

I'd be happy if you can just pass-through the result... 😕

Yes, as long as you return the result rather than passing it to the done() callback.

Ah. That should be documented somewhere. Good to know, thanks.

@nex3
Copy link
Contributor Author

nex3 commented May 25, 2018

An import that loads a Sass file is a fundamentally different construct than an import that compiles to CSS. They operate at different times and do different things. They're so different that we're planning to change the name of "load a Sass file" to @use to make it clear that they're different things. Allowing one to transform into the other, sometimes, depending on logic hidden in an importer would be confusing and difficult to support.

@ryanelian
Copy link

ryanelian commented May 25, 2018

They're so different that we're planning to change the name of "load a Sass file" to @use to make it clear that they're different things.

Interesting. I'm trying to understand this statement. Please correct me if I'm wrong.

So in future Sass,

  • The @import as we know it will become @use

  • and the new @import will cause Sass to insert the content of the target source file into the line where @import will be called (like C #include)?

@nex3
Copy link
Contributor Author

nex3 commented May 28, 2018

That's not exactly accurate. @use will be a new at-rule that loads Sass files with new semantics, designed to improve on @import in many ways. The existing @import semantics will continue to work for a long time, but the plan is to eventually deprecate and even more eventually remove them. Once they are removed, @import will be handled like any other CSS at-rule: it will be passed through directly to CSS, and not processed by Sass in any way. So when you write @import "something" in your Sass file, @import "something" will end up in your CSS as well.

@ryanelian
Copy link

The existing @import semantics will continue to work for a long time, but the plan is to eventually deprecate and even more eventually remove them.

Should probably just remove them when @use is available IMHO. Less headache for both you and me...

(Make Sass ignore @import so I can use postcss-import to do my custom importer.)

@nex3
Copy link
Contributor Author

nex3 commented May 28, 2018

We can't just break backwards-compatibility for such a fundamental part of the language.

@ryanelian
Copy link

ryanelian commented May 28, 2018

Then how are you planning to support import semantics such as:?

@import "bootstrap";

It is a common practice (I did not invent this) to do that, then having whatever bundler you use (PostCSS, webpack, etc.) to read style field in node_modules/bootstrap/package.json (which will result in importing node_modules/bootstrap/dist/css/bootstrap.css)

  • I cannot rely on Sass custom importer because you won't let converting dynamic import to static import.

  • Nor I can rely on postcss-import (after Sass compilation) because Sass will scream compile error due to processing @import because you still want to keep the old @import...

Once again, I'd be happy if you can just pass through the:

@import url("node_modules/bootstrap/dist/css/bootstrap.css")

@nex3
Copy link
Contributor Author

nex3 commented May 28, 2018

If you want to ensure that an @import compiles to a CSS import today, there are four things Sass looks for to decide to treat an import as plain CSS:

  • The file extension .css (although this is semi-broken in LibSass).
  • The filename beginning with http://.
  • The filename being wrapped in url().
  • Any media or supports queries after the import.

You can use any of these to pass the import through for postprocessing.

@ryanelian
Copy link

  • The file extension .css (although this is semi-broken in LibSass).

This won't be practical for above example, because you'd make people write: @import "bootstrap.css" which is wrong (the npm package name is bootstrap)

  • The filename being wrapped in url().

While this can be practical @import url("bootstrap"), this introduces weird quirks to the source code; imagine me telling this to the team:

"hey guys, unlike other standard CSS bundlers import syntax, in Sass you need to wrap library imports using url because reasons."

That's not really a convincing pitch...

@nex3
Copy link
Contributor Author

nex3 commented May 28, 2018

Yes, that's why we're working on moving away from using the name @import.

@RobertWHurst
Copy link

RobertWHurst commented Jan 6, 2019

The documentation should be updated to note that importer is not properly supported for render. It does not behave the way node-sass does, and more importantly, it cannot be used for path rewrite as expected by build systems and bundlers.

I'm still not clear on why dart sass is not calling importers for @import 'bootstrap' or @import './relative/file'.

When are importers actually called?

@nex3
Copy link
Contributor Author

nex3 commented Jan 9, 2019

The documentation should be updated to note that importer is not properly supported for render. It does not behave the way node-sass does, and more importantly, it cannot be used for path rewrite as expected by build systems and bundlers.

If Dart Sass's JS API behaves differently than Node Sass's, that's a bug. Please file an issue for it, along with a minimal JS reproduction and an explanation of how the behavior differs.

I'm still not clear on why dart sass is not calling importers for @import 'bootstrap' or @import './relative/file'.

Dart Sass should call importers for both of those importers.

When are importers actually called?

When the interpreter reaches an @import statement.

@RobertWHurst
Copy link

@nex3 Ok, makes sense. I'll file a ticket with an example.

@ivandosreisandrade
Copy link

Greetings,
I know this issue is quite old and is already closed.
Is there a way of using the --importer flag in dart-sass similar to how it was used in node-sass?

Here's a reference from the official API:
https://www.npmjs.com/package/node-sass-magic-importer#cli

@nex3
Copy link
Contributor Author

nex3 commented Nov 7, 2022

The command-line interface is shared between both the Node.js distribution and the standalone distribution. Since we can't support JS importers in the standalone distribution, we can't support them through the CLI in general. Generally speaking, if you want to use code to customize your Sass compilation, that's specifically what the JS API is for.

@ivandosreisandrade
Copy link

Thank you very much for the quick response. I will try that approach.

nex3 added a commit that referenced this issue May 10, 2023
This doesn't yet support first-class function values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement JavaScript Issues particular to the Node.js distribution
Projects
None yet
Development

No branches or pull requests

6 participants