Incorrect nested import ordering

[adam-h]

I just discovered this bug, where two imports that both import the same file can end up with the imported file out-of-order intermittently (due to parallel loading).

For example, if two separate files import base.css, in some cases the second file will process first and actually include it. Whereas the first file will not (due to de-duping) and then the result will be out-of-order and can affect the resulting applied styles (or variable definitions).

I have a full test case in my branch: adam-h@569ea1e

Note how the first import is delayed so that the second will always process first (removing the randomness of occurance due to the filesystem).

[jednano]

Can you create a PR with a breaking test?

[adam-h]

Done :) #169

[ghost]

What is the status on this issue? I am also affected by it.

[MoOx]

The status of an opened issue is: the issue is still open, so not fixed.

[ghost]

No worries, does anybody has a workaround for this issue? Perhaps disabling de-duping?

[ghost]

For anyone looking at this issue, temporary fix is to set skipDuplicates: false.

[simplesmiler]

skipDuplicates: false does not solve the issue for me.

---

EDIT: maybe there could be an option to disable parallel loading?

[paulyoung]

I believe I am also experiencing this issue. I was about to post the following which sounds like postcss/postcss-simple-vars#42 and looks a lot like #169.

Doing the following causes intermittent TypeError: Cannot read property 'toString' of undefined errors.

a.css

@import 'some-module/variables.css';
// use variables from variables.css

b.css

@import 'some-module/variables.css';
// use variables from variables.css

index.css

@import './a.css';
@import './b.css';

As a result we've had to resort to:

a.css

/*@import 'some-module/variables.css';*/
// use variables from variables.css

b.css

/*@import 'some-module/variables.css';*/
// use variables from variables.css

index.css

@import 'some-module/variables.css';
@import './a.css';
@import './b.css';

[MoOx]

Please use v7 while it's resolved.

[paulyoung]

That appears to work. Thanks!

[MoOx]

@RyanZim maybe everything is a mess because of this line

postcss-import/index.js

Line 166 in 6bbcdbd

return Promise.all(statements.map(function(stmt) {

Promise.all might resolve without taking the correct expected order (really not sure about that, but that may be a clue)

[RyanZim]

@MoOx Did a little fooling, couldn't figure it out. Mind elaborating a bit?

---

(Un)related question: What does resolveImportId() actually do? It returns a promise, but the final then in the promise chain doesn't return anything. I assume it mutates an object somewhere, but what object does it mutate?

[MoOx]

As I said before, I am not the author of this code base, and the author decided to ignore this project for some reasons (that's why I talked about reverting to v7, which is a codebase I know). Anyway I will try to give you some pointers.

At the end, resolveImportId() mutate the initial passed statement (called stmt) childrens. The naming of "result" inside the function is a bit confusing since the parent block level also have a "result" variable.

I merged a failing test into master, so know you can do the following:

• ava test/order.js to trigger the failure and iterate with this.

[RyanZim]

@MoOx Progress! 🎉 I think I am getting closer to the issue.

[RyanZim]

@MoOx @ai I have found the issue and made a fix, however, I broke the should apply plugins to root test at https://github.com/postcss/postcss-import/blob/master/test/plugins.js in the process. I suspect the test depended on the the old incorrect behavior. Will investigate tomorrow.

@MoOx If that test is depending on incorrect behavior, please tell me/submit a PR.

[RyanZim]

Thought I would leave this for tomorrow; I was so engrossed, I had to fix it tonight! The test was faulty.

PR done: #236. 🎉