3.5.1 introduces a breaking change

[kdzwinel]

Our build server just downloaded latest version of gulp-sass (2.3.0) which in turn downloaded node-sass 3.5.1. This caused a previously unseen critical error in the code that wasn't touched since January:

Message:

 21-Apr-2016 08:54:12   node_modules/style-guide/src/sass/_mixins.scss
 21-Apr-2016 08:54:12   Error: Mixins may not be defined within control directives or other mixins.
 21-Apr-2016 08:54:12           on line 90 of node_modules/style-guide/src/sass/_mixins.scss
 21-Apr-2016 08:54:12   >>   @mixin mintBreakpoint($name) {
 21-Apr-2016 08:54:12      --^

Relevant code:

https://github.com/brainly/style-guide/blob/42136dd914ea0626cc76f7d1a39ee37bf836c376/src/sass/_mixins.scss#L88

Last successful build was using version node-sass@3.4.2.

[xzyfer]

This project is just an implementation of the Sass language. It aims to be
100% compatible with Sass.

This is happening because your Sass is not valid. As the message says
mixins and functions cannot be defined in control structures like if
statements.

This previously worked due to a bug in LibSass which was fixed in latest
version.
On 21 Apr 2016 7:46 PM, "Konrad Dzwinel" notifications@github.com wrote:

Our build server just downloaded latest version of gulp-sass (2.3.0) which
in turn downloaded node-sass 3.5.1. This caused a previously unseen
critical error in the code that wasn't touched since January:

Message:

21-Apr-2016 08:54:12 node_modules/style-guide/src/sass/_mixins.scss
21-Apr-2016 08:54:12 Error: Mixins may not be defined within control directives or other mixins.
21-Apr-2016 08:54:12 on line 90 of node_modules/style-guide/src/sass/_mixins.scss
21-Apr-2016 08:54:12 >> @mixin mintBreakpoint($name) {
21-Apr-2016 08:54:12 --^

Relevant code:

https://github.com/brainly/style-guide/blob/42136dd914ea0626cc76f7d1a39ee37bf836c376/src/sass/_mixins.scss#L88

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#1459

[slarti-b]

I just had the same. In my case it's grunt-sass which downloaded the dependency. But suddenly my existing tags don't build anymore!

I have a mixin defined inside a @if not mixin-exists block which is apparently not allowed anymore. Also variables defined inside @if not variable-exists now seem to be only valid within that block unless declared global.

It may be that these have been seen as bug fixes ("we weren't interpreting the sass spec properly, now we are" but they should still be flagged as breaking changes - by making the interpretation much stricter you are breaking stuff which worked. And should therefore have been a version which did not come as "compatible" according to npm.

[kdzwinel]

@xzyfer thanks for explanation! I agree with @slarti-b though that it's a node-sass versioning issue.

[slarti-b]

@xzyfer : yeah, I guessed that. But this much stricter is still a breaking change - even if a "breaking bug-fix"

[xzyfer]

To be clear this was never allowed but it mistakenly worked. This is no different to depending on an undocumented API. For that reason we do not consider these breaking changes.

If valid Sass started breaking we would consider that a breaking change. We don't claim responsibility for bugs in the Sass code being compiled.

[kdzwinel]

That makes sense @xzyfer . Thank you for a quick response.

[igoradamenko]

Same issue here, but OK, I understand that this is a bug and it worked mistakenly. So is it possible to get a “legal” conditional import (in my case @import inside of @if does not work)?

[xzyfer]

Conditional imports are not possible in Sass.
On 21 Apr 2016 8:25 PM, "Igor Adamenko" notifications@github.com wrote:

Same issue here, but OK, I understand that this is a bug and it worked
mistakenly. So is it possible to get a “legal” conditional import (in my
case @import inside of @if does not work)?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1459 (comment)

[igoradamenko]

So sad. OK, thank you @xzyfer. I will search a workaround.