Fix crash when @importing a Sass file that @uses another file#2599
Conversation
|
Please fix the CI |
|
I believe sass-parser is broken at |
Goodwine
force-pushed
the
import-use-crash
branch
from
d521d43 to
320dd58
Compare
Goodwine
changed the title
nex3
left a comment
nex3
left a comment
There was a problem hiding this comment.
Please make the commit message shorter. The Linux git style guide recommends 50 characters for the first line of a commit.
| _importParent = _parent; | ||
| _parent = _root; |
There was a problem hiding this comment.
It seems like the issue here is that _parent is reset to _root. What happens if we don't do that, rather than adding an extra variable to track (and then need to un-track) a different type of parent?
There was a problem hiding this comment.
It seems like this is used to track ordering when doing @import containing @use.
e.g.
// This file is @imported into main.scss
@use 'uptream'
.foo-midstream{}Results in
.foo-midstream{}
.foo-upstream{}There was a problem hiding this comment.
Why does the existence of _parent cause that?
Goodwine
force-pushed
the
import-use-crash
branch
from
320dd58 to
af92d93
Compare
Goodwine
changed the title
|
I updated the tests but I'm still not clear whether |
discussed offline, it should fail regardless. It is only I'll also try to figure out why was |
Goodwine
force-pushed
the
import-use-crash
branch
from
7cdf182 to
7c9aa14
Compare
Goodwine
force-pushed
the
import-use-crash
branch
from
7c9aa14 to
33b0338
Compare
|
I think this is now ready, the crash actually went away recently. The Sass compiler still fails but the crash was less crashy :) |
Co-authored-by: Natalie Weizenbaum <[email protected]>
2 participants
The crash happened when
@importinga Sass file that@usesa user-defined module, only when the Sass file@includesa mixin at the root level if the mixin emits a declaration that would end up emitted at the root. This is allowed because the@importwithin a style rule would end up causing that top-level declaration to be wrapped by the style rule, and therefore won't be a top-level declaration. However, this crashed the compiler instead.Fixes: #2588
Specs: sass/sass-spec#2060