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

Order of includes changed in libsass 3.3 #1123

Closed
saper opened this issue Sep 6, 2015 · 11 comments
Closed

Order of includes changed in libsass 3.3 #1123

saper opened this issue Sep 6, 2015 · 11 comments

Comments

@saper
Copy link
Member

saper commented Sep 6, 2015

I am getting this trying to test an update to #1040 (using sass/libsass@5b4122d):

  1) api .render({stats: {}}) should contain an array of all included files:

      Uncaught [ '/home/saper/node_modules/node-sass/test/fixtures/include-files/index.scss',
  '/home/saper/node_modules/node-sass/test/fixtur deepEqual [ '/home/saper/node_modules/node-sass/test/fixtures/include-files/bar.scss',
  '/home/saper/node_modules/node-sass/test/fixtures
      + expected - actual

       [
      -  "/home/saper/node_modules/node-sass/test/fixtures/include-files/index.scss"
         "/home/saper/node_modules/node-sass/test/fixtures/include-files/bar.scss"
         "/home/saper/node_modules/node-sass/test/fixtures/include-files/foo.scss"
      +  "/home/saper/node_modules/node-sass/test/fixtures/include-files/index.scss"
       ]



  2) api .renderSync({stats: {}}) should contain an array of all included files:

      '/home/saper/node_modules/node-sass/test/fixtures/include-files/index.scss' == '/home/saper/node_modules/node-sass/test/fixtures/include-files/bar.scss'
      + expected - actual

      -/home/saper/node_modules/node-sass/test/fixtures/include-files/index.scss
      +/home/saper/node_modules/node-sass/test/fixtures/include-files/bar.scss

Is the order now correct?

@xzyfer
Copy link
Contributor

xzyfer commented Sep 7, 2015

@saper I expect the new order is correct. This sounds like it's related to sass/libsass#1262

@saper
Copy link
Member Author

saper commented Sep 7, 2015

so what are we testing here actually :)

@xzyfer
Copy link
Contributor

xzyfer commented Sep 7, 2015

My gut is the test was just based on the output Libsass produced assuming it was correct. Before my time :)

@saper
Copy link
Member Author

saper commented Sep 7, 2015

Not my department, either :)

@xzyfer
Copy link
Contributor

xzyfer commented Sep 7, 2015

I'm not entirely sure either is correct to be honest.

@xzyfer
Copy link
Contributor

xzyfer commented Sep 7, 2015

includedFiles (Array) - Absolute paths to all related scss files in no particular order.

@saper
Copy link
Member Author

saper commented Sep 7, 2015

includedFiles (Array) - Absolute paths to all related scss files in no particular order.

I'm relieved, thanks!

@saper
Copy link
Member Author

saper commented Sep 7, 2015

Well, still valid - we need to fix the test to ignore the order.

@xzyfer
Copy link
Contributor

xzyfer commented Sep 7, 2015

I figured it all belonged in #1040

@saper
Copy link
Member Author

saper commented Sep 7, 2015

poor me :) no problem

saper added a commit to saper/node-sass that referenced this issue Sep 7, 2015
@saper
Copy link
Member Author

saper commented Sep 7, 2015

Should be fixed in saper@160eb46

saper added a commit to saper/node-sass that referenced this issue Sep 7, 2015
saper added a commit to saper/node-sass that referenced this issue Sep 7, 2015
saper added a commit to saper/node-sass that referenced this issue Sep 9, 2015
saper added a commit to saper/node-sass that referenced this issue Sep 16, 2015
@xzyfer xzyfer modified the milestone: next.libsass Oct 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants