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

Mixins as functions opposite normal lazy loading rules #1892

Closed
scottgit opened this issue Feb 21, 2014 · 12 comments
Closed

Mixins as functions opposite normal lazy loading rules #1892

scottgit opened this issue Feb 21, 2014 · 12 comments

Comments

@scottgit
Copy link

Forgive me if this relates to a previously submitted issue (I know there are a number of issues dealing with variables and mixins and order of operations).

When using a mixin as a function does the reverse of the typical lazy load for variables. For example (note this is tested with 1.6.0):

.setVar(@set) {
  @var: @set;
}

.test {
  @v1: first;
  @v1: second;
  .setVar(1);
  .setVar(2);
  without-mixin: @v1;
  with-mixin: @var;
}

Produces:

.test {
  without-mixin: second;
  with-mixin: 1;
}

So @v1 over writes to the second call as I expect lazy loading to do, but not so with the two "calls" to .setVar(), where the first call is the winner. Is this a bug or is it simply because of the way LESS processes mixin order, that it does it in reverse (and therefore, I must take that into account). I discovered this reversal behavior while working on this Stack Overflow solution where we are calling multiple times the same .setVar() type of "function."

@lukeapage
Copy link
Member

Okay, its a kind of side effect.. variables are only copied from a mixin if
they don't already exist in scope. After the first call they are in scope
:( I will have to think whether we should do anything about this behavior.

@seven-phases-max
Copy link
Member

So far the only workaround for this is to scope each "function" call (well, or use the method suggested by @scottgit in the SO answer). That's why eventually I'll re-propose #538 (when it will be the right time, I'm not ready for the moment :)

But for now this is expected behaviour. I don't think we can or should change any lazy-loading stuff for this, because this is not a scope or variables problem, it's just "you can't define a function in Less", hence:

This allows us to create a mixin that can be used almost like a function.

@scottgit
Copy link
Author

@lukeapage your explanation makes complete sense. @seven-phases-max I would argue it is a scope related issue, but agree it is probably not a "problem." In one sense, I think you can see why I expected it to behave as I did, and why it seemed confusing that it didn't. However, given Luke's explanation, it makes sense. We would not want a mixin overriding with its local variables if that variable existed already in the parent scope, so I understand why it works the way it does. Ultimately some easier way to do some self-made functions would be best. Thank you both for commenting back on this. I learned from it.

@seven-phases-max
Copy link
Member

@scottgit I think I agree that:

without-mixin: second;
with-mixin: 1;

is unexpected (probably I concentrated on "function" stuff too much to think of the issue itself deeper).

@lukeapage
Copy link
Member

Would plugins allowing js functions help or you still want a way in less?

@seven-phases-max
Copy link
Member

Would plugins allowing js functions help or you still want a way in less?

In short: no, not really. Usually those are just very tiny task/project/page-specific snippets that simply do not deserve anything but 2-5 lines of Less code. Prerequisites of knowing javascript (and how to use Less types there), knowing how to write and setup a plugin, knowing that it never will work anywhere except less.js - all that makes such "functions" still be written via hacks mentioned here.

(Not diminishing the value of the plugins ability to extend Less with a "standard" re-usable function sets of course).

@scottgit
Copy link
Author

For this particular issue, perhaps some "flag" setting on whether the variable was originally defined by a mixin or not (and which mixin) would allow that same mixin to override the variable if called again. That way, in this particular use case it does function in a normal and expected lazy loading fashion. Of course, is that use case common enough to warrant a change in the LESS code to make it more predictable.

@lukeapage
Copy link
Member

I think this should be solved by allowing functions defined in less, not by a flag changing scope behaviour.

@scottgit
Copy link
Author

@lukeapage I'm all for "allowing functions" but I'm not sure that of itself would resolve the "unexpected" (lazy loading expectation) fact that a variable defined in the scope by a mixin cannot be overridden by another call to that mixin (as the case here). That was the only reason for the "flag" suggestion, as a possible way to make the behavior more as expected.

I think the only way a function would solve the unexpected behavior is if you then no longer allowed such variable setting by mixin calls at all (expecting users to use functions instead), in which case you may have a large legacy issue to deal with for backwards compatibility.

However, if functions are on the horizon, perhaps this unexpected behavior should just be left "as is," only update the documentation for using "mixins as functions" to specifically note that a variable value can only be set once by such a call. Maybe that should be done even if functions are not coming in the near future.

@seven-phases-max
Copy link
Member

I guess just a matter of changing there:

All variables defined in a mixin are visible and can be used in caller's scope (unless the caller defines its own variable with the same name).

to

All variables defined in a mixin are visible and can be used in caller's scope (unless the caller already has a variable with the same name defined).

or something... (w/o getting into the details too deep...)

@scottgit
Copy link
Author

@seven-phases-max : That works, though I would probably choose wording that was a little more explicit. Something like this for the parenthetical note:

(unless the caller already has a variable with same name defined, including previously defined by another mixin call)

Anything to make it a bit more clear that a mixin cannot be called twice to reassign a variable value. Just my thoughts--I like explicit clarity.

seven-phases-max added a commit to less/less-docs that referenced this issue Aug 16, 2014
@seven-phases-max
Copy link
Member

Closing as "intended/expected behaviour" (Docs are updated. Feel free to reopen if you have any idea of improving this feature or so (apart from the "function feature" itself of course)).

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