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

for loop fails on undefined vars #446

Open
joskoomen opened this Issue Jun 24, 2016 · 46 comments

Comments

Projects
None yet
@joskoomen

joskoomen commented Jun 24, 2016

My project need compiles based on theme colors. This means i have to add vars dynamically to override scss vars.

The project uses Foundation sites (6.1) scss. But some loops return undefined var errors:

@for $i from 1 through $grid-column-count {
  // Column width
  .#{$-zf-size}-#{$i} {
    @include grid-col-size($i); // this $i is undefined in the compiler
  }

  // Source ordering
  @if $i < $grid-column-count {
    .#{$-zf-size}-#{$push}-#{$i} {
      @include grid-col-pos($i); // this $i is undefined in the compiler
    }

    .#{$-zf-size}-#{$pull}-#{$i} {
      @include grid-col-pos(-$i); // this $i is undefined in the compiler
    }
  }

  // Offsets
  $o: $i - 1;
  .#{$-zf-size}-#{$offset}-#{$o} {
    @include grid-col-off($o);
  }
}

I've added comments where the variables are undefined. I've noticed it with a similar situation:

// Heading sizes
@each $size, $headers in $header-sizes {
  @include breakpoint($size) {
    @each $header, $font-size in $headers {
      #{$header} {
        font-size : rem-calc($font-size); // this $font-size var is undefined
      }
    }
  }
}

So it seems to fail within loops and inside mixin includes

@FMCorz

This comment has been minimized.

Show comment
Hide comment
@FMCorz

FMCorz Jul 11, 2016

Contributor

I have reproduced this error when trying to compile Bootstrap 4 alpha 2.

Exception - Undefined variable $i: line: 15

line 3321 of scssphp/src/Compiler.php: Leafo\ScssPhp\Exception\CompilerException thrown
line 2991 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->throwError()
line 1990 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->get()
line 3615 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->reduce()
line 1717 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->applyArguments()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 958 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1449 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileBlock()
line 1186 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1644 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildren()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1741 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 958 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1449 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileBlock()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1722 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1722 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 958 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1449 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileBlock()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 295 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 194 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileRoot()
line 14 of /test.php: call to Leafo\ScssPhp\Compiler->compile()

Test case:

@mixin container() {
    #container {
        @content;
    }
}
@mixin make-col-span($size) {
    width: $size;
}

@mixin make-stuff() {
    @include container() {
      @for $i from 1 through 3 {
        .col-#{$i} {
            color: red;
            @include make-col-span($i);
        }
      }
    }
}

div {
    @include make-stuff();
}

Expected:

div #container .col-1 {
  color: red;
  width: 1;
}
div #container .col-2 {
  color: red;
  width: 2;
}
div #container .col-3 {
  color: red;
  width: 3;
}
Contributor

FMCorz commented Jul 11, 2016

I have reproduced this error when trying to compile Bootstrap 4 alpha 2.

Exception - Undefined variable $i: line: 15

line 3321 of scssphp/src/Compiler.php: Leafo\ScssPhp\Exception\CompilerException thrown
line 2991 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->throwError()
line 1990 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->get()
line 3615 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->reduce()
line 1717 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->applyArguments()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 958 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1449 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileBlock()
line 1186 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1644 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildren()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1741 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 958 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1449 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileBlock()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1722 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 1722 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 958 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 1449 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileBlock()
line 1205 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChild()
line 295 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileChildrenNoReturn()
line 194 of scssphp/src/Compiler.php: call to Leafo\ScssPhp\Compiler->compileRoot()
line 14 of /test.php: call to Leafo\ScssPhp\Compiler->compile()

Test case:

@mixin container() {
    #container {
        @content;
    }
}
@mixin make-col-span($size) {
    width: $size;
}

@mixin make-stuff() {
    @include container() {
      @for $i from 1 through 3 {
        .col-#{$i} {
            color: red;
            @include make-col-span($i);
        }
      }
    }
}

div {
    @include make-stuff();
}

Expected:

div #container .col-1 {
  color: red;
  width: 1;
}
div #container .col-2 {
  color: red;
  width: 2;
}
div #container .col-3 {
  color: red;
  width: 3;
}
@robverhoef

This comment has been minimized.

Show comment
Hide comment
@robverhoef

robverhoef Jul 12, 2016

I see a similar problem with foundation-sites 6.2.3 (compiling bower_components/foundation-sites/assets/foundation.scss):

Parse error: Undefined variable $i: line: 86

robverhoef commented Jul 12, 2016

I see a similar problem with foundation-sites 6.2.3 (compiling bower_components/foundation-sites/assets/foundation.scss):

Parse error: Undefined variable $i: line: 86

@FMCorz

This comment has been minimized.

Show comment
Hide comment
@FMCorz

FMCorz Jul 12, 2016

Contributor

Could someone please review and comment on the following patch?

master...FMCorz:issue-446

It's clearly not ready to be merged, but I just wonder if I'm heading in the right direction, and how I can improve the patch further so that this can be fixed.

In short, I think that the problem only arises when we've got two @include leading to resolving the variables of a function call. Because the environments of both @include are marked as mixin the variable resolution stops.

Ultimately I think that the function resolving the variables should be given an appropriate scope to start with rather than changing the variable resolution, but I need some guidance as to how to do this.

Cheers!

Contributor

FMCorz commented Jul 12, 2016

Could someone please review and comment on the following patch?

master...FMCorz:issue-446

It's clearly not ready to be merged, but I just wonder if I'm heading in the right direction, and how I can improve the patch further so that this can be fixed.

In short, I think that the problem only arises when we've got two @include leading to resolving the variables of a function call. Because the environments of both @include are marked as mixin the variable resolution stops.

Ultimately I think that the function resolving the variables should be given an appropriate scope to start with rather than changing the variable resolution, but I need some guidance as to how to do this.

Cheers!

@Juje

This comment has been minimized.

Show comment
Hide comment
@Juje

Juje Jul 13, 2016

@FMCorz your patch works as it should for the issue from #449.

So as far as I can see it it should be imported into the repository @leafo?

Juje commented Jul 13, 2016

@FMCorz your patch works as it should for the issue from #449.

So as far as I can see it it should be imported into the repository @leafo?

@robverhoef

This comment has been minimized.

Show comment
Hide comment
@robverhoef

robverhoef Jul 13, 2016

@FMCorz Unfortunately it doesn't work for compiling foundation-sites 6.2.3. I still see:
Parse error: Undefined variable $i: line: 86

robverhoef commented Jul 13, 2016

@FMCorz Unfortunately it doesn't work for compiling foundation-sites 6.2.3. I still see:
Parse error: Undefined variable $i: line: 86

@FMCorz

This comment has been minimized.

Show comment
Hide comment
@FMCorz

FMCorz Jul 14, 2016

Contributor

@Juje I don't think it should be imported as it, the patch need some clean-up. Beside @robverhoef pointed that Foundation doesn't compile so more work is needed.

Contributor

FMCorz commented Jul 14, 2016

@Juje I don't think it should be imported as it, the patch need some clean-up. Beside @robverhoef pointed that Foundation doesn't compile so more work is needed.

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Aug 22, 2016

Has there been any traction WRT this patch?

paxperscientiam commented Aug 22, 2016

Has there been any traction WRT this patch?

@FMCorz

This comment has been minimized.

Show comment
Hide comment
@FMCorz

FMCorz Aug 23, 2016

Contributor

Have you guys tried this? #453

Contributor

FMCorz commented Aug 23, 2016

Have you guys tried this? #453

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Aug 23, 2016

HI @FMCorz

I've required your issue-446 branch but it's hash is "08a458c...". Have you not commited to said branch?

paxperscientiam commented Aug 23, 2016

HI @FMCorz

I've required your issue-446 branch but it's hash is "08a458c...". Have you not commited to said branch?

@FMCorz

This comment has been minimized.

Show comment
Hide comment
@FMCorz

FMCorz Aug 24, 2016

Contributor

I'm not sure I am following but that branch was not up-to-date. Most of the remaining work on this library has been done under the repo moodlehq/scssphp. I have updated my pull request #453 to use the latest patch. Could you check now?

Edit: Never mind, I confused myself... my branch is not used in my pull request (moodlehq's nested-mixins is). Ignore the branch issue-446, even though it should now be up-to-date.

Contributor

FMCorz commented Aug 24, 2016

I'm not sure I am following but that branch was not up-to-date. Most of the remaining work on this library has been done under the repo moodlehq/scssphp. I have updated my pull request #453 to use the latest patch. Could you check now?

Edit: Never mind, I confused myself... my branch is not used in my pull request (moodlehq's nested-mixins is). Ignore the branch issue-446, even though it should now be up-to-date.

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Aug 24, 2016

@FMCorz ohhhhh. Sorry, somewhere I got confused.

OK, I will ignore issue-446 and try the "nested-mixins" branch of moodlehq/scssphp.

BRB.

paxperscientiam commented Aug 24, 2016

@FMCorz ohhhhh. Sorry, somewhere I got confused.

OK, I will ignore issue-446 and try the "nested-mixins" branch of moodlehq/scssphp.

BRB.

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Aug 24, 2016

Unfortunately, it is still not working for me. I'm getting an error similar to that reported by @robverhoef as I too am using Foundation. scssphp doesn't seem to like iteration variables.

paxperscientiam commented Aug 24, 2016

Unfortunately, it is still not working for me. I'm getting an error similar to that reported by @robverhoef as I too am using Foundation. scssphp doesn't seem to like iteration variables.

@darknailblue

This comment has been minimized.

Show comment
Hide comment
@darknailblue

darknailblue Aug 24, 2016

I'm attempting to compile foundation and running into the same issue. @robocoder Any idea on a quick fix for this?

darknailblue commented Aug 24, 2016

I'm attempting to compile foundation and running into the same issue. @robocoder Any idea on a quick fix for this?

@darknailblue

This comment has been minimized.

Show comment
Hide comment
@darknailblue

darknailblue Aug 24, 2016

I know this is slightly off-topic but @robverhoef @paxperscientiam Just to give you guys a heads up the @includes that AREN'T compiling on foundation are the foundation-grid include and the foundation-flex-classes. If you @include every component manually you can get around this and have a separate CSS file for grid and flex classes.

darknailblue commented Aug 24, 2016

I know this is slightly off-topic but @robverhoef @paxperscientiam Just to give you guys a heads up the @includes that AREN'T compiling on foundation are the foundation-grid include and the foundation-flex-classes. If you @include every component manually you can get around this and have a separate CSS file for grid and flex classes.

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Aug 24, 2016

Hey @darknailblue,

Thanks for pointing this out, though I also had to comment out "@include foundation-flex-classes" to get a successful transpilation.

Thanks for the suggestion too, though, for now, I'm going to stick with using a combination of nodemon, node-sass, and browsersync for development. scssphp is by far the fastest transpiler I've come across, so I hope this bug can be remedied soon.

EDIT: grammar

paxperscientiam commented Aug 24, 2016

Hey @darknailblue,

Thanks for pointing this out, though I also had to comment out "@include foundation-flex-classes" to get a successful transpilation.

Thanks for the suggestion too, though, for now, I'm going to stick with using a combination of nodemon, node-sass, and browsersync for development. scssphp is by far the fastest transpiler I've come across, so I hope this bug can be remedied soon.

EDIT: grammar

@darknailblue

This comment has been minimized.

Show comment
Hide comment
@darknailblue

darknailblue Aug 24, 2016

@paxperscientiam I ended up having to do the same.

darknailblue commented Aug 24, 2016

@paxperscientiam I ended up having to do the same.

@robocoder robocoder added needs-test and removed has-test-case labels Sep 11, 2016

@robocoder

This comment has been minimized.

Show comment
Hide comment
@robocoder

robocoder Sep 11, 2016

Collaborator

v0.6.6 has been tagged and released. Can someone provide an updated test case for this issue? Thanks.

Collaborator

robocoder commented Sep 11, 2016

v0.6.6 has been tagged and released. Can someone provide an updated test case for this issue? Thanks.

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Sep 18, 2016

Hi @robocoder ,

I'm testing out the latest release of scssphp (with foundation-sites 6.2.3); however, I am still seeing this error manifest as "Parse error: Undefined variable $i".

EDIT: As mentioned by @darknailblue, it's the grid files where it gets tripped up.

paxperscientiam commented Sep 18, 2016

Hi @robocoder ,

I'm testing out the latest release of scssphp (with foundation-sites 6.2.3); however, I am still seeing this error manifest as "Parse error: Undefined variable $i".

EDIT: As mentioned by @darknailblue, it's the grid files where it gets tripped up.

@JumpLink

This comment has been minimized.

Show comment
Hide comment
@JumpLink

JumpLink Sep 21, 2016

I've created a fork of bootstrap 4 Alpha 4 with backward compatibility for older versions of Sass which should working: https://github.com/JumpLinkNetwork/bootstrap-backward

JumpLink commented Sep 21, 2016

I've created a fork of bootstrap 4 Alpha 4 with backward compatibility for older versions of Sass which should working: https://github.com/JumpLinkNetwork/bootstrap-backward

@garrett-eclipse

This comment has been minimized.

Show comment
Hide comment
@garrett-eclipse

garrett-eclipse Oct 12, 2016

Having this same issue with foundation and the grid. Commenting so as to follow this thread for any fixes/updates. Thanks

garrett-eclipse commented Oct 12, 2016

Having this same issue with foundation and the grid. Commenting so as to follow this thread for any fixes/updates. Thanks

@Crunky

This comment has been minimized.

Show comment
Hide comment
@Crunky

Crunky Oct 13, 2016

@JumpLink Thanks. I had similar issues with leafo scssphp. Compile work fine with backwards-files but afterwards there are issues in boostrap colums, col-md-3 + col-md-9 e.g. aren´t side by side in a row.

I double checked it.

Crunky commented Oct 13, 2016

@JumpLink Thanks. I had similar issues with leafo scssphp. Compile work fine with backwards-files but afterwards there are issues in boostrap colums, col-md-3 + col-md-9 e.g. aren´t side by side in a row.

I double checked it.

@JumpLink

This comment has been minimized.

Show comment
Hide comment
@JumpLink

JumpLink Oct 13, 2016

@Crunky did you use the latest Version? I use it in productive and it Works fine for me

JumpLink commented Oct 13, 2016

@Crunky did you use the latest Version? I use it in productive and it Works fine for me

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam commented Dec 12, 2016

bump

@ivanjaros

This comment has been minimized.

Show comment
Hide comment
@ivanjaros

ivanjaros Feb 4, 2017

Is there any working solution for Foundation(6.3.0)?
*
Commenting out grid and flex classes makes the foundation to compile.
*
The code is too big for me to wrap my head around but maybe the for loops could use new compiler instance instead of the global one?

ivanjaros commented Feb 4, 2017

Is there any working solution for Foundation(6.3.0)?
*
Commenting out grid and flex classes makes the foundation to compile.
*
The code is too big for me to wrap my head around but maybe the for loops could use new compiler instance instead of the global one?

@ivanjaros

This comment has been minimized.

Show comment
Hide comment
@ivanjaros

ivanjaros Feb 5, 2017

btw, any guidance on what is causing the issue would be nice. I'd like to look at it but the code is just too big to just dive into.

ivanjaros commented Feb 5, 2017

btw, any guidance on what is causing the issue would be nice. I'd like to look at it but the code is just too big to just dive into.

@ivanjaros

This comment has been minimized.

Show comment
Hide comment
@ivanjaros

ivanjaros Feb 5, 2017

If someone's interested, I'd pay 50€ for fixing this and making foundation 6.3.0 compile with flex grid.
100€ if the fix is merged before sunday 12th.


I have moved to NodeJS solution so the 50€ is no longer in play but that 100€ is still worth it for me(only 3 days left though).

ivanjaros commented Feb 5, 2017

If someone's interested, I'd pay 50€ for fixing this and making foundation 6.3.0 compile with flex grid.
100€ if the fix is merged before sunday 12th.


I have moved to NodeJS solution so the 50€ is no longer in play but that 100€ is still worth it for me(only 3 days left though).

@GitNey

This comment has been minimized.

Show comment
Hide comment
@GitNey

GitNey Feb 7, 2017

Stumbled across this today while trying to find a work around for compiling SASS without SSH access on WP-Joints theme in WP. Hoping the attention moves this patch closer to completion.

GitNey commented Feb 7, 2017

Stumbled across this today while trying to find a work around for compiling SASS without SSH access on WP-Joints theme in WP. Hoping the attention moves this patch closer to completion.

@paxperscientiam

This comment has been minimized.

Show comment
Hide comment
@paxperscientiam

paxperscientiam Feb 11, 2017

For anyone interested in a temporary solution, here is a project I've put together that leverages node-sass, browser-sync, and some other stuff.

@ivanjaros You say you've moved on; maybe we can swap ideas. Feel free to check out my repo.
https://github.com/paxperscientiam/webdev-proxy-server

YMMV.

paxperscientiam commented Feb 11, 2017

For anyone interested in a temporary solution, here is a project I've put together that leverages node-sass, browser-sync, and some other stuff.

@ivanjaros You say you've moved on; maybe we can swap ideas. Feel free to check out my repo.
https://github.com/paxperscientiam/webdev-proxy-server

YMMV.

@penguinstampede

This comment has been minimized.

Show comment
Hide comment
@penguinstampede

penguinstampede Mar 16, 2017

fwiw I'm running into two issues in Foundation 6.3.1, both dealing with foundation-grid.
in _row.scss, Undefined variable $gutters

@mixin grid-row-nest($gutters: $grid-column-gutter) { @include -zf-each-breakpoint { $margin: rem-calc(-zf-get-bp-val($gutters, $-zf-size)) / 2 * -1; margin-right: $margin; margin-left: $margin; } }

and in _classes.scss, lots of problems with $i

@include -zf-each-breakpoint { @for $i from 1 through $grid-column-count { // Column width .#{$-zf-size}-#{$i} { @include grid-col-size($i); } ...

Trying to get a proper failing test case based on this.

penguinstampede commented Mar 16, 2017

fwiw I'm running into two issues in Foundation 6.3.1, both dealing with foundation-grid.
in _row.scss, Undefined variable $gutters

@mixin grid-row-nest($gutters: $grid-column-gutter) { @include -zf-each-breakpoint { $margin: rem-calc(-zf-get-bp-val($gutters, $-zf-size)) / 2 * -1; margin-right: $margin; margin-left: $margin; } }

and in _classes.scss, lots of problems with $i

@include -zf-each-breakpoint { @for $i from 1 through $grid-column-count { // Column width .#{$-zf-size}-#{$i} { @include grid-col-size($i); } ...

Trying to get a proper failing test case based on this.

@maplesyrupsucker

This comment has been minimized.

Show comment
Hide comment
@maplesyrupsucker

maplesyrupsucker Mar 22, 2017

Any updates on the flex-grid and grid compilation error with Foundation? Came across it today unfortunately. Seems to be affecting lots of Wordpress themes built with Foundation 6.2+ including WP-Joints

maplesyrupsucker commented Mar 22, 2017

Any updates on the flex-grid and grid compilation error with Foundation? Came across it today unfortunately. Seems to be affecting lots of Wordpress themes built with Foundation 6.2+ including WP-Joints

@sekjal

This comment has been minimized.

Show comment
Hide comment
@sekjal

sekjal May 4, 2017

Working on this now so I can use Foundation 6.3.1.

Looks like the for variable ($i in this case) value isn't getting passed to it's children.

The issues with $gutters is separate, I think; I was able to fix it by altering the .scss file slightly. Not ideal, but it got it out of the way so I could work on this.

sekjal commented May 4, 2017

Working on this now so I can use Foundation 6.3.1.

Looks like the for variable ($i in this case) value isn't getting passed to it's children.

The issues with $gutters is separate, I think; I was able to fix it by altering the .scss file slightly. Not ideal, but it got it out of the way so I could work on this.

@sekjal

This comment has been minimized.

Show comment
Hide comment
@sekjal

sekjal May 5, 2017

Further investigation shows that variables were being set properly in the for environment, and that the whole structure of the system doesn't pass variables to children, but rather appends the child environment to the parent environment tree.

This led me to Compiler::get(), where environments are popped off the tree until the appropriate one is found. There is a little bit of extra logic here to essentially "fall out" of the tree under certain conditions, skipping variables at higher levels and going straight to the root environment. In doing so, $i was never resolved, because it was skipped.

Solution: comment out Compiler.php line 3122 $nextIsRoot = true;

This also solves the issue with $gutters, and the whole of Foundation 6.3.1 now compiles, and seems to render the correct CSS when compared to the distribution file (whitespace aside).

I'm sure, however, there are side effects to this change. I presume the nextIsRoot logic was placed there for a reason. Perhaps there are situations where a variable is defined at one of those skipped levels of the tree that is also defined at root, and that's the value we really want, but I can't think of an example at the moment. I would think, though, if you redefine a variable in a narrower scope, then call a mixin or include, it would make sense to use that value, not the root value, for all child scopes... does this jive with SCSS spec?

sekjal commented May 5, 2017

Further investigation shows that variables were being set properly in the for environment, and that the whole structure of the system doesn't pass variables to children, but rather appends the child environment to the parent environment tree.

This led me to Compiler::get(), where environments are popped off the tree until the appropriate one is found. There is a little bit of extra logic here to essentially "fall out" of the tree under certain conditions, skipping variables at higher levels and going straight to the root environment. In doing so, $i was never resolved, because it was skipped.

Solution: comment out Compiler.php line 3122 $nextIsRoot = true;

This also solves the issue with $gutters, and the whole of Foundation 6.3.1 now compiles, and seems to render the correct CSS when compared to the distribution file (whitespace aside).

I'm sure, however, there are side effects to this change. I presume the nextIsRoot logic was placed there for a reason. Perhaps there are situations where a variable is defined at one of those skipped levels of the tree that is also defined at root, and that's the value we really want, but I can't think of an example at the moment. I would think, though, if you redefine a variable in a narrower scope, then call a mixin or include, it would make sense to use that value, not the root value, for all child scopes... does this jive with SCSS spec?

@ADoebeling

This comment has been minimized.

Show comment
Hide comment
@ADoebeling

ADoebeling May 8, 2017

Solution: comment out Compiler.php line 3122 $nextIsRoot = true;

@sekjal Thanks a lot, this patched the bug in our cms @contao

ADoebeling commented May 8, 2017

Solution: comment out Compiler.php line 3122 $nextIsRoot = true;

@sekjal Thanks a lot, this patched the bug in our cms @contao

blairwinans added a commit to RhymeDigital/contao_zurb_foundation that referenced this issue May 12, 2017

@RedLucas

This comment has been minimized.

Show comment
Hide comment
@RedLucas

RedLucas Jul 5, 2017

Hey, I got a different Foundation error. Foundation 6.4 specifically involving foundation-sites/scss/components/_breadcrumbs.scss and the $breadcrumbs-item-separator-item-rtl: '\\' !default; variable. I believe the backslash causes some problems.

I also get the issues with the -zf-each-breakpoint mixin. I'm just about to investigate it right now because I must have it working today.

RedLucas commented Jul 5, 2017

Hey, I got a different Foundation error. Foundation 6.4 specifically involving foundation-sites/scss/components/_breadcrumbs.scss and the $breadcrumbs-item-separator-item-rtl: '\\' !default; variable. I believe the backslash causes some problems.

I also get the issues with the -zf-each-breakpoint mixin. I'm just about to investigate it right now because I must have it working today.

@pi-tavis

This comment has been minimized.

Show comment
Hide comment
@pi-tavis

pi-tavis Jul 17, 2017

I also get the issues with the -zf-each-breakpoint mixin. I'm just about to investigate it right now because I must have it working today.

@RedLucas Do you got a solution for that issue? I am stuck on the same problem.

pi-tavis commented Jul 17, 2017

I also get the issues with the -zf-each-breakpoint mixin. I'm just about to investigate it right now because I must have it working today.

@RedLucas Do you got a solution for that issue? I am stuck on the same problem.

@RedLucas

This comment has been minimized.

Show comment
Hide comment
@RedLucas

RedLucas Jul 17, 2017

I do have it working... though to be honest the way I got it working I'm not sure I remember. I believe I came to the conclusion that it was the foundation-sites' dependency, specifically sassy-lists, that wasn't working properly. As in the function doesn't exist! Importing the whole foundation sass as a whole fixed it I think, though you could probably import those dependencies separately and have it work as well.

@import 'foundation-sites/_vendor/sassy-lists/stylesheets/helpers/true';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/purge';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/remove';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/replace';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/to-list';

So is this a scssphp issue? I suppose not really... though I'd have loved a little better error handling for missing functions.. I'm not gonna blame someone else for myself not including the dependencies I needed.

RedLucas commented Jul 17, 2017

I do have it working... though to be honest the way I got it working I'm not sure I remember. I believe I came to the conclusion that it was the foundation-sites' dependency, specifically sassy-lists, that wasn't working properly. As in the function doesn't exist! Importing the whole foundation sass as a whole fixed it I think, though you could probably import those dependencies separately and have it work as well.

@import 'foundation-sites/_vendor/sassy-lists/stylesheets/helpers/true';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/purge';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/remove';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/replace';
@import 'foundation-sites/_vendor/sassy-lists/stylesheets/functions/to-list';

So is this a scssphp issue? I suppose not really... though I'd have loved a little better error handling for missing functions.. I'm not gonna blame someone else for myself not including the dependencies I needed.

@pi-tavis

This comment has been minimized.

Show comment
Hide comment
@pi-tavis

pi-tavis Jul 18, 2017

Thanks for your suggest. It seems my problem was different to yours. I managed to get a successfull compile with these two changes:

  • comment out Compiler.php line 3122 $nextIsRoot = true; (as mentioned by @sekjal )
  • define variable $str: ''; somewhere in settings.scss

I don't know if this is a 100% scssphp issue.

pi-tavis commented Jul 18, 2017

Thanks for your suggest. It seems my problem was different to yours. I managed to get a successfull compile with these two changes:

  • comment out Compiler.php line 3122 $nextIsRoot = true; (as mentioned by @sekjal )
  • define variable $str: ''; somewhere in settings.scss

I don't know if this is a 100% scssphp issue.

@RedLucas

This comment has been minimized.

Show comment
Hide comment
@RedLucas

RedLucas Jul 18, 2017

Oh, I had to do $nextIsRoot = true; as well. but that defined variable thing is an interesting thing to try if I run into any similar issue.

I would say that anything compiled in scssphp that is inconsistent with compiling the same thing in libsass (of whatever version it is declared to be at) deserves a look at. I'm sure people expect parity eh?

Regardless scssphp is an awesome piece of software. :)

RedLucas commented Jul 18, 2017

Oh, I had to do $nextIsRoot = true; as well. but that defined variable thing is an interesting thing to try if I run into any similar issue.

I would say that anything compiled in scssphp that is inconsistent with compiling the same thing in libsass (of whatever version it is declared to be at) deserves a look at. I'm sure people expect parity eh?

Regardless scssphp is an awesome piece of software. :)

@slucas-nwx

This comment has been minimized.

Show comment
Hide comment
@slucas-nwx

slucas-nwx Aug 3, 2017

Is there any status on fixing this?

slucas-nwx commented Aug 3, 2017

Is there any status on fixing this?

@CotswoldPhoto

This comment has been minimized.

Show comment
Hide comment
@CotswoldPhoto

CotswoldPhoto Dec 14, 2017

To bump the above question, is this yet fixed?

CotswoldPhoto commented Dec 14, 2017

To bump the above question, is this yet fixed?

@santiagoarizti

This comment has been minimized.

Show comment
Hide comment
@santiagoarizti

santiagoarizti Feb 9, 2018

This is happening to me today with foundation version 6.4.4-rc1 (according to package-lock.json)
and scssphp version v0.7.4 (according to composer.lock)

error is: Undefined variable $i: line: 86

referring to file _classes.scss on line 86 (below the part where it says: // Sizing (percentage) )

santiagoarizti commented Feb 9, 2018

This is happening to me today with foundation version 6.4.4-rc1 (according to package-lock.json)
and scssphp version v0.7.4 (according to composer.lock)

error is: Undefined variable $i: line: 86

referring to file _classes.scss on line 86 (below the part where it says: // Sizing (percentage) )

@TehTux

This comment has been minimized.

Show comment
Hide comment
@TehTux

TehTux Feb 9, 2018

Leafo\ScssPhp\Exception\ParserException: parse error: failed at $breadcrumbs-item-separator-item-rtl: '\\' !default; ../vendor/zurb/foundation/assets/../scss/components/_breadcrumbs.scss on line 52

leafo/scssphp: 0.7.5
zurb/foundation: 6.4.3

TehTux commented Feb 9, 2018

Leafo\ScssPhp\Exception\ParserException: parse error: failed at $breadcrumbs-item-separator-item-rtl: '\\' !default; ../vendor/zurb/foundation/assets/../scss/components/_breadcrumbs.scss on line 52

leafo/scssphp: 0.7.5
zurb/foundation: 6.4.3

@santiagoarizti

This comment has been minimized.

Show comment
Hide comment
@santiagoarizti

santiagoarizti Feb 9, 2018

yes, also this happened @TehTux luckily I do not need the breadcrumbs from Foundation so I was able to just comment it out from the build.

I cannot just comment lines in the scssphp library because it is being kept by composer, so
my solution in the mean time to this issue was to temporarily comment the suggested line, compile the foundation.css file, commit the compiled file to my repo, then uncomment the scssphp lines. I have many other simpler scss files that do need more frequent compilation, those have no problems with the current state of the library.

santiagoarizti commented Feb 9, 2018

yes, also this happened @TehTux luckily I do not need the breadcrumbs from Foundation so I was able to just comment it out from the build.

I cannot just comment lines in the scssphp library because it is being kept by composer, so
my solution in the mean time to this issue was to temporarily comment the suggested line, compile the foundation.css file, commit the compiled file to my repo, then uncomment the scssphp lines. I have many other simpler scss files that do need more frequent compilation, those have no problems with the current state of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment