less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.01k stars 3.41k forks source link

Global Variables with a literal "." in the value causes the variable to be undefined #3767

Open rdwoodring opened 1 year ago

rdwoodring commented 1 year ago

To reproduce: This can be reproduced using the CLI or the programmatic API. Assuming that LESS is already installed, it is most straightforward to reproduce using the CLI:

echo "body { color: @buildVersion; }" | npx lessc --global-var="buildVersion=a.hello" -

On the other hand, removing the literal period allows this to work:

echo "body { color: @buildVersion; }" | npx lessc --global-var="buildVersion=hello" -

LESS Code:

body { color: @buildVersion; }

Current behavior: The LESS compiler throws an error that the global variable is undefined.

Expected behavior: The LESS compiler should not throw an error and the global variable should be defined.

Environment information:

Appears to have been introduced in 3.5.0. Version 3.0.4 appears to be working fine.

matthew-dean commented 1 year ago

What's the use case?

rdwoodring commented 1 year ago

My use case is that we're cache busting things like background images, font files (font icons), etc. Based on the semantic version of my web app. That global variable gets appended to the url or the resources in a query string and so cache busts when we roll out a new version .

Certainly we could select some other string on which to cache bust, but as noted, we have existing code in the wild that's already working using the semantic version number on an older version of less.

Hopefully that makes sense.

EDIT: I should add that I'm happy to submit a PR to resolve, just might need a little help where to look. I spent a little bit of time triaging but just didn't have bandwidth during my daily work to really dig in. Also wanted to understand if this was a known issue etc. Before committing the time

Stegen54 commented 1 year ago

I am working on this.

praisennamonu1 commented 1 year ago

After conducting deep research into the issue, I discovered that some special characters were excluded (on purpose) when variables are to be declared. Here is a list of them:

., #, @, $, +, /, ', ", *, `, (, ;, {,} and -

These were excluded because they are used as selector elements when executing some of the features of Less.

In this case, permitting a literal period to be used when declaring a variable causes some important functionality of Less to fail:

@rdwoodring, if you insist on using a literal period when declaring a variable, you can include them in quotes like this:

echo "body { color: @buildVersion; }" | npx lessc --global-var="buildVersion='a.hello'" -

Conclusion: I think this is by design and not a bug.

@matthew-dean, can you confirm?

matthew-dean commented 1 year ago

@praisennamonu1 Yes, that sounds right. While the parsing is not 100% identical to the CSS spec, in general (to my recollection), variable names correspond with the rules for a CSS identifier. That is, in general, it's alphanumeric + dash as the only valid characters. (CSS does allow for escapes, such that .a\.class I believe is a valid class name with an actual dot in it, but Less is more strict about variable names.

rdwoodring commented 1 year ago

Still seems like a regression to me given that it's a breaking change somewhere between 3.0.4 and 3.5.0, but I won't argue.

This is probably enough info for me to find a workaround to move forward and obviously up to you, @matthew-dean , if you want to close this issue or keep it open

lubber-de commented 1 year ago

@rdwoodring Workaround: use string ecsaping if you really need this

@buildVersion: e("a.hello");
body { color: @buildVersion; }

results in

body {
  color: a.hello;
}
johnwc commented 10 months ago

A good use case for needing to be able to put some of those excluded characters. Setting a global variable for a cdn domain or path. Development builds use http://cdn01.example.com/path/base.css and production builds use http://cdn02.example.com/path/base.min.css

matthew-dean commented 10 months ago

Oh wait a minute, I think I mis-read the original post. This is not about periods in the variable, but periods in the value, correct?

rdwoodring commented 10 months ago

Yep, in the value. My use case is that we have a custom library of font icons imported in a less file. We cache bust the url for that font file using the current version of our software as a query parameter on the url. So semantic version numbers like 6.4.2 no longer work as the variable value.

rdwoodring commented 1 week ago

I personally still see this as a regression since it used to work and no longer does, but the suggestion by @lubber-de does indeed resolve the issue I had.

@matthew-dean I'm happy to close this or if there's any documentation or code fixes you'd like to see, please let me know and I can try to squeeze in some time to tackle it.

matthew-dean commented 1 week ago

@rdwoodring If you'd like to submit a PR for this particular issue with the CLI, that would be fine!