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

If you call a mixin using .a > .b it can't see variables in its own scope and not its parents. #1882

Closed
lukeapage opened this issue Feb 17, 2014 · 3 comments

Comments

@lukeapage
Copy link
Member

Probably a duplicate but may be worth moving to this issue since I investigated and I expect this has alot more detail. These are all called with issues in the way the code stores scope around mixins/rulesets.

The following errors because ruleset grabs frames for mixin definitions before any further calls. But if you use a call into a ruleset, that will not have evaluated, so the frames property will not be set and it cannot find the variable.

.a {
  .b > .c();
}
.b {
  @e: true;
  .c() {
    e: @e;
  }
}

To fix this we would have to make the evaling of mixin definitions (and detached rulesets to be consistent though they don't need this) happen across the whole ast before evaluating mixin calls etc.

This would have to be a delicate change and one possibly not worth making (we would have to seperate out creating initial frames / assigning frames to definitions and calling mixins / evaluating nodes etc.)

If we were going to fix this it might be worth fixing the fact that normal rulesets (mixins not defined with ()) do not store frames at all - hence the following bug

.b {
  @a: var;
  .c {
    @a: green;
    .a();
  }
  .a {
    color: @a;
  }
}

which doesn't occur if you make a a mixin definition

.b {
  @a: var;
  .c {
    @a: green;
    .a();
  }
  .a() {
    color: @a;
  }
}
@SomMeri
Copy link
Member

SomMeri commented Feb 17, 2014

I think that this #996 is the duplicate . It seems like exactly the same problem.

@seven-phases-max
Copy link
Member

And I guess it is also linked to #1316 (at least to consider that when thinking of the fixing way, it looks like it's all the same thing internally after all).

@seven-phases-max
Copy link
Member

Merging to #996 (also a duplicate of #921 which I'll also merge to #996) - it's all the same.

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

No branches or pull requests

3 participants