search

sass / node-sass

:rainbow: Node.js bindings to libsass
https://npmjs.org/package/node-sass
MIT License
8.51k stars 1.32k forks source link

rgb and rgba functions misinterpret CSS variables as a single value instead of their output #2251

Open jannisborgers opened 6 years ago

jannisborgers commented 6 years ago

Example:

:root { --color--text: 18, 18, 26; }
body { color: rgba(var(--color--text), 0.8); }

Expected outcome:

body {
color: rgba(18, 18, 26, 0.8);
}

Actual outcome:

When using rgba():

{
  "status": 1,
  "file": "/path/to/partial.scss",
  "line": 30,
  "column": 23,
  "message": "argument `$color` of `rgba($color, $alpha)` must be a color\n\nBacktrace:\n\tsrc/scss/elements/_projectTeaser.scss:30, in function `rgba`\n\tsrc/scss/elements/_projectTeaser.scss:30",
  "formatted": "Error: argument `$color` of `rgba($color, $alpha)` must be a color\n\n       Backtrace:\n       \tsrc/scss/elements/_projectTeaser.scss:30, in function `rgba`\n       \tsrc/scss/elements/_projectTeaser.scss:30\n        on line 30 of src/scss/elements/_projectTeaser.scss\n>>     background-color: rgba(var(--color--fadePrimary), 0.8);\n   ----------------------^\n"
}

When using rgb(), without the alpha-value:

{
  "status": 1,
  "file": "/path/to/partial.scss",
  "line": 30,
  "column": 23,
  "message": "Function rgb is missing argument $green.",
  "formatted": "Error: Function rgb is missing argument $green.\n        on line 30 of src/scss/elements/_projectTeaser.scss\n>>     background-color: rgb(var(--color--fadePrimary));\n   ----------------------^\n"
}

Notes: This works if I wrap the CSS variable values in their own rgb() function and just output those as property values, but in order to add different alpha-values on the fly, storing only the R, G, and B values in variables is a lot more flexible.

As I posted this on Twitter and instantly got a backlash: please note that of course I could use $sass-variables instead, but CSS variables are a lot better to work with for obvious reasons (mutability, element scoping).

Versions:

- NPM version (`npm -v`): 5.6.0
- Node version (`node -v`): 6.10.2
- Node Process (`node -p process.versions`): 
{ http_parser: '2.7.0',
  node: '6.10.2',
  v8: '5.1.281.98',
  uv: '1.9.1',
  zlib: '1.2.11',
  ares: '1.10.1-DEV',
  icu: '58.2',
  modules: '48',
  openssl: '1.0.2k' }
- Node Platform (`node -p process.platform`): darwin
- Node architecture (`node -p process.arch`): x64
- node-sass version (`node -p "require('node-sass').info"`): 
node-sass       4.7.2   (Wrapper)       [JavaScript]
libsass         3.5.0.beta.2    (Sass Compiler) [C/C++]
- npm node-sass versions (`npm ls node-sass`): node-sass@4.7.2
nschonni commented 6 years ago

var support doesn't come until https://github.com/sass/libsass/issues/2244 is landed in this repo

jannisborgers commented 6 years ago

@nschonni I see, thank you.

Temporary workaround: Ana Tudor noticed on Twitter that you can use RGB((--var)) or RGBA((--var),0.8) (note the uppercase function name), as sass will ignore it and CSS will parse it just fine. (see https://twitter.com/anatudor/status/963758591529365504)

Jakob E also noted that wrapping the whole function in #{' '} interpolation will work, although this approach failed in my case. (see https://twitter.com/anatudor/status/963758591529365504)

fvsch commented 6 years ago

Unless I’m mistaken the expected outcome is not possible and will not be possible with what @nschonni mentions.

CSS Custom Properties are dynamic, they inherit through the DOM tree and can be set in JS at run-time, can be set on different DOM nodes by selectors, etc. Some Postcss plugins use the CSS Custom Properties syntax with :root { --some-var: … } and treat them as global variables similar to Sass variables, which is a pretty bad idea IMO.

This code should always throw:

body { color: rgba(var(--color--text), 0.8); }

And this one (a pure CSS feature) should probably be passed as-is by Sass:

body { color: color(var(--color--text) alpha(0.8)); }

But note it’s syntax from a very incomplete draft of CSS Color Module Level 4, so syntax may change and browser support is far off. (It also introduces a bunch of syntax that Sass will probably choke on, like + modifiers before numeric values that have a meaning in CSS and should be left alone by Sass.)

xzyfer commented 6 years ago

@fvsch is mostly correct.

The expected output is impossible because rgba(color, alpha) is a Sass function. The color argument must be a Sass Color type. Sass does not evaluation css custom property values, because these are run-time variables.

However this input should not throw. Node Sass should pass through values it doesn't understand, but are still valid CSS values. The correct output should be:

:root {
  --color--text: 18, 18, 26; }

body {
  color: rgba(var(--color--text), 0.8); }

This output is obviously jibberish, but the behaviour is correct as far as Sass is concerned. This will be resolved in v4.8.0.

xzyfer commented 6 years ago

Fixed in 4.8.0

ro0gr commented 6 years ago

Unfortunately it doesn't seem to be fixed.

I've just installed node-sass@4.9.0 and the following code snipet:

var sass = require('node-sass');
var result = sass.renderSync({
    data: `body {
        --c: 1,2,3;
        background-color: rgb(var(--c));
    }`
});
console.log(result);

fails for me with the following stack trace:

Ruslans-MBP:sass-test RHR$ node index.js
/Users/RHR/workspace/tmp/sass-test/node_modules/node-sass/lib/index.js:439
  throw assign(new Error(), JSON.parse(result.error));
  ^

Error: Function rgb is missing argument $green.
    at Object.module.exports.renderSync (/Users/RHR/workspace/tmp/sass-test/node_modules/node-sass/lib/index.js:439:16)
    at Object.<anonymous> (/Users/RHR/workspace/tmp/sass-test/index.js:2:19)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:140:18)
    at node.js:1043:3

@xzyfer do I miss something, do I need some special treatment to make it work?

mrahhal commented 6 years ago

This only seems to work when setting css variables. In @ro0gr's example, this still causes problems. But then.. I don't understand, was this only fixed when setting css vars?

Uppercase RGB seems to be working for now though.

xzyfer commented 6 years ago

Can you confirm you're using 4.9.0 with npm ls node-sass?

xzyfer commented 6 years ago

Looks like we don't have test for RGB with a single argument. https://github.com/sass/sass-spec/pull/1002/files

It's entirely possible this specific case is not handle correctly.

mrahhal commented 6 years ago

I am using node-sass@4.9.2.

It only works with:

--some-var: rgb(var(...));

Never with:

color: rgb(var(...));

Had to use RGB.

UstymUkhman commented 6 years ago

I think I'm having a related issue today. My test was:

$green-code: '0, 204, 0';
$green: unquote('rgb(#{$green-code})');

$gray: rgb(33, 33, 33);

.test {
  background-color: rgba($green, 0.5); // This line gives me "$color: rgb(0, 204, 0) is not a color."
  color: rgba($gray, 0.5);
  color: $green;
}

Although, as you can test here, both $green and $gray are RGB variables.

vasicvuk commented 5 years ago

Same problem with latest version getting: Argument `$color` of `fade-out($color, $amount)` must be a color where $color is var(--primary)

jalovatt commented 5 years ago

For what it's worth the same problem exists using the other color functions:

--bg-color: 210, 25%, 15%;
background-color: hsla(var(--bg-color), 0.9);

--> Error: Function hsl is missing argument $lightness.

Running node-sass 4.10.0 / libsass 3.5.4.

Jordaneisenburger commented 5 years ago

I'm also having the issue described above, is there already a solution ?

ro0gr commented 5 years ago

@Jordaneisenburger definitely not a solution, but defining custom rgb/rgba functions which accepts a single argument allows to work-around the issue for us:

@function rgb($r, $g: null, $b: null ) {
  @if ($g == null) {
    @return unquote('rgb(#{$r})');
  }

  @if ($g and $b) {
    @return unquote('rgb(#{$r}, #{$g}, #{$b})');
  }

  @error "wrong number of arguments";
}

@function rgba($r, $g, $b: null, $a: null ) {
  @if ($b == null) {
    @return unquote('rgba(#{$r}, #{$g})');
  }

  @if ($b and $a) {
    @return unquote('rgba(#{$r}, #{$g}, #{$b}, #{$a})');
  }

  @error "wrong number of arguments";
}
Jordaneisenburger commented 5 years ago

@ro0gr thanks for the response, unfortunately i didn't get it to work because the rgb() functions are used inside a .css file. I've tried to include a fix.scss file with your function in it at the top of the src code but somehow didn't work. Thanks anyway!

rezalas commented 5 years ago

This issue appears to still exist in 4.12 and applies to hsl as well.

html {
    --hue: 210;
    --background: hsl(var(--hue), 20%, 12%);
}

.testClass {
    background: var(--background);
}

Will always attempt to interpret the variable as SASS and throw an error that hue must be a number.

mattlohkamp commented 4 years ago

I've gotta admit I'm surprised that this is still an issue, in v4.14.1, almost two and a half years later - using RGBA() instead of rgba() is still a valid workaround though.

AdrianoCahete commented 4 years ago

I'm migrating back to Sass since Stylus is deprecated and stepped into a problem that I think that it's the same as this issue.

I'm trying to update an old function that I have to determine if background is dark or not and choose the text color, but I found that this not work with CSS variables.

// vars file
:root {
  --primaryColor: rgba(39, 190, 213, 1);

  --textColor: rgba(51, 51, 51, 1);
  --textColorInverted: rgba(255, 255, 255, 1);
}
// Function
@function isDark($color) {
  @if (lightness( $color ) > 40) {
      @return var(--textColor);
    }
  @else {
    @return var(--textColorInverted);
  }
}
// In use
button {
  background-color: var(--primaryColor);
  color: isDark(var(--primaryColor));
}

The error is:

SassError: argument `$color` of `lightness($color)` must be a color
cpylua commented 3 years ago

dart-sass doesn't seem to have this issue, so switch to dart-sass if possible.

If you're out of luck with dart-sass and don't like the uppercase RGB workaround, here's another solution for node-sass:

:root { 
    --color--text: 18, 18, 26; 
}

$color-background: var(--background-color, 237, 237, 237);

body { 
    color: unquote("rgba(var(--color--text), 0.8)");

    // or with sass variable
    background: unquote("rbg(#{$color-background})");
}

Use a function if you don't want to write unquote every time.

:root { 
    --color--text: 18, 18, 26; 
}

$color-background: var(--background-color, 237, 237, 237);

@function safe-rgb($var) {
    @return unquote("rgb(#{$var})");
}

@function safe-rgb2($css-var) {
    @return unquote("rgb(var(#{$css-var}))");
}

body { 
    background: safe-rgb($color-background);
    color: safe-rgb2(--color-text);
}
BenRacicot commented 3 years ago

Working within Angular 11.0.2. Can confirm, still a problem.

midblue commented 3 years ago

Somehow this error doesn't occur locally but then on my Heroku server (running Nuxt) the Nuxt build fails with the same $green error. Can confirm that RGB( instead of rgb( does work, however.

BenRacicot commented 3 years ago

@midblue I cannot remember how I resolved the green issue but I did witness it in Angular 11. You could check that node-sass version.

See https://stackoverflow.com/a/11973298/1440240

Are you deploying SASS files or part of your CI or something?

nhhockeyplayer commented 3 years ago

this is a show stopper for me... someone has to nail this finally its grown and endless threads on color parsing

Karmalakas commented 2 years ago

Having exact same issue...

  --color: 123,214,123;

  //color: scale-color(rgb(var(--color)), $lightness: -30%); // <-- Error: $color: rgb(var(--color)) is not a color.
  color: scale-color(rgb(123,214,123), $lightness: -30%); // <-- Works just fine

Neither uppercase nor sass variable solutions helped

BTW:

sass --version
1.43.4 compiled with dart2js 2.14.4
nschonni commented 2 years ago

@Karmalakas the sass package is https://github.com/sass/dart-sass, not this repo

Prajapatisantu commented 2 years ago

simple take rgb value and go to rgb to hex site, then simply convert and put it in your code problem solved!

ghost commented 1 year ago

It's 2023, this has been a major issue for 5 years. Can this issue please have more attention so it can be fixed, instead of having to specify like 4 separate color variables just to add a little transparency to your website?

QWERTAL124 commented 9 months ago

Which color --bs-bg-opacity var ? I couldn't find this variables value !

cdwmhcc commented 6 months ago

It’s unbelievable that this problem still exists to this day!