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 visibility across multiple & scopes (questionable). #1877

Closed
seven-phases-max opened this issue Feb 16, 2014 · 7 comments
Closed

Mixins visibility across multiple & scopes (questionable). #1877

seven-phases-max opened this issue Feb 16, 2014 · 7 comments

Comments

@seven-phases-max
Copy link
Member

Since version 1.6.2 & {} blocks are merged together in the output (cool feature, btw.) but this also changed mixins visibility across multiple & scopes, e.g.:

.something {
    & {
        .a {value: a}
    }

    & {
        .b {.a} // was Err. before 1.6.2
    }
}

result:

.something .a {
  value: a;
}
.something .b {
  value: a;
}

At first I thought it's a bug actually, but now I think it's questionable. Either way this leads to another confusing example:

.something {
    & {
        @a: A;
        .a {
            value: a;
        }
    }

    & {
        .b {
            .a;        // mixins are visible
            value: @a; // but variables are not -> Error
        }
    }

    .a; // still invisible here -> Error
}

Looks quite chaotic.

@lukeapage
Copy link
Member

I could change when they are folded in so it goes back to old behaviour.

@seven-phases-max
Copy link
Member Author

Yes, I would vote for the old behaviour unless someone comes with an argument againts it.

@seven-phases-max
Copy link
Member Author

I reopen this as I faced another "& and mixin visibility" issue which is basically the same but makes things even more uncertain because of its context.

Consider the following example:

#something {
    & .a {
        1: 1;
    }

    & { .a {
        2: 2;
    }}
}

.test {
    #something > .a;
}

Before 1.6.2 only the first .a was matched, now (in the current master) both are matched. The new behaviour seems to be more expected in this case since both .a result in the same CSS selectors and it's not so easy to find a reason for them to work differently when searched for the mixin match. But this leads to another problems:
Beside the initial example of this topic (which now looks even more questionable) there's also different behaviour of the same code with parametric mixins, e.g.:

#parathing() {
    & .a {
        1: 1;
    }

    & { .a {
        2: 2;
    }}
}

.test {
    #parathing > .a; // matches only the first .a (same for current master and earlier versions)
}

I bother with these examples because I'm working on some mixin matching improvement patch - and there the "selector.match" function (or one of its internals) looks like the correct place to handle all this, but now I'm not sure what would be the right choice (i.e. I assume the consistent behaviour for all of above cases would be either "always match" or "never match").
So I need some feedback :)

@lukeapage
Copy link
Member

I thought about this when fixing and decided accessing mixins through
another class was an ok exception (e.g use of > bypasses &) but with your
further examples I'm not sure.

I suggest we think about splitting up the eval phase into multiple visitors
but given the cycle dependencies I'm not sure how to do it in way that
isn't super slow.

@seven-phases-max
Copy link
Member Author

OK, I guess we'll wait for more opinions (@SomMeri?) and meanwhile I'll keep moving with both variants in mind. Ah, I forgot to mention that the main goal of my patch is to enable "matching for selectors concatenated across nesting boundary" (see #1624 example) and to do that I reworked the whole selector.match thing to a method mentioned there thus opening a room for further improvements (if any). Luckily the performance did not suffer (it's even +3% speed up :))

@SomMeri
Copy link
Member

SomMeri commented Feb 28, 2014

I would expect the latest example:

#something { 
    & .a {
        1: 1;
    }

    & { .a {
        2: 2;
    }}
}

.test {
    #something > .a;
}

to be compiled into:

#something .a {
  1: 1;
}
#something .a {
  2: 2;
}
.test {
  1: 1;
}

Eg. #something > .a; does not see inside & {}. This would be my argumentation:

  • When I think about mixins and variables, I think about them in the context of less and not in the context of generated css. They generate the same css, but source less is different so it is ok for them to behave differently.
  • The &{ } is just special case of .selector & .class,so they should behave the same way. If one of them creates new scope, the other one should too.
  • I like the &{ } trick to create "private" variables and mixins, but that may be me being used to everything private from java.

@seven-phases-max
Copy link
Member Author

Merging to #2072 (because it's actually the same feature/issues: visibility of variables and mixins should always be equal, so whichever decision comes in #2072 it should also apply here).

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