chroma.js accepts invalid values (and converts them to 0), but not always

[bakert]

> const { default: chroma } = await import("chroma-js");
undefined
> chroma("rgb(127 0 255)").hex()
'#7f00ff' # ok
> chroma("rgb(-1000 0 255)").hex()
'#0000ff' # silently converted my invalid r value to 0
> chroma("rgb(null 0 255)").hex()
Uncaught Error: unknown format: rgb(null 0 255) # correctly detected invalid input
    at new Color (file:///Users/bakert/spectrum.ts/node_modules/chroma-js/src/Color.js:39:19)
    at chroma (file:///Users/bakert/spectrum.ts/node_modules/chroma-js/src/chroma.js:5:12)
> chroma("rgb(nonsense 0 255)").hex()
Uncaught Error: unknown format: rgb(nonsense 0 255) # correctly detected invalid input
    at new Color (file:///Users/bakert/spectrum.ts/node_modules/chroma-js/src/Color.js:39:19)
    at chroma (file:///Users/bakert/spectrum.ts/node_modules/chroma-js/src/chroma.js:5:12)
> chroma(127, 9, 255, 'rgb').hex()
'#7f09ff' # ok
> chroma(-1000, 9, 255, 'rgb').hex()
'#0009ff' # silently converted my invalid r value to 0
> chroma(null, 9, 255, 'rgb').hex()
'#0009ff' # silently converted my invalid r value to 0
> chroma('nonsense', 9, 255, 'rgb').hex()
'#0009ff' # silently converted my invalid r value to 0

Would we consider "silently converted my invalid r value to 0" to be a bug in each case above or is this in some way intended behavior? If I have user input for r, g and b is there a good way to validate it using chroma.js or do I need to do my own typechecking and bounds checking before passing to the library?

[gka]

I think converting rgb(-1000, 0, 255) to [0, 0, 255] is the intended behavior, as this is just clipping the components to their valid range. Negative RGB channels can also be the result of converting certain Lab colors to RGB (which makes sense as RGB covers just a subset of colors).

Converting "nonsense" to zero is a bug, on the other hand, this should throw an error.

There's an exception for the string "none" which is actually valid in CSS for many color spaces and the W3C specs say to interpret it as 0.

As for null I'm leaning towards also throwing an error, as it's likely the result of a previous error that we don't want to "mask".