Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mixin inside at-root not working correctly #2006

Closed
xzyfer opened this issue Apr 17, 2016 · 3 comments · Fixed by #2199
Closed

mixin inside at-root not working correctly #2006

xzyfer opened this issue Apr 17, 2016 · 3 comments · Fixed by #2199

Comments

@xzyfer
Copy link
Contributor

xzyfer commented Apr 17, 2016

This code was recently added to foundation.

@mixin main() {
  bar {
    baz: 1;
  }
}

foo {
  @at-root {
    @include main();
  }
}

Ruby Sass

bar {
  baz: 1; }

LibSass

foo bar {
  baz: 1; }

I've done some digging and looks like this has never worked. The issue appears to be due to parentize not respecting at-root.

Spec added sass/sass-spec#769

xzyfer added a commit to xzyfer/sass-spec that referenced this issue Apr 17, 2016
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Apr 17, 2016
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Apr 17, 2016
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Apr 17, 2016
@xzyfer xzyfer added this to the 3.3.6 milestone Apr 17, 2016
@mgreter mgreter modified the milestones: 3.4, 3.3.6 Apr 21, 2016
@xzyfer xzyfer self-assigned this May 3, 2016
@xzyfer
Copy link
Contributor Author

xzyfer commented May 11, 2016

I'm pretty close on this. Should have something this weekend.

@xzyfer xzyfer modified the milestones: 3.3.7, 3.4, 3.3.8 May 11, 2016
@xzyfer xzyfer modified the milestones: 3.3.8, 3.4 May 20, 2016
@xzyfer xzyfer removed the Dev - WIP label May 21, 2016
@xzyfer
Copy link
Contributor Author

xzyfer commented May 21, 2016

I thought this may be fixed a side effect of correcting our cssizg'ing of at-root in #2089 but that wasn't the case.

@xzyfer
Copy link
Contributor Author

xzyfer commented May 21, 2016

I've compared our at-root implementation against Ruby Sass. Our implementation is correct in this case.
The problem here is that the nested selector is being resolved (parentised) when it shouldn't have been.
This appears to related to #1890.

This is the AST when entering CSSize.

At_Root_Block 0x7fcdc0d139a0 (0@[7:2]-[7:10]) 0
 :At_Root_Query 0x7fcdc0d12cb0 (0@[7:2]-[7:10])
  Block 0x7fcdc0d12d70 (0@[7:11]-[9:3]) [root] 0
   Trace 0x7fcdc0d12ff0 (0@[8:13]-[8:17]) [name:main]
    Block 0x7fcdc0d12f40 (0@[8:13]-[8:17]) 0
     Ruleset 0x7fcdc0d13900 (0@[1:2]-[3:3]) [indent: 0]
     >Selector_List 0x7fcdc0d130d0 (0@[1:2]-[1:5]) <8166671770794171514> [@media:0x0] - - - -
     > Complex_Selector 0x7fcdc0d13310 (0@[6:0]-[6:3]) <8166671593091081176> [weight:200] [@media:0x0] - - - - --  <>
     >  Compound_Selector 0x7fcdc0d133b0 (0@[6:0]-[6:0]) <8166660432830525886> [weight:100] [@media:0x0] - - - - - <>
     >   Type_Selector 0x7fcdc0d01610 (0@[6:0]-[6:3]) <8166660258955216540> <<foo>> - - - - <>
     > { }Complex_Selector 0x7fcdc0d135c0 (0@[1:2]-[1:5]) <8275265288974811151> [weight:100] [@media:0x0] - - - - --  <>
     > { } Compound_Selector 0x7fcdc0d00a70 (0@[1:2]-[1:2]) <8275253853970583923> [weight:100] [@media:0x0] - - - - - <>
     > { }  Type_Selector 0x7fcdc0d00b40 (0@[1:2]-[1:5]) <8275253679961073361> <<bar>> - - - - <>
      Block 0x7fcdc0d137b0 (0@[1:6]-[3:3]) 0
       Declaration 0x7fcdc0d13860 (0@[2:4]-[2:7]) 0
        prop: String_Constant 0x7fcdc0d01020 4 (0@[2:4]-[2:7]) [baz] <>
        value: String_Constant 0x7fcdc0d01150 4 (0@[2:9]-[2:11]) [1] <>

xzyfer added a commit to xzyfer/libsass that referenced this issue May 26, 2016
This is required to prevent some instances of parentizing when we
shouldn't like sass#2006. This isn't a complete fix because we incorrectly
add parent refs during parsing which also needs to be addressed.
xzyfer added a commit to xzyfer/libsass that referenced this issue May 28, 2016
This is required to prevent some instances of parentizing when we
shouldn't like sass#2006. This isn't a complete fix because we incorrectly
add parent refs during parsing which also needs to be addressed.
xzyfer added a commit to xzyfer/libsass that referenced this issue May 28, 2016
This is required to prevent some instances of parentizing when we
shouldn't like sass#2006. This isn't a complete fix because we incorrectly
add parent refs during parsing which also needs to be addressed.
xzyfer added a commit to xzyfer/libsass that referenced this issue May 28, 2016
This is required to prevent some instances of parentizing when we
shouldn't like sass#2006. This isn't a complete fix because we incorrectly
add parent refs during parsing which also needs to be addressed.
@xzyfer xzyfer modified the milestones: 3.4, 3.4.1 Sep 8, 2016
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Oct 3, 2016
xzyfer added a commit to xzyfer/libsass that referenced this issue Oct 4, 2016
LibSass fully parses selectors during the parsing stage. This is
different from Ruby Sass which parse selectors and string during
the parsing stage, and lazily eval'ing when required. This
difference causes some pathological selector issues. Our eager
parsing of selectors has resulted in us hacking in fake
`Parent_Selector` into `Sequence_Selector` during parsing.

These fake `Parent_Selector` play havoc with `resolve_parent_refs`
when within `@at-root` blocks. I spent a couple weeks going down
the rabbit whole of refactoring our selector parsing to be lazy before
bailing.

Explicitly marking which `Parent_Selector` are fake during parsing
allows us faithfully implement the `implicit_parent` flag in
`resolve_parent_refs`.

Fixes sass#2006
Fixes sass#2198
Spec sass/sass-spec#936
@xzyfer xzyfer modified the milestones: 3.4, 3.4.1 Oct 4, 2016
xzyfer added a commit to xzyfer/libsass that referenced this issue Oct 4, 2016
LibSass fully parses selectors during the parsing stage. This is
different from Ruby Sass which parse selectors and string during
the parsing stage, and lazily eval'ing when required. This
difference causes some pathological selector issues. Our eager
parsing of selectors has resulted in us hacking in fake
`Parent_Selector` into `Sequence_Selector` during parsing.

These fake `Parent_Selector` play havoc with `resolve_parent_refs`
when within `@at-root` blocks. I spent a couple weeks going down
the rabbit whole of refactoring our selector parsing to be lazy before
bailing.

Explicitly marking which `Parent_Selector` are fake during parsing
allows us faithfully implement the `implicit_parent` flag in
`resolve_parent_refs`.

Fixes sass#2006
Fixes sass#2198
Spec sass/sass-spec#936
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment