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

CSS variables in LESS variables breaks sourcemaps #3300

Closed
eamodio opened this issue Jul 31, 2018 · 11 comments
Closed

CSS variables in LESS variables breaks sourcemaps #3300

eamodio opened this issue Jul 31, 2018 · 11 comments

Comments

@eamodio
Copy link

eamodio commented Jul 31, 2018

@color: var(--foo); // works if this isn't a css variable

body {
	border-left: 1px solid @color;
	width: calc(50% - 5px);
	border-top: 1px solid @color; // Fails here
}

If you compile this with sourcemaps on, it fails with SyntaxError: Cannot read property 'substring' of undefined on the border-top. If you don't use a CSS variable for @color it will work fine, and also until a use of calc() everything will also work fine. But any rules after the use of calc will fail if the rule is a mix of css and a variable.

@matthew-dean
Copy link
Member

Interesting...

@eamodio
Copy link
Author

eamodio commented Aug 1, 2018

lol 😄 Took me forever to figure out what was going on -- for the longest time I thought it was a webpack/less-loader issue

@pascallaprade-beslogic
Copy link

I believe I have a similar issue, that may be caused by the same thing. I'm on less 3.8.0, as published 10 days ago on NPM (lessc 3.8.0 (Less Compiler) [JavaScript]).

My sourcemaps are not broken, but my app style is. We defined rgba values in variables, and they come out broken from the compilation process.

Our original code made use of transparency in hex form, stored in CSS variables (--semi-transparent-dark-background: #001e00ee;). The code that came out was this: --semi-transparent-dark-background: #001e00ee ee;.

We tried changing it to an rgba function, but then it is converted into a hex without transparency. --semi-transparent-dark-background: rgba(0, 30, 0, 238); becomes simply --semi-transparent-dark-background: #001e00;.

I too thought it was related to the webpack bundling process one way or another, until I tried compiling the files directly using lessc, and it gave me the two cases I listed above.

So: I don't know what else is broken in the variables system, but for us, it means that we cannot get transparency go through. We'll revert to the previous version of less until then.

matthew-dean added a commit to matthew-dean/less.js that referenced this issue Aug 6, 2018
@matthew-dean
Copy link
Member

matthew-dean commented Aug 6, 2018

@pascallaprade-beslogic No, your issue is not related. Your issue is an older one where the RGBA form was not supported at all, and that's been added only recently. See this PR - #3291

@matthew-dean
Copy link
Member

@eamodio Can you test this branch and verify that source maps are generated as expected? https://github.com/matthew-dean/less.js/tree/bugfix-3300. If so, I'll do a proper release. The sourcemaps test coverage was pretty minimal and I had to do lots of rewriting of tests, so I want to verify it's working properly.

@matthew-dean
Copy link
Member

@pascallaprade-beslogic Can you test against that branch as well as it should have your fix?

@pascallaprade-beslogic
Copy link

Thank you very much!

I tested the branch you linked above with our two cases, the rgba hex and the rgba function: the hex came out without the duplicated alpha value at the end, and the rgba function was left as is in the output, instead of being transformed into a hex without the alpha value.

Thanks again!

@matthew-dean
Copy link
Member

@pascallaprade-beslogic So, not to contradict you, but I'm re-testing, and I the rgba() you provided will come out as hex for the simple reason that it's an invalid rgba alpha value. Alpha is between 0 and 1, and you have an alpha value of 238. Less will then cap that at a value of 1. So I'm not sure what you were hoping as output, but it will be a hex value on that branch.

@pascallaprade-beslogic
Copy link

No, you are right! The rgba function I showed in my first message in this thread was wrong. I rarely use rgba functions, so I did not use an alpha value in the correct range. (As I said in my first message, we only use hex values in our app, so the rgba attempt was just me trying to see if it affected rgba as well, and going too fast to realize I was doing it wrong.)

However, I realized it when I did the test today, and wrote the proper rgba function.

So indeed, only the hex was broken, the rgba worked as expected.

Here's my latest test that shows the three attempts I made, hex, wrong rgba and correct rgba:

.test {
    --hex: #00112233;
    --rgba-wrong: rgba(1, 2, 3, 4);
    --rgba-right: rgba(1, 2, 3, 0.5);
}

Output with lessc 3.8.0 (Less Compiler) [JavaScript]

.test {
  --hex: #00112233 33;
  --rgba-wrong: #010203;
  --rgba-right: rgba(1, 2, 3, 0.5);
}

Output with less.js-bugfix-3300

.test {
  --hex: #00112233;
  --rgba-wrong: #010203;
  --rgba-right: rgba(1, 2, 3, 0.5);
}

So, again, sorry for the false alarm about the rgba function being affected as well, I do realize it was my mistake there.

And thank you for your time about validating that the hex is indeed fixed in that unreleased branch!

@eamodio
Copy link
Author

eamodio commented Aug 8, 2018

@matthew-dean sorry for the delay in getting back to you, but it looks resolved. No more errors and the source maps (as far as a quick test) look good. Thank you!!

@matthew-dean
Copy link
Member

@eamodio Good to hear! You're welcome!

robmcguinness pushed a commit to Availity/availity-workflow that referenced this issue Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants