@import's processed breadth-first instead of depth-first

[pkmiec]

I'm looking to speed up my asset compilation by switching from http://github.com/rails/sass-rails to http://github.com/bolandrm/sassc-rails. I have created http://github.com/appfolio/sassc-import_once to provide "import once" functionality that's normally provided by http://github.com/Compass/compass/tree/master/import-once for Sass.

The problem is that @import seems be processed by sassc / libsass breadth-first instead of depth-first. Here is how why that matters,

/* component.scss */
@import "component/button";
@import "component/color";

/* component/button.scss */
@import "component/color";
$button-default-text-color: $text-color;

/* component/color.scss */
$text-color: #303030;

Using sassc (with my sassc-import_once) produces,

SassC::SyntaxError: Error: Undefined variable: "$text-color".
        on line 3 of test/fixtures/component/_button.scss
>> $button-default-text-color: $text-color;

The reason is that files are imported in the following order,

imports   component/button from component.scss
imports   component/color  from component.scss
imports   component/color  from component/_button.scss

Notice the component/color from component.scss is imported first ... this causes the "import once" from re-importing component/color next it is seen.

The depth-first approach would import files in the following order,

imports   component/button from component.scss
imports   component/color  from component/_button.scss
imports   component/color  from component.scss

I have a failing test that replicates this case,

1. clone sassc-import_once
2. bundle
3. rake

edit: fixed typos

[pkmiec]

This is related to #872

[xzyfer]

I've done some experimenting here and as far as I can tell Libsass acts the same as Ruby Sass. I've compared SassC, Ruby Sass, and node-sass async render (with and without a custom importer). The behaviour is identical.

Please confirm this is not a logic bug in your plugin, otherwise provide a test using SassC directly that show this error.

[saper]

I think this import-once approach may have some trouble due to #1160. Would love to see a self-contained test.

[pkmiec]

Thanks. I'll work on getting a self-contained example today.

[xzyfer]

Ok I've been able to confirm that although the imports are being resolved depth-first (i.e. the output is correct), the custom importers are being execute breadth-first. This looks like an issue with custom importers.

[saper]

Is this different from ruby sass?

Since it seems normal to me - when the piece of code is encountered, it is executed; if the file is about to be included we need to call the importer, parse and execute contents.

I guess that the "import once" functionality simply overshadows the old definition of "component/color" and then you get the error. Does it work without the "import once" functionality?

[xzyfer]

This issue is independent of any import-once functionality. To be clear Libsass is resolving the imports in the expected order, so all code works as expected.

However the custom importers aren't not fired as expected. I have a proof of concept I'm iterating on. First I need to determine if the issue is on the node-sass side or the Libsass C API side.

[pkmiec]

That's been my experience as well; the resulting .css is correct (thus it is difficult to make a spec in sass-spec). However, the following patch to libsass shows the problem,

diff --git a/parser.cpp b/parser.cpp
index 2c6bc72..ec41473 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -236,6 +236,7 @@ namespace Sass {
   {
     bool has_import = false;
     string load_path = unquote(import_path);
+    cout << "-- " << import_path << "\n";
     for (auto importer : importers) {
       // int priority = sass_importer_get_priority(importer);
       Sass_Importer_Fn fn = sass_importer_get_function(importer);

/* example.scss */
@import "_one.scss";
@import "_three.scss";

/* _one.scss */
@import "_two.scss";
.one { color: #100; }

/* _two.scss */
.one { color: #200; }

/* _three.scss */
.one { color: #300; }

compiling sassc with the patched libsass,

sassc %> bin/sassc /tmp/example.scss /tmp/out.scss
-- /tmp/example.scss
-- "_one.scss"
-- "_three.scss"
-- "_two.scss"

Which means that importers will see _three.scss before _two.scss.

[drewwells]

Confirmed the import ordering is a problem (results below), but compilation is correct. Good catch @pkmiec !

The custom importer is being pinged for every import breadth first. It will require a more complex process where the parser sends every import encountered to the custom importer, waits for the response, then continues. Going on and on until it finds no more imports.

go run ../wt/main.go component.scss
importing button
importing color
importing color
Rebuilt: component.scss
2015/06/11 13:44:31 main.go:95: Compilation took: 8.049ms

The latter example...

importing _one.scss
importing _three.scss
importing _two.scss
/* example.scss */
/* line 1, /Users/drew/src/github.com/wellington/wellington/example/_two.scss */
.one {
  color: #200; }

/* line 2, /Users/drew/src/github.com/wellington/wellington/example/_one.scss */
.one {
  color: #100; }

/* line 1, /Users/drew/src/github.com/wellington/wellington/example/_three.scss */
.one {
  color: #300; }
Rebuilt: example.scss
2015/06/11 13:53:24 main.go:95: Compilation took: 8.932ms

[mgreter]

This should be fixed by #1528!

I get now this do_import order:

-- foo.scss
-- foo1
-- foo1a
-- foo1b
-- foo2