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

fix: Allows less overriding of imported css custom properties. #3721

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lumburr
Copy link
Contributor

@lumburr lumburr commented Apr 28, 2022

Overwrite declaration in import file.

What: fix #3563

Checklist:

  • Documentation
  • Added/updated unit tests
  • Code complete

@matthew-dean
Copy link
Member

I see this works in this test case, but I'm kind of suspicious about how this works. This works by just re-assigning the declaration pre-eval? 🤔

@lumburr
Copy link
Contributor Author

lumburr commented May 5, 2022

Let's first look at how the problem happened.
For the

@import "./a.less";

@base-color: var(--primary-color);
@dark-color: var(--bg-color);
// a.less
@base-color: green;
@dark-color: darken(@base-color, 50%);

less appends the imported AST to the source AST, so the AST we process is similar to the following structure

- Declaration  base-color // green;
- Declaration  dark-color // darken(@base-color, 50%);
- Declaration  base-color // var(--primary-color);
- Declaration  dark-color // var(--bg-color);

When the program eval() the second nodeDeclaration dark-color // darken(@base-color, 50%), where @base-color is find() to be var(--primary-color) at:

variable = this.find(context.frames, function (frame) {
const v = frame.variable(name);
if (v) {
if (v.important) {
const importantScope = context.importantScope[context.importantScope.length - 1];
importantScope.important = v.important;
}
// If in calc, wrap vars in a function call to cascade evaluate args first
if (context.inCalc) {
return (new Call('_SELF', [v.value])).eval(context);
}
else {
return v.value.eval(context);
}
}
});

Then the value of the node is darken(var(--primary-color), 50%). Since var(--primary-color) cannot be converted to color, the node cannot be serialized by less.


The current modification scheme is to overwrite the duplicate declaration when appending the AST in the import, so that the structure of the AST is as follows:

- Declaration  base-color // var(--primary-color);
- Declaration  dark-color // var(--bg-color);
- Declaration  base-color // var(--primary-color);
- Declaration  dark-color // var(--bg-color);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lazy variables of css custom properties with less functions (even if it's overwritten) will break building
2 participants