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

Ruleset variables aren't overridden within the same scope #2212

Closed
matthew-dean opened this issue Sep 29, 2014 · 13 comments
Closed

Ruleset variables aren't overridden within the same scope #2212

matthew-dean opened this issue Sep 29, 2014 · 13 comments

Comments

@matthew-dean
Copy link
Member

I'm still trying to find a way to "extend" rulesets. I thought I had a way, but then ran into this issue: I can't both use a mixin to call the ruleset AND allow overriding the ruleset variable. Here's a simplified test case:

#library {
    .rule-scope() {
        @rules: {
            display: block;
            color: red;
        };
        .rules() { @rules(); } 
    }
}
#library {
    .rule-scope() {
        // Override the variable
        @rules: {
            display: inline;
        };
        // Extend the ruleset
        .rules() {
            height: 50px;
        }
    }
}
.output1 {
    #library > .rule-scope();
    @rules();
}
.output2 {
    #library > .rule-scope();
    .rules();
}

Expected output:

.output1 {
  display: inline;
}
.output2 {
  display: inline;
  height: 50px;
}

Actual output:

.output1 {
  display: inline;
}
.output2 {
  display: block;
  color: red;
  height: 50px;
}

Can someone explain the discrepancy? Is this a known bug or expected scoping behavior?

@matthew-dean
Copy link
Member Author

May be related to #1316?

@matthew-dean
Copy link
Member Author

UPDATE: I found I could get the desired output with a slight modification of the example. But, I can't pretend to understand why it works this way, and I'm hoping there's a different / better solution.

Here's the modified source:

#library {
    .rule-scope() {
        @rules: {
            display: block;
            color: red;
        };
        .rules() { #library > .rule-scope(); @rules(); } 
    }
}
#library {
    .rule-scope() {
        // Override the variable
        @rules: {
            display: inline;
        };
        // Extend the ruleset
        .rules() {
            height: 50px;
        }
    }
}
.output1 {
    #library > .rule-scope();
    @rules();
}
.output2 {
    #library > .rule-scope();
    .rules();
}

Outputs:

.output1 {
  display: inline;
}
.output2 {
  display: inline;
  height: 50px;
}

@seven-phases-max
Copy link
Member

Yes, this is related to #1316, though not really the same. Basically in this example the parent scope always has higher precedence than caller scope (and if I'm not mistaken this behaviour somewhat goes after namespaces variables handling where it's intentional for the scope where you expand some #ns > .mixin to not occasionally override variables defined in #ns (even if it's parametric)). I.e. the simplified example would be:

.mixin() {
    @var: red;
    .sub() {var-in-sub: @var}
}

.mixin() {
    @var: blue;
}

.output {
    .mixin();
    var-in-mixin: @var;
    .sub();
}

resuting in:

.output {
  var-in-mixin: blue;
  var-in-sub: red;
}

.sub looks in its parent scope first and only then in the caller scope (even if it's own visibility is the result of already expanded parent). I.e. this can be yet more simplified to just:

.parent() {
    @var: red;
    .sub() {var-in-sub: @var}
}

.output {
    @var: green;
    .parent();
    var-in-output: @var;
    .sub();
}

resulting in:

.output {
  var-in-output: green;
  var-in-sub: red;
}

which already looks like a Lazy Loading violation (though I'm not sure if it's actually a bug or not. - need to see what's in the sources: if this is really something required for #ns > .mixin stuff then I guess we'd probably prefer to leave it as is to not introduce specific scope handling for various ns/nested/mixin/call combos as those are infinite).

@seven-phases-max
Copy link
Member

P.S. and .rules() { #library > .rule-scope(); @rules(); } works simply because overridden @rules is expanded directly into local scope which of course always overrides parent scope.

@SomMeri
Copy link
Member

SomMeri commented Sep 30, 2014

@seven-phases-max I do not think your latest example is a bug. Variables values are usually taken from declaration scope. Caller scope is searched only if the declaration scope does not contain the value.

Lazy loading (as I understand it) means that the expression will be evaluated at the latest possible occasion, so the following will compile without errors:

.parent() {
    .sub() {var-in-sub: @var} // var is undefined in caller
}

.output {
    @var: green;
    .parent();
    var-in-output: @var; // lazy loading kick in - uses local value
    .sub();
}

results in:

.output {
  var-in-output: green;
  var-in-sub: green;
}

IMO, if we would fix this, using mixins would become too error prone. You would have to be super careful about local variable names. You would have to remember "internal" variables used define third party mixins just so you name your own differently and do not override them.

@matthew-dean
Copy link
Member Author

Interesting. As I played with it more, it started to seem like the mixin behavior was consistent, but it just felt disjointed operating next to detached rulesets. Detached rulesets look very much like variably-assigned mixins in their syntax, but their behavior is quite different from mixins and other variables. It's fine if it's by design, but hopefully you can see why side-by-side, it can get confusing fast.

Basically, I was trying a variety of things to do one thing, which was extending a ruleset in some fashion, and I ran into some unexpected behaviors.

@seven-phases-max
Copy link
Member

@SomMeri

I do not think your latest example is a bug.

I'm somewhat agreed. Which means then that all results above are not a bug too (since all these examples are equal in scope handling). Getting back to the original @matthew-dean example, probably what was confusing there is that one may think of two #library definitions as a single namespace where later body internals override earlier bodies. But in fact they are still different namespaces just using the same name and thus do not share their internals (so only things directly expanded with #library > .rule-scope(); are what is merged in the caller and thus overridable. Hence @rules is changed but .rules() is not).


In that sense I guess to get the desired result it's possible to rewrite the original example to something like this:

#library {
    .rule-scope() {
        .rules() {@rules();}
    }
}

#library {
    .rule-scope() {
        @rules: {
            display: block;
            color: red;
        };
    }
}

#library {
    .rule-scope() {
        // Override the variable
        @rules: {
            display: inline;
        };
        // Extend the ruleset
        .rules() {
            height: 50px;
        }
    }
}

.output {
    #library > .rule-scope();
    .rules();
}

Though this is not something I would want to use in a practical project. (Too tricky, I believe there much more simple ways, though I need to think of it more to come up with a better method :)

@matthew-dean
Copy link
Member Author

Oh. #library and #library are not the same namespace? Huh. So.... ohhhhhhhh.... So each .rules() was calling its local @rules ruleset. But when I re-evaluated the scope, I imported the foreign @rules, which THEN overrode the local @rules. Ohhhhh...... Now that makes sense.

I believe there much more simple ways, though I need to think of it more to come up with a better method

I'm hoping there are too. I'm literally just trying to support overriding and extending rulesets from a base Less library, which would allow really granular replacement of a library's components. Basically it's one piece that would help support "theming" of a library.

@seven-phases-max
Copy link
Member

I'm literally just trying to support overriding and extending rulesets from a base Less library, which would allow really granular replacement of a library's components.

Probably I'd started with something in this direction:
#1848 (comment) (and the one next after it).
I.e. instead of overriding "default" rules as a variable one could put it as an explicit argument/parameter for .rule-scope() (and that parameter in its turn may have its default value too). Though it's all heavily depends on what "front-end interface" you'd want for such library.

@matthew-dean
Copy link
Member Author

Oh, right, accessing namespaced variables. Yes, that comment is exactly the thrust of where I'm headed. It's funny, there's probably about 4 or 5 solutions in the works for what I'm trying to do, but it hasn't gotten there yet.

It's also too bad that when we use the word "namespace", we don't actually treat it as a common combined namespace (same scope for variables in those namespaces). Or, if that would cause problems, some way to force namespace combining or define a namespace that is auto-confined. Like I said, there are a lot of ways to get there (such as expanding ":extend" to detached rulesets). Maybe we could sort / evaluate them in terms of priority?

@matthew-dean
Copy link
Member Author

Should this be closed because it's not actually the "same scope" according to how Less.js has defined it? The code is actually working as designed, albeit different from the goals I wanted. But that can be addressed in other namespacing- / extend- related discussions.

@seven-phases-max
Copy link
Member

Should this be closed

I guess we could. If nobody minds.

@matthew-dean
Copy link
Member Author

Nope. Closing.

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