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

[matthew-dean]

This was mentioned here by @lukeapage: #637 (comment), 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.