less / less/less.js

Reflecting current set of CSS selectors : context.selectors not being carried over in eval contexts

Open
#2,619 12 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

feature request support as plugin up-for-grabs
Dominant language
JavaScript
Stars
17k
Forks
3.4k
Avg merge
7h 42m
Merged PRs (30d)
26

Description

In working on some complex framework components written in Less, I have a few times already come across situations where it would have been beneficial to be able to reflect the current set of CSS selectors into a variable for processing.

I've succesfully built a custom function that does just this, but it runs afoul of an issue deeper in the Less compiler.

My usecase

My current case and the one I'll treat here as an example for the reflection feature and how it leads up to the problem with the compiler, is a stylized button that contains both a left/right positioned font-icon and a text span that has horizontal ellipsis applied for overflow and can have its own text left/right/center aligned within the remaining space on the button. E.g.

<button type="button" class="imp-button" data-iconpos="left">
  <i class="imp-icon" data-icon="check"></i>
  <span class="imp-button-text">OK</span>
</button>

Currently, I have a mixin which renders the base of the button's CSS; handling things like stripping default button styles, setting proper padding, line-height, etc. and handling the quirks of ::moz-focus-inner on older Firefox builds.

As part of this mixin, I have to set both CSS for the button itself and for the text span inside it and both class names have a shared root in the name. (OOCSS principles. This pattern is also re-used across multiple of our sites.)

No problem right?

.lib-button(@height; @hor-padding) {
   // Set some properties for the main button here

   & > &-text {
     // And for the text span here.
   }
}

Call via:

.imp-button {
  .lib-button(40px, 10px);
}

And done:

.imp-button { }
.imp-button > .imp-button-text {}
Where my usecase breaks

This is a nice pattern, but it still breaks when we subclass the button, e.g. make a compact variation:

.imp-button--compact {
  .lib-button(20px, .5px);
  font-size : 10px;
  // etc.
}
.imp-button--compact { }
.imp-button--compact > .imp-button--compact-text {} /* <-- Uh oh! */

It should generate .imp-button--compact > .imp-button-text instead, so that we don't have to double up on all the nested classes on subcomponents for different variations.

Arriving at a solution

I've come up with a few complex builder patterns to work around this, but none of them really fit the bill 100% and really, they all feel like just stacking on needless complexity, when I could solve this tout-de-suite if I could have a little look at the right-hand key-selector and chop off the part after the double dash that we use in our convention and thus simply hide all the complexity from my libraries' consumers.

E.g.

.lib-button(@height; @hor-padding) {
   // Pull up the current right-hand key selectors and work with the first one.
   // (KISS: assume people aren't calling this with multiple selectors.)
   @selector : extract(key-selectors(),1);

   // Some extraction of the base and variation part of a classname. Probably regex match?
   @class-base      : ...
   @class-variation : ...

   .lib-button-text() {
     // Shared text span implementation here.
   }

   &-text when ( @class-variation = "" ) { .lib-button-text();  }
   & > @{class-base}-text  when not ( @class-variation = "" ) { .lib-button-text();  }
}


.imp-button {
  .lib-button(40px, 10px);
}

.imp-button--compact {
  .lib-button(20px, .5px);
}

Output:

.imp-button {}
.imp-button-text {} /* <-- Nicely shortened, removing the redundant child combinator! */

.imp-button--compact {}
.imp-button--compact > .imp-button-text {} /* <-- Perfect! */

I've already implemented a generic selectors() function to get the full set of active selectors (taking into account & and variable-name token replacement) and a key-selectors() function that does the same, but returns the key-selector (i.e. the most right-hand element) only.

It took some digging to figure out how to work with the data structures that the compiler has in place, but it was very doable as a pluggable extension function that walks back over the stack of selectors maintained in the Eval context of the function call node and assembles them into the full set of complete selectors. If that sounds as a surprise: yes they are accessible via this.context.selectors, as functions are executed with this bound to the node instance.

(Friendly Note: The fact that you can acccess the current eval context that way is also a god-sent that empowers plugin functions to fullfill several very complex scenarios. Please, please, please; don't ever change that!)

The actual problem

The problem is that this works fine for regular nested rulesets, but currently BREAKS when performed directly or indirectly inside a mixin or detached ruleset.

I traced down the root cause, which is the fact that when callEval is executed for a DetachedRuleset or MixinDefinition node, a new Eval context is created and the selectors are not part of the copy-constructed properties. Ergo, inside a mixin or ruleset call this.context.selectors resets to the empty set.

Would there be any harm in adding "selectors" to the array of cloned properties for Eval contexts? (I could imagine it breaking the join-selector visitor in some way, but I'm not sure.)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Trace callEval for DetachedRuleset and MixinDefinition nodes, focusing on how the new Eval context copies properties. Confirm where selectors are reset during mixin or detached-ruleset evaluation and inspect the join-selector visitor for affected assumptions. Done means the existing selectors remain available through this.context.selectors in these calls without breaking regular nested rulesets.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.