new Color() handles bogus space vs bogus coords differently

[sidewayss]

https://codepen.io/sidewayss/pen/ogvjYrd

I'm looking for clarification on the different results here. In that codepen you'll see 3 Color instances created based on versions of the same serialized color string. The first one is "good", so no issue there.

The "ugly" color has a bogus coordinate: "foo". The Color instance gets created without throwing an error. It is obviously not a displayable color, so display() is filled with NaN.

The "bad" color has a bogus color space --bar. An error is thrown and the Color instance does not get created.

Why are "ugly" and "bad" treated differently? "ugly" could be considered equally illegitimate.

[facelessuser]

I don't think they should be treated differently (I'm not sure if others would disagree though). A bad input value, I think, should throw an error. I don't know how well the parameters of a color function are validated.

[sidewayss]

I have updated the codepen to illustrate the same behavior with colors instances created using this syntax:

color = new Color("spaceId", [0, 0, 0]);

For real-world reference: I stumbled on this different treatment because this comment in an issue in the elements repo here triggered the idea to test it. So it might be relevant in that situation. My code is definitely affected, though it's only a few extra lines. I just added an additional check for this situation where I already have a try...catch block. The main thing is knowing that you have to handle it.

[sidewayss]

To clarify:

• The "ugly"s have only one NaN coordinate. It's the conversion to the display space that is causing them to all be NaN.

So there is a difference between the "ugly" and the "bad", though both result in undisplayable colors.

[facelessuser]

So, originally, I didn't look to close at the example, but now that I've looked closer, I have some comments.

The "ugly"s have only one NaN coordinate. It's the conversion to the display space that is causing them to all be NaN.

This is expected. In this example, the string is passed to a parser, the unexpected value is parsed and identified as an error and converted to NaN which is an error. If you perform any operation with NaN, it infects. Color.js has been reworked recently such that channel errors are NaN and undefined are null. Errors are meant to stand out. So I guess that is the way they are signalling errors. Some could argue it should throw, and maybe there should be a way to configure color.js to do so, but this is there way of showing an error.

The bad is failing because there is no space with that name to create a color with, so it is stopped dead in its tracks and is forced to throw.

Later you feed strings into the numerical interface. It isn't being "parsed" per se, and it seems to expect that the user knows how to use the interface. Since the values aren't validated, when serializing occurs, the bad values just get serialized. Color.js could validate all numerical inputs, but it adds overhead. This is a user error, feeding in non-numeric values into the numeric interface.

[sidewayss]

fyi - this is my additional check for NaN:

if (color.display().includes("NaN"))

Otherwise I'm not sure which of these might be correct:

if (color.coords.some(v => isNaN(v));

or

if (color.coords.some(v => Number.isNaN(Number(v)));

or maybe parseFloat() should be in there. But display() is definitely reliable.

[sidewayss]

@facelessuser - I didn't see your last comment until I had posted mine 2 minutes later. I think we're on the same page. What you say is the reason why I'm using display() instead of coords to determine if it has any bad coordinates. The coordinates can definitely be a string and there are several way to convert those to numbers. I don't know what other values or types could be in there, so display() is the only way to go.

This is a user error, feeding in non-numeric values into the numeric interface.

In my case I'm validating the text in an <input type="text"/>, so it's definitely a user error, but I don't know that until I check for it specifically. As I said, if you know that this is an issue up front, and you know how to handle it, then it's easy. I would prefer that it throw an error, as it's less work for me. But I don't know what use there might be for an undisplayable color instance, or what other people are doing with this library.

[sidewayss]

...and isn't supplying an invalid color space id to the color space id interface a user error too? Why would that be different than an invalid number? I understand the mechanics of "getting stopped in its tracks".

[facelessuser]

I would prefer that it throw an error, as it's less work for me.
...and isn't supplying an invalid color space id to the color space id interface a user error too? Why would that be different than an invalid number? I understand the mechanics of "getting stopped in its tracks".

Sure, I'm not the one who decided to convert it to NaN instead of throwing an error, so I don't know what the reasoning is. I also haven't looked through that area of code to see how intentional it was either. Technically, NaN is correct as the value is not a number, but with all kidding aside, I would probably prefer it to throw an error as well.

[sidewayss]

Sure, I'm not the one who decided to convert it to NaN instead of throwing an error, so I don't know what the reasoning is. I also haven't looked through that area of code to see how intentional it was either.

It's not converting to anything in the constructor. The conversion to NaN happens in display(). There is no validation of the coordinates in new Color() AFAICT.

[sidewayss]

fyi - Color.js accepts a lot of strings with non-numeric characters for coordinates and successfully converts them to numbers. For example, display() uses a % suffix for lightness coords, but you can remove that suffix or replace it with several other characters and new Color() interprets it as the same color.

In my small set of test cases it is more restrictive than parseFloat(). parseFloat() allows suffixing a number with any of these characters: a-zA-Z!@#$%^&*()[]=_. Color.js doesn't accept the underscore _ or any alphabetic characters.

But the parentheses are weird. new Color() considers this a valid color, essentially ignoring the first right parenthesis:
color(--lchuv 86.8% 42.9)230)
Note that there is no space between the 2nd and 3rd coords, the only separator is the right parenthesis. It's the same result with a space after the right paren.

Not complaining, just observing.

[LeaVerou]

Unitless numbers are valid in CSS now too, for all color spaces.

The latter Id consider a bug though.

[sidewayss]

@LeaVerou - I've submitted issue #614.

What about the original question here? Is there any sense in submitting an enhancement request to throw an error instead of instantiating an undisplayable Color?