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

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

Open
rdwoodring opened this issue Dec 13, 2022 · 14 comments
Labels

Comments

@rdwoodring
Copy link

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:

  • less version: 3.5.0 - 4.1.3 (current)
  • nodejs version: 16.13.2
  • operating system: Windows

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

@rdwoodring rdwoodring added the bug label Dec 13, 2022
@matthew-dean
Copy link
Member

What's the use case?

@rdwoodring
Copy link
Author

rdwoodring commented Apr 1, 2023

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
Copy link

Stegen54 commented Jul 7, 2023

I am working on this.

@praisennamonu1
Copy link

praisennamonu1 commented Aug 1, 2023

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
Copy link
Member

matthew-dean commented Aug 2, 2023

@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
Copy link
Author

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
Copy link

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

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

results in

body {
  color: a.hello;
}

@johnwc
Copy link

johnwc commented Nov 15, 2023

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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

@puckowski
Copy link
Contributor

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.

puckowski added a commit to puckowski/less.js that referenced this issue Dec 15, 2024
* Add more variable tests for issue less#3767.
@puckowski puckowski mentioned this issue Dec 15, 2024
3 tasks
@puckowski
Copy link
Contributor

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).

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

No branches or pull requests

7 participants