Variables from inner mixin override variables in parents

[newkind]

Looks like the master version of compiler stopped properly recognizing mixin multiple values for ie. Bourbon flex()

This code which is proper and was working fine before :

@include flex(X Y);

now excludes Y and only considers X.

Sample code :

@include flex(0 100%);

After compilation using master branch :

flex: 0;

Should be :

flex: 0 100%;

The only solution right now is to stop using this mixin and break it into two separate rules.

[w00fz]

This is the sassmeister gist https://gist.github.com/w00fz/6ee4744601ef81992ce03cd79398b743
You can see the output is not consistent with the SCSS compiler.

[w00fz]

After further testing, it looks like it's the nested @include causing the issue. Consider the following example (using bourbon for simplicity):

// Load Bourbon
@import "vendor/bourbon/bourbon";

#foo {
    @include flex(0 100%);
}

@include hidpi(1.5) {
    #foo {
        @include flex(0 100%);
    }
}

This is the output after compilation:

#foo {
  -webkit-box-flex: 0;
  -moz-box-flex: 0;
  box-flex: 0;
  -webkit-flex: 0 100%;
  -moz-flex: 0 100%;
  -ms-flex: 0 100%;
  flex: 0 100%;
}
@media only screen and (-webkit-min-device-pixel-ratio: 1.5), only screen and (min--moz-device-pixel-ratio: 1.5), only screen and (-o-min-device-pixel-ratio: 1.5/1), only screen and (min-resolution: 144dpi), only screen and (min-resolution: 1.5dppx) {
  #foo {
    -webkit-box-flex: 0;
    -moz-box-flex: 0;
    box-flex: 0;
    -webkit-flex: 0;
    -moz-flex: 0;
    -ms-flex: 0;
    flex: 0;
  }
}

So as you can see, the second output is not rendering as expected.

[mahagr]

Alright, I found the cause, this is minimal code to reproduce the issue without any external dependencies:

@mixin inner($value) {}

@mixin outer($value) {
  #{$value} {
    value: $value;
    @content;
    value: $value;
  }
}

@include outer('foo') {
  @include inner('break');
  @include outer('bar') {
    @include inner('break');
  }
}

Result looks like:

/* line 4, test.scss */
foo {
  value: 'foo';
  value: 'foo';
}
/* line 4, test.scss */
foo bar {
  value: 'bar';
  value: 'break';
}

Basically $value from outer() gets replaced by $value from inner(), but only in the second rule. The issue happens always when there is an outer rule with @content; and then 2 nested @includes which use the same variable name. Note that the outmost include doesn't need to have variable, I just added it to get rid of one extra mixin.

If I rename the variable name from either of the mixins, the functions start working properly.

[mahagr]

The issue seems to be that getStoreEnv() always points to the same object, which makes the inner arguments to override the original values in the outer store.

[mahagr]

@robocoder I think I found a fix for this bug. Basically what I'm doing is to change the storeEnv to the current one just after push and then restore it before pop.

            case Type::T_INCLUDE:
                // including a mixin
                list(, $name, $argValues, $content) = $child;

                $mixin = $this->get(self::$namespaces['mixin'] . $name, false);

                if (! $mixin) {
                    $this->throwError("Undefined mixin $name");
                    break;
                }

                $callingScope = $this->getStoreEnv();

                // push scope, apply args
                $this->pushEnv();
                $this->env->depth--;

                $storeEnv = $this->storeEnv;
                $this->storeEnv = $this->env;

                if (isset($content)) {
                    $content->scope = $callingScope;

                    $this->setRaw(self::$namespaces['special'] . 'content', $content, $this->env);
                }

                if (isset($mixin->args)) {
                    $this->applyArguments($mixin->args, $argValues);
                }

                $this->env->marker = 'mixin';
                $this->compileChildrenNoReturn($mixin->children, $out);

                $this->storeEnv = $storeEnv;

                $this->popEnv();
                break;

What do you think?

Do me mixin is its local scope with its own variables which are specific to mixin itself. I think this will fix a lot of issues with nested mixins as its common to use the same parameters and variable names in them.

[mahagr]

OK, the fix seems to break something else, some variables are now missing...

[mahagr]

Ok, same is needed for function calls:

    protected function callScssFunction($name, $argValues, &$returnValue)
    {
        $func = $this->get(self::$namespaces['function'] . $name, false);

        if (! $func) {
            return false;
        }

        $this->pushEnv();

        $storeEnv = $this->storeEnv;
        $this->storeEnv = $this->env;

        // set the args
        if (isset($func->args)) {
            $this->applyArguments($func->args, $argValues);
        }

        // throw away lines and children
        $tmp = new OutputBlock;
        $tmp->lines    = [];
        $tmp->children = [];

        $this->env->marker = 'function';

        $ret = $this->compileChildren($func->children, $tmp);

        $this->storeEnv = $storeEnv;

        $this->popEnv();

        $returnValue = ! isset($ret) ? self::$defaultValue : $ret;

        return true;
    }

[mahagr]

Here is test case for the function calls:

@function contains($list, $values...) {
  @each $value in $values {
    @if type-of(index($list, $value)) != "number" {
      @return false;
    }
  }

  @return true;
}

@function mapping($items) {
  $src: ();
  $map: (
          one:   1px 1px,
          two:   2px 2px,
  );

  @each $key, $values in $map {
    @if contains($items, $key) {
      $value1: nth($values, 1);
      $value2: nth($values, 2);

      $src: append($src, $value1 $value2, space);
    }
  }

  @return $src;
}

@mixin test() {
  padding: mapping(one two);
}

foo {
  margin: mapping(one two);
  @include test();
}

[robocoder]

Looks promising. I look forward to seeing your pull request.

[mahagr]

Fixes also #370

[mahagr]

Looks the same as #348

[robocoder]

@mahagr : 👍