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

Document special case - mixins return values #80

Open
SomMeri opened this issue Jan 11, 2014 · 8 comments
Open

Document special case - mixins return values #80

SomMeri opened this issue Jan 11, 2014 · 8 comments

Comments

@SomMeri
Copy link
Member

SomMeri commented Jan 11, 2014

Mixins returned within one call do not see each others return values. This:

@returned: global;

.mixin() {@returned: returned1;}
.mixin() {returned: @returned;}
.mixin() {@returned: returned2;}

div-1 {
  .mixin();
}

compiles into:

div-1 {
  returned: global;
}

I briefly considered to file it as an "issue", but we do not have to over-complicate things. It is good as it is.

@seven-phases-max
Copy link
Member

It makes sense.. I wonder where it is to be documented as so far we don't have any detailed "inter-scope" documentation. There're only very very basic "Overview:Scope" and "Mixins as functions". I also would love to have some more detailed scope documentation but for the moment there're a few dark corners (e.g. #1316) that make writing of such article not so easy (indeed, it's not always evident if something is a feature or an issue).

@matthew-dean
Copy link
Member

I don't see the issue? That the property in mixin 2 is not set based on the assignment in 1 or 3?

@seven-phases-max
Copy link
Member

@matthew-dean, yes, taking lazy-loading into account someone would expect (in a perfect world) the result to be returned2. (because the 2nd expansion inherites the scope of div-1 and there the value of @returned is set by 1st and 3rd expansions). But the world is not perfect so I guess that's why @SomMeri suggests to document it as a feature :)

@SomMeri
Copy link
Member Author

SomMeri commented Jan 12, 2014

@matthew-dean You are right, that was bad example. Compilation result is good, because mixin1 hits global @returned: global; before searching caller scope.

This one is better, I removed global variable definition:

.mixin1() {@returned: returned1;} //use variable
.mixin2() {mixin2: @returned;}  //declare it

div-1 {
  div: @returned;
  .mixin1();
  .mixin2();
}

compiles into:

div-1 {
  div: returned1;
  mixin2: returned1;
}

If you try the same with same name mixins on one call, less.js throws in exception:

.mixin() when (@variable = 2) {
  property: value;
}
.mixin() {
  @variable: 2;
}

#use {
  .mixin();
} 

compiles into

NameError: variable @variable is undefined in test.less on line 1, column 16:
1 .mixin() when (@variable = 2) {
2   property: value;

Edit: This is real equivalent of the same on multiple calls:

.mixinUse() when (@variable = 2) {
  property: value;
}
.mixinDefine() {
  @variable: 2;
}

#use {
  .mixinDefine();
  .mixinUse();
} 

It compiles into:

#use {
  property: value;
}

@SomMeri
Copy link
Member Author

SomMeri commented Jan 12, 2014

@seven-phases-max Pretty much yes, as a feature, quirk or wont fix :). It could go somewhere in scoping section.

What would be scary is something like this:

.mixin() when (@variable = 2) {
  property: value;
}
.mixin() when (default()) {
  @variable: 2;
}

#use {
   @variable: 1;
  .mixin();
} 

It is hard to define, would be much useful (imho) and the only gain would be theoretical purity. So, I through we can simply define it as is, in easy to define and understand version.

@seven-phases-max
Copy link
Member

@SomMeri Btw., for the last example I think it should not work even theoretically (with or without default), i.e. assuming that expansion "rule" is:

  1. evaluate all guards for the mixins of the same name
  2. expand them

and not:

  1. evaluate guard of the first mixin and expand it
  2. evaluate guard of the second mixin and expand it
  3. etc.

I.e. I would expect it to behave more like switch/case not really like if/elseif (especially because if/elseif like expansion would be spoiled by lazy-loading as in your example). Questionable as always (because of guard operators that make it to look like if/else i.e. when (a > b)), but it already works in switch/case manner anyway :).


Hmm, interesting... that points me to the fact that mixin argument list and its guard actually have their own scope which sits somewhere in between caller's scope and "normal" scope of the mixin itself. e.g.:

.mixin(@a: 2, @b: @a) when (@b > 0) { // it's already not in the caller but not yet inside the mixin
    @b: 0;
    a1: @a;
    b1: @b;
}

.caller {
    @a: 1;
    .mixin();
}

There's nothing really new but I did not think of that another scope from formal point of view before (e.g. #1316).

@SomMeri
Copy link
Member Author

SomMeri commented Jan 12, 2014

@seven-phases-max I did not through about such "expansion rule", but it makes sense. Then it could be possible to make this "perfect" and fix instead of mentioning it in docs.

I'm not saying it is worth effort, it may stay in "we will fix it once everything else is done and we are all old, retired and bored" category.

@matthew-dean
Copy link
Member

On the general topic, we should be careful documenting behavior of Less.js which is not "designed". If it wasn't specifically built a certain way but just happens to exhibit strange edge cases, then we'd not want to imply that the behavior is supported (i.e. it could change).

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

No branches or pull requests

3 participants