Discussion: Variable visibility of mixin call/definition scope and caller scope to each other

[matthew-dean]

This was mentioned here by @lukeapage: https://github.com/less/less.js/issues/637#issuecomment-13598784, but was not the point of the main thread, so I'm assuming this is still an issue. (If not, close.)

It would be a cleaner architecture (and more consistent, and less error-prone) to only have variable inheritance one-way. Outer scope -> inner scope. Caller scope -> mixin scope.

This would potentially be a breaking change (3.x).

[seven-phases-max]

Any particular examples? Currently (starting from v1.4.2) I can't recall any leaking issues. (The only known var scope issue is not really solvable conflict between orthogonal parent and caller scopes - e.g #2435 and related stuff like #1306). The mentioned comment was about earlier 1.4.x version problems fixed in 1.4.2 (+/-).

[rjgotten]

This would potentially be a breaking change (3.x).

Yes, it would. It would break return values from mixins. I'd love to see the counter-intuitive variable bubbling to be removed, as I've suggested before. But you can't really do so without providing a suitable built-in syntax replacement to facilitate return values.

[seven-phases-max]

Just a brief remark... (only to ensure you both are talking of the same thing).

@rjgotten

the counter-intuitive variable bubbling...

Subjective - it's just because you think of a mixin like a function. But it's not - the very purpose of the mixin feature (in Less) is to expand the contents of some block scope into another block scope (thus effectively merging both). Obviously this does not automatically mean it should expand everything, but notice a mixin call must expand at least its CSS properties to the calling scope - now considering #2654 - disabling variables would not make any difference (just because as soon as #2654 merged, properties become "variables" too, and we simply can't disable these "variables" to leak into the calling scope).

In other words, speaking of counter/pro-intuitiveness, try to think of mixins like of expandable namespaces, then a mixin call becomes just something similar to "using namespace X" and voila.

---

Regardless of above - Personally I would not mind disabling variable expanding into the caller scope (if Less has real functions and "using namespace" (typical use-case) facilities), but I believe there must be some reason to do this beyond subjective "feelings". Notice in every issue (after 1.4.2) this idea pops up - the problems have very few to do with "leaking" itself (#2436, #2543 etc.)

So yet again, particular examples are welcome (example where leaking harms and example where disabling it would actually solve something).

[SomMeri]

@seven-phases-max Mixins do not really fully work as "merging content in". Calling mixin and writing the code directly can lead to different results (like here).

Those who did not grep up on JavaScript are used to think about blocky syntax as about closed non-leaking scope. Especially when value returned by mixin overrides something deep down. Everything is copied into caller rule is unusual. That being said, I do not think we can just disable return values. We can only make it disabled by default, e.g. add some new syntax would allow mixins to return values and assign them to callers variables.

---

However, what I would be keen on removing is the "mixins sees callers variables" thing. There is no reason for that since mixins have parameters. I do not like it much because:

• It is not explicit, the mixin does not work unless variable is defined and does not says so in its definition. The usage may be hidden behind system of guards, so the fact that mixin requires something else in caller is non-straightforward.
• It prevents compiler optimizations - you can not easily pre-compile mixin when it is defined and use that wherever it is called. It is impossible (np-complete I think) to determine which callers variables and mixins are relevant.

It is ok to see outer scope variable and return values are something that is too useful. Mixin sees callers variables is something that should be deprecated.

[matthew-dean]

Either way has its merits. I'm thinking about it in terms of simplifying evaluation and re-evaluation by establishing clear dependency chains, similar to @SomMeri's second point. If both are true: mixins can see caller's vars and caller can see mixins vars, then you have a circular dependency.

I know it's not the only case where things currently need to be evaluated more than once. I've never used "return vars" so I don't know the use cases there. However, I have extensively used inheritance of vars into mixins. There are Less patterns that are impossible or at least difficult or much more verbose without that inheritance. (I actually have a Less lib I'm working on that pretty much entirely relies on this feature.)

So, it may be that we can't really change this behavior without some sort of feature expansion on one side or another.

[rjgotten]

Mixin sees callers variables is something that should be deprecated.

This I will agree with 100%. It makes no sense to expose caller scope like that and it already has a suitable, predictible alternative available in the form of argument/parameter passing.

[matthew-dean]

@rjgotten Yes, it has an alternative, but it's only suitable under simple circumstances. In some cases, a mixin may expand a tree of other mixins, and some of those mixins can rely on an expansion of vars at the top of the tree. You could pass said set of vars into the mixin, but what if you're dealing with 100 variables? 100 arguments into every mixin? You could call the mixin within each individual mixin that expands the custom variables, but that's all of the sudden making a bunch of copy/paste statements.

Whether or not that made sense, making that change would basically kill my project. It relies on custom variables being propagated within specific scopes without explicit calls by a user or plugin author. There is no alternative to that, because it's explicitly relying on variable inheritance into mixin scope.

Simply put, variable inheritance allows a much more DRY approach when there's multiple (sometimes dozens of) mixins inheriting a parent's scope. Less currently doesn't allow extending mixins, so there is no way to do simple inheritance other than constructing parent scopes.

[seven-phases-max]

So, it may be that we can't really change this behavior without some sort of feature expansion on one side or another.

That's what is I'm talking about. We can't remove a feature "just because I don't like it" w/o offering something in exchange to use to implement stuff "a bad feature" is usually used for.

[rjgotten]

In some cases, a mixin may expand a tree of other mixins, and some of those mixins can rely on an expansion of vars at the top of the tree.

But is that a reliance on caller scope, or a reliance on declaration scope? I.e. is it caller scope:

& {
  @foo : "foo";
  .bar();
  .baz();
}

.bar() {
  .baz() {
   value : @foo;
  }
}

or declaration scope:

& {
  .bar("foo");
  .baz();
}

.bar(@foo) {
  .baz() {
   value : @foo;
  }
}

I cannot really see how the first would ever be a workable, maintainable code pattern.

[matthew-dean]

I haven't posted code examples to this point because my involvement with Less is not to advertise for myself, but it looks like real examples are needed.

Granola has, in fact, the exact pattern of caller scope given in your example. As in:

& {
        // Import vars into an isolated scope (don't leak)
    #granola();
    #vars();
    #granola._make(@defaults, @all, @type, @layout, @forms, @nav, @buttons, @icons, @tables, @panels);
 }

( https://github.com/Crunch/Granola/blob/master/lib/granola.less#L17 )

By calling the vars mixin, and then calling the entire library "render" sequence, it allows reference of these vars through the library. And by calling a vars mixin within the scope, it allows the variables to be simply named, yet to not override any local vars an author might have. So it provides code isolation.

On the library end, the Less code uses a lot of tricks, but for the intended end user, it makes their job extremely easy and yes, maintainable.

// styles.less

@import "granola";
// Change custom vars
#granola {
  .vars() {
    @borders__scale: 0.5;
  }
}
// This is what actually renders CSS
#granola.make();

IMO variable isolation and namespacing is a better pattern than something like Bootstrap's completely flat global variable pattern. And, as an example of one mixin:

#granola {
    // Mixins to help with UI rendering
    .ui {
        // Draw a border based on math!
        .border(@width, @style, @color) {
            border: ceil(@width*@borders__scale+(@borders__base - 1)) @style @color;
        }
    }
...
}

There are utility classes which do not demand that the end user pass in every variable that might be required, since some are essentially constants within the library. You could, of course, call #granola.vars() within every single mixin that might need those constants. In fact, an early version had that pattern, but that pattern was completely unmaintainable. So then I began using a simple scoping pattern, and that made my life easier and my code cleaner.

I call this pattern an "OOLESS" pattern (similar to the OOCSS pattern). However, to my knowledge, this is the only OOLESS library that exists. But that doesn't mean that this pattern isn't being used elsewhere, or that using it makes something unmaintainable or unworkable. In my case, this turned out to be the best solution.

[seven-phases-max]

@matthew-dean

I've never used "return vars"

_{Actually this}

#granola {
  .vars() {
    @borders__scale: 0.5;
  }
}

_{is what "return vars" is. Above I named this feature using namespace (C++) thing, but one more familiar with other langs could probably name it a withstatement. And http://lesscss.org/features/#mixins-as-functions-feature is nothing but just a reduced/edge-case of the feature (where instead of multiple variables we "expand/expose/unlock" (named "return" since we simulate a function) only one).}

[matthew-dean]

Oh, good point. In that case, it turns out I'm using both features. Lol oops. So, yes, that's true, I'm expanding vars into a scope (via a mixin), and then inheriting those in both mixin calls and mixin declarations.

Which doesn't mean this isn't a problem. But for me, there really isn't a better solution. At least in Less right now. The ability to do both is what makes this pattern really powerful.

[SomMeri]

@matthew-dean Theoretically, your use case would not require "the mixin sees caller scope" if we returned values from detached rulesets too. E.g.:

    @allParametersDetachedRuleset: {
      @granola();
      @vars();
      //own modifications
    };
    #granola._make(@allParametersDetachedRuleset);

Incidentally, since we are returning mixins from detached rulesets and both mixins and variables from mixins, returning variables from detached rulesets would make sense.

[SomMeri]

Found workaround to "detached rulesets don't return values" - just nest one more mixin into it:

#use-place {
    //here we define all variables needed by _make
    @newDetachedRuleset: {
      //we need nested mixin, because detached ruleset
      //does not return variables
      .workaround() {
        //default values
        #granola();
        #vars();
        //local modifications
        @override: overriden;
      }
    };
    //pass all variables to _make mixin
    //it assumes variables are defined in @newDetachedRuleset
    #granola._make(@newDetachedRuleset);
}

//granola library
#granola() {
  ._make(@parametersPack) {
    //return all needed variables
    @parametersPack();
    .workaround();
    //use variables
    making: made;
    configuration: @configuration;
    override: @override;
  }
}
//default configuration of granola library
#vars() {
  @configuration: configured;
  @override: default;
}

compiles into:

#use-place {
  making: made;
  configuration: configured;
  override: overriden;
}

Edited to add: if we want to avoid the hard to maintain thing, it is possible to send the mixin to #granola library instead of into _make:

#use-place {
    //here we define all variables needed by _make
    @newDetachedRuleset: {
      //we need nested mixin, because detached ruleset
      //does not return variables
      .workaround() {
        //default values
        #vars();
        //local modifications
        @override: overriden;
      }
    };
    //pass all variables to granola library
    //it assumes variables are defined in @newDetachedRuleset
    #granola(@newDetachedRuleset);
    //use ._make from granola
    ._make();
}

//granola library
#granola(@parametersPack) {
  //return all needed variables
  @parametersPack();
  .workaround();
  ._make() {
    //use variables
    making: made;
    configuration: @configuration;
    override: @override;
  }
}
//default configuration of granola library
#vars() {
  @configuration: configured;
  @override: default;
}

@lukeapage The less-preview thing is simply awesome. Ability to link live preview of code is really neat addition.

[matthew-dean]

@SomMeri Your first example wouldn't work because the .vars() mixin is defined multiple times. By having vars defined within the library, and allowing the user to define vars, then a call to #granola.vars() expands multiple mixins into the current scope.

What that does is effectively emulate namespaced variables (#1848), which IMO is ready to implement. If that was done, then I would be doing things like:

#granola {
    .ui {
        .border(@width, @style, @color) {
            border: ceil(@width*#granola.borders@scale}+(#granola.borders@base - 1)) @style @color;
        }
    }
}

So, I'm not sure return vars from detached rulesets would help me, since I'd always have to pass the detached ruleset into every mixin all the way down the line. However, if I could directly pierce a namespace and reference a variable, then I wouldn't need to expose variables in a parent scope. Essentially, I'm importing a scope into the current scope rather than referencing that scope directly (since the latter is not yet possible).

[SomMeri]

@matthew-dean You can unpack it in library root. Every mixin defined inside the library will see that scope and variables. Like this:

//granola library
#granola(@parametersPack) {
  //return all needed variables
  @parametersPack();
  .workaround();
  //library mixins
  ._make() {
    //use variables
    making: made;
    configuration: @configuration;
    override: @override;
  }

  //default configuration of granola library
  #vars() {
    @configuration: configured;
    @override: default;
  }
}

._make() will search its own scope for variables first and will see everything unpacked by .parametersPack workaround combo.

I do not see which the mixin is added twice?

[rjgotten]

You can unpack it in library root. Every mixin defined inside the library will see that scope and variables

Ooh! Now that is clever. That actually solves another issue I had with a few library components. Thanks for that idea.

[seven-phases-max]

Incidentally, since we are returning mixins from detached rulesets and both mixins and variables from mixins, returning variables from detached rulesets would make sense.

Yes (#2004). I did not mentioned this issue because it would be weird to say so in a topic that propose exactly the opposite :). (Personally I treat variables and mixins (actually same for properties and rulesets) as just different forms of "an abstract object", thus their scope visibility should go as similar as possible (though not always identical in details, specifically because "variables are overridable" but "mixins are cascadable"). Thus, ideally, things should never go as "variables are visible but mixins of the same level are not" or up-side-down. And in this sense current DR expansion is totally inconsistent, especially counting that a DR is nothing but a unnamed ruleset assigned to a variable, therefore a DR call is (supposed to be) nothing but an ordinal "non-parametric mixin" call. Obviously this should apply to both "caller variables" and "callee variable" problems - which brings me back to the roots.)

[matthew-dean]

@SomMeri Isn't this what I'm already doing? I'm basically expanding variables within each #granola() {} block so that they can be treated as a unified namespace. Ohhhhh I think I see what you're doing. Since you can't pass a mixin as a variable, you're passing around a DR which contains all the needed mixins w/ respective variables to expand within that scope. You expand the DR, then expand the mixin. I'll play with that, thanks.

therefore a DR call is (supposed to be) nothing but an ordinal "non-parametric mixin" call

Except that in the documentation it explicitly says that a DR has private vars. But that may be more a description than a design strategy? You'd have to ask @lukeapage on that one. I think he designed and implemented it.

But, yes, the syntax is so similar to mixins that I'd prefer if "detached rulesets" were actually "variable mixins", as in, mixins assigned to a variable. Like:

.i-am-a-mixin(@var) when (@var = true) {
  stuff: yo;
}
@i-am-a-variable-mixin(@var) when (@var = true) {
  stuff: yo;
}

To me, this syntax of DRs has always felt wrong:

@i-am-a-variable-mixin: {
  stuff: yo;
};

I get that it makes sense if you're just considering Less variables, but it's completely inconsistent with CSS block definitions, which require neither a colon to assign to a keyword, nor termination of a block with a semi-colon.

I don't think discussing DRs are completely off-topic, since it sounds like we're saying that we can't change behavior of mixin scope without filling in the gaps with something, which may include DR functionality. In fact, I'll tweak this issue heading a bit.

[seven-phases-max]

Except that in the documentation it explicitly says that a DR has private vars.

Yes, the documentation was written to reflect the current state which never was considered finished (as was remarked in every DR-related PR). I would probably complain (as well as I would be against using the internal codename "DR" in the public docs) - but some documentation is better then nothing. (So if it's about docs I'd rather added "subject to change" note there. Notice the docs say nothing at all about nested mixins exposed to the caller scope by calling their outer mixins - but this does not stops us to use this feature in every example above :) So the docs are important but can't be used as "something written in stone" - specifically when it comes to relatively new feature like DR. Or in other words: "a documented defect remains to be a defect").

[seven-phases-max]

But, yes, the syntax is so similar to mixins that I'd prefer if "detached rulesets" were actually "variable mixins", as in, mixins assigned to a variable.

Btw., don't forget DRs were implemented specifically to target "passing rulesets to mixins" problem - that is, their main and the primary use is:

.mixin({ /* ta-da */ });

The variable assignment syntax is just naturally raises from above, since mixins parameters are represented as variables, it automatically turns into:

.mixin(@a, @b) {}

.mixin(1, {/* 2 */}); // -> @a: 1; @b: {/* 2 */};

Nothing more than that.

[matthew-dean]

The variable assignment syntax is just naturally raises from above, since mixins parameters are represented as variables

I recognize that, and yet their declaration is still at odds with other parts of Less/CSS. At the least, I would change it so that the final semi-colon is optional. It both makes logical sense and yet looks completely weird next to other blocks. It's not like we don't know when a detached ruleset has ended. The semi-colon is redundant.

Beyond that bit, even though, yes, they were intended for passing to mixins, they do turn out to have other uses, and they do share some commonalities with other Less features that at least raises the question of how those features could better align.

For instance, one of the ways I've used detached rulesets is where they are acting effectively as a mixin, but I want that mixin to have the ability to be completely overwritten. Rather than using some intricate system of guards and vars, an end user can set the DR to a new value.

Of course, this is a hack. What I'm trying to do is allow mixins to be either replaced or appended to, and hacking DRs is the only way I've found to solve this.

So the docs are important but can't be used as "something written in stone" - specifically when it comes to relatively new feature like DR

Yes, this is true. Ironically, I've been in discussions where someone said, "But the docs say..." and I've said, "Okay, well the docs are wrong then. That's not what was intended." I think the only cautionary piece then is that if we are implementing a feature, we either write the docs for that feature, or at the least, review it as soon as possible.

I think you said this best:

So yet again, particular examples are welcome (example where leaking harms and example where disabling it would actually solve something).

So, far, we've individually only been able to provide examples of why mixins need to have access to caller scope and why the caller needs access to the vars from the mixins. But none of us have been able to demonstrate, to @seven-phases-max's point, why limiting the scope might fix any particular problem, except to say it might make some optimizations in the compiler theoretically easier, which doesn't seem like that much of a pressing concern.

While this is a great discussion, I wonder if we shouldn't just close this at this point, since there doesn't seem anything actionable that isn't tracked somewhere else.

Agree?

[SomMeri]

@matthew-dean I still do not think mixins needs to see callers scope :)

---

One issue with scoping is that it has subtle bugs that are very hard to fix (some of #2038, #2093, #996, #1316 from those opened by me - there are others). It is buggy on edges, once you deal with details and there is no easy way to fix it all. I have fixed one or two of those issues in less4j, but it required datastructures I would not dare to implement in javascript.

As of now, it is pretty much impossible to think of all possible cases that should be tested when adding a new feature, because there is too much variants and subtlety around. So basically, fragility.

On optimizations: I had to give up on some in less4j (which someone used on unexpected large huge sheets), because it would be too much work compared to if scoping would be simpler. As of now, everything sees everywhere. Of course that is less4j problem and somewhat edge problem.

---

This ties to variables in import statements and similar features too - we can sort of half make them, but it is very hard to define them consistently, implement in mostly bug free way and reasonable estimate is that they will never be fully fixed. We basically do the part that is easy to implement and the rest is left as open issue forever and the reason is that the wall to fully-works is too big to climb.

---

I tend to think about detached rulesests as sort of lambdas - pieces of code defined at will and passed around randomly. I agree that it should work as similarly to mixins as possible.

---

Imo, fragility, how easy it is to learn it and how many special cases to think about influence a lot whether people want to continue using something a lot or not. Then again, backward compatibility is important too and from this discussion maybe this is not the right place where to cut. And yet again, it seem to me that we are not able to define all the feature we want in simultaneously right now.

[matthew-dean]

@SomMeri Okay, that helps. Then there is some evidence that current scoping rules cause current problems.

I can think of a few things that would likely solve the need to see caller's scope for me:

1. Referencing vars by namespace (as mentioned).
2. Exporting vars from DRs when evaluated.
3. Some new kind of "nested" variable structure? Or maybe the ability to overwrite a DR's internal variable? If DRs were more like mixins, then we could imagine namespaced references for DRs as well.
4. The ability to "alias" a namespace to a variable e.g.:

@ns: {{#namespace.vars.borders}}  // Pretend syntax
color: [@ns]@color;  // more pretend syntax

I think all of this relates to a general issue where Less is designed with a certain "flatness" that doesn't scale well into larger projects. It has the concept of scope, but when called or defined, that scope tends to be further flattened. Or as @SomMeri puts it: "everything sees everywhere".

A lot of these problems are systemic and architectural, which is why I put up https://github.com/less/less-meta/issues/10. I don't think they are huge in terms of workload, but IMO I think we need to rethink Less from the bottom up. That is: given Less's current feature-set, if you were design those features today, from scratch, what would it look like? Is Less's feature-set appropriate for the web in 2016?

It reminds me of the old adage: "If you find yourself headed in the wrong direction, stop going that way."

[SomMeri]

@matthew-dean Is there a reason not to have DR behaving the same way as mixins? It seem to me that it could be optimal for both users (easier to learn and simultaneously more powerful) and code base (single code path for both things).

"A lot of these problems are systemic and architectural, which is why I put up less/less-meta#10. I don't think they are huge in terms of workload, but IMO I think we need to rethink Less from the bottom up."

I agree with that. Roadmap Proposal is a good thing, I think that we need to create a vision of where we want to go and decide which features/changes matter the most.

[matthew-dean]

Is there a reason not to have DR behaving the same way as mixins?

I don't think so? Not sure if you were saying I said something like that. If they do behave more like mixins, though, then the syntax shouldn't be as different as it is. We could even say that a detached ruleset is an anonymous mixin assigned to a variable. I think thinking of it as a "lambda" is correct.

If you can write:

.selector {
  & when (@a=1) {
    put: this;
  }
}

... then you should be able to write:

@my-lambda: when (@a = 1) {
  put: this;
}
.selector {
  @my-lambda();
}

Of course, a fascinating thing about that example is that & when isolates the local variable scope when we enter that block. Even though & is a "merge" of that current selector, we regularly use this as a tool for scope isolation.

Don't you think it would be easier for people to think about these blocks in the same way? That is:

& { }   // anonymous mixin (DR) immediately evaluated, like ```(function() { })();``` in JavaScript
@lam: { }  // anonymous mixin assigned to a variable
.mixin { }   // named mixin

Since the beginning of Less, selectors have been mixins. Less didn't try to differentiate the two (except that certain selector types were excluded). So it makes sense to say that & { } is a mixin. Some second thoughts about this at the bottom.

Why not just make everything a mixin, and align the behavior? Rather than splitting the behavior depending on how the block is defined? Currently those 3 types all behave differently. We already added guards to & { } and it's probably only a matter of time before we add passing parameters to DRs. Let's stop confusing ourselves by thinking of these as all different kinds of things. Less doesn't need to be that complicated.

---

Was thinking about it more, and & { } is a tricky one. It really only works in the context of another selector, so calling it a mixin might not be useful, and perhaps there should be a different syntax if you want to define an anonymous mixin and call it right away. Aligning the latter two is much more intuitive, but then again.... maybe it still is a kind of mixin? Not sure.

[matthew-dean]

Further thoughts:

If we align DRs and mixins, something like this should be valid:

@my-lambda: (@a, @b) when (@b = 1) {
  do: stuff;
}

However, anonymous mixins couldn't be "overloaded" like named mixins for obvious reasons:

// This makes no sense. The second variable declaration replaces the first, and doesn't overload it
// And "default()" should be invalid for anonymous mixins
@my-lambda: (@a, @b) when (@b = 1) {
  do: stuff;
}
@my-lambda: (@a, @b) when (default()) {
  do: stuff;
}

Buuuut... a variable could be overloaded like:

.mixin (@a, @b) when (@b = 1) {
  do: stuff;
}
.mixin (@a, @b) when (default()) {
  do: stuff;
}
@my-lambda: [.mixin];    // pretend mixin reference syntax

[seven-phases-max]

& { } is a tricky one.

Yes, here we've discussed why & {} should not be treated as an "immediately evaluated mixin". That's quite weird (in context of syntax bloating) but it's too late for & to be such (technically it could be anything else, e.g. {...}(); or so :)

@my-lambda: (@a, @b) when (@b = 1) {...

2543

@my-lambda: [.mixin]; // pretend syntax

Wait, oh ... (further discussed after https://github.com/less/less.js/pull/1648#issuecomment-33855917)

---

I mean you again forget the DR (in its current state) is nothing but a quick and dirty and thus limited/preliminary implementation of just one part of all those ideas :) So let me to rephrase the question: anyone to object if "ruleset expansion via variable (DR)" will have the same scope visibility behaviour as "ruleset expansion via selector (mixin)" has (or vice-versa)?

[matthew-dean]

If your final question is essentially: should mixins and DRs align in behavior? I think we're all saying yes. Yes?

[SomMeri]

@matthew-dean Yes, that was the question and I agree with yes too.

[matthew-dean]

@seven-phases-max (or @SomMeri) Would you be willing to write-up to write up a proposal in less-meta that includes examples for DRs and mixins and the changes in their scoping behavior? That would probably need to include comparisons to current behavior to illustrate the differences. As well, it would be good to demonstrate how someone would replicate any deprecated behavior using some other means.

[SomMeri]

@matthew-dean I will think about it, but wont be able to produce anything big for next two months or so.

[matthew-dean]

Closing in favor of discussion in https://github.com/less/less-meta/issues/16