Leading zero in string gets removed

[patrickbense]

We were using a setup similar to below:

const BUILD_ID = "012345"; // (generated git hash)
options: {
  vars: {
    version: BUILD_ID
  }
}

The issue we were running into was that upon using $version in our sass files, the leading 0 was being removed from the string which was causing our assets to look at the wrong path on our server.

I was able to "solve" this by setting version to

`"${BUILD_ID}"`

so that the string value we were sending in included double quotes around it. This solved our problem but figured 1) you may want to know about this in case someone else raises the issue and 2) it's probably a good idea to handle that scenario by default.

Line 22 of convertJsToSass.js could be altered to:

if (typeof value === 'string') {
    return `"${value}"`;
}

I'm not totally sure if this creates other problems, but just throwing it out there!

[epegzz]

Thaaaaat is a very good point you have there! :D
Will add it over the weekend 👍 :)

[epegzz]

Just released the change in v4.2.0 👍

[patrickbense]

Thanks @epegzz :)

[patrickbense]

Actually @epegzz, would it be possible to not add quotes if the string sent in already has them? I realize this will cause an issue for us and possibly others that created the work around. Unless you already did that :)

[epegzz]

@patrickbense Yeah you're right, double-quoting should be avoided. Just fixed it in v4.3.0 :)

[epegzz]

@patrickbense I think I have to undo the change because of #29 😬

Because if I want to add colors as vars such as #000 it won't work as CSS property once it's quoted:

background-color: #000 <- will work
background-color: "#000" <- will not work

Or do you see a good way to solve this? 🤔

[epegzz]

Hm, we could maybe only add quotes if the string starts with a space or a zero

[epegzz]

Done in v4.3.1 :)

[jacklorusso]

Hi @epegzz, I really love sass-vars-loader! Thanks for all your work on this. Just wanted to report a use case that may not have been considered — setting a value to something like "0px"

I am working on a whitelabeled application that can be themed via design tokens so that multiple client builds can use the same code. I am using very low level variables to style the application, eg:

buttonBorderRadius: "8px"

For one theme I wanted to create a very angular style, so didn't want to use border-radii:

buttonBorderRadius: "0px"

I believe that because of this change, this didn't work as I expected. I get an "Invalid property value" because of the quote marks. It's frustrating because I can set any other value except for 0px, so it will work for other themes. I don't think this should be the default behaviour if it breaks CSS interoperability.

Actual result:

Expected result:

[jacklorusso]

Happy to open a new issue if you would prefer, just thought that I would keep the conversation in context here.

[epegzz]

@lol-russo True, this should be changed, will do 👍

I'll change it so that the following will not be quoted:

• any 0 followed by a valid CSS unit
• any standalone 0

[epegzz]

@patrickbense I think that maybe it wasn't such a good idea to automatically quote vars after all. It makes the behavior of the lib less predictable and causes other issues.
As the author of this PR says, maybe the best solution really is to explicitly add quotes if you want quotes in the CSS.

[epegzz]

@lol-russo Auto-quoting now got removed in v5.0.0
Sorry for the delay, I totally forgot about this issue!

[jacklorusso]

No worries, thanks for your time @epegzz!