-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Global Variables with a literal "." in the value causes the variable to be undefined #3767
Comments
What's the use case? |
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 |
I am working on this. |
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:
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:
Conclusion: I think this is by design and not a bug. @matthew-dean, can you confirm? |
@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 |
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 |
@rdwoodring @buildVersion: e("a.hello");
body { color: @buildVersion; } results in body {
color: a.hello;
} |
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 |
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? |
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. |
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. |
@rdwoodring If you'd like to submit a PR for this particular issue with the CLI, that would be fine! |
I'm taking a look at this issue. I see how it can be solved for browser bundle and how it can be solved for the CLI. I'll experiment further with the best solution. |
* Add more variable tests for issue less#3767.
I have one potential solution in PR #4299 If that is not accepted, I'll try to tweak the Less.js CLI logic instead (but that may be more breaking). |
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:
On the other hand, removing the literal period allows this to work:
LESS Code:
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:
less
version: 3.5.0 - 4.1.3 (current)nodejs
version: 16.13.2operating system
: WindowsAppears to have been introduced in 3.5.0. Version 3.0.4 appears to be working fine.
The text was updated successfully, but these errors were encountered: