Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Adding a test for importer callback order #998

Closed
wants to merge 1 commit into from

Conversation

beatak
Copy link
Contributor

@beatak beatak commented Jun 10, 2015

This pull request is to add a test case for sass/libsass#1262 and this test is currently failing. I have chatted with @xzyfer and he thinks it's a bug.

Besides the main point (the callback order is not DFS), I also found that the importer won't callback for the nested nodes.

This test add index.scss and tree is as follows:

                   +-----+            
                   |index|            
                   +--+--+            
                      |               
         +-------------------------+  
        +-+        +------+       +-+ 
        |a|        |common|       |b| 
        +++        +--+---+       +++ 
         |            |            |  
   +-----+--+     +---+----+       |  
+------+  +--+  +----+ +------+   ++-+
|common|  |a1|  |vars| |struct|   |b1|
+--+---+  +--+  +----+ +------+   +--+
   |                                  
  +--------+                          
+----+ +------+                       
|vars| |struct|                       
+----+ +------+                       

Actually, node-sass calls nodes in the order of [a, common, b, common, a1, vars, struct, b1] instead of [a, common, vars, struct, a1, common, vars, struct, b, b1] Not only the order is wrong, but also vars and struct didn't get called twice.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 11, 2015

Great, I'll dig into this with you tomorrow. I looked into it briefly for sass/libsass#1262 but was not able to reproduce as per sass/libsass#1262 (comment).

@xzyfer
Copy link
Contributor

xzyfer commented Jun 11, 2015

I wonder if you're running into #894

@xzyfer
Copy link
Contributor

xzyfer commented Jun 11, 2015

I've been able to confirm this is an issue with custom importers only. Libsass is resolving @imports depth-first, but custom importers are being call breadth-first.

@xzyfer
Copy link
Contributor

xzyfer commented Mar 23, 2016

Superseded by #1420

@xzyfer xzyfer closed this Mar 23, 2016
jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants