Ambiguous error when keyword doesn't exist about ansi-styles HOT 10 CLOSED

wrapAnsi16m seems to be the culprit here. Should it throw an error if the fn returns undefined or should the error be thrown inside of the color-conversion library when it converts a keyword to rgb?

@Qix- I'm happy to do a PR to either project, but I felt like I should run it by you first.

from ansi-styles.

I think it should be an error in color-conversion, but I'll let @Qix- make the final call.

from ansi-styles.

I think it's correct that color-convert returns null if the keyword doesn't exist - in which case, ansi-styles should throw.

cc @danthegoodman - feel free :) It'd be super appreciated.

from ansi-styles.

I'm happy with the solution implemented for color.ansi16m.keyword, since it will cleanly handle other functions if they decide to start returning null or undefined.

Unfortunately, the error handling for color.ansi256.keyword and color.ansi.keyword is specific to the keyword function. These functions caused the same ambiguous error deeper within the color-convert library.

When I had only investigated the ansi16m fix, I was in agreement with @Qix-, that the fix should be in this project. Now that i've seen the error get thrown from within there, I'm learning a bit more in line with @sindresorhus that the conversion library receive a patch instead.

from ansi-styles.

I think it's correct that color-convert returns null if the keyword doesn't exist

@Qix- I disagree. Incorrect input should throw, not silently return null`. That's what the user would expect, and how Node.js and browser APIs work too. Can you please reconsider?

from ansi-styles.

@sindresorhus My only hangup with it is that none of the other methods throw for incorrect input.

> const cc = require('color-convert')
undefined
> cc.rgb.hsl([1000, 522, 10])
[ 31, -198, 198 ]

color-convert was only ever intended to house the math behind the conversions - nothing more. I still think consuming applications should wrap their own throw logic since there are usecases where merely checking if a keyword is valid is useful.

Unless, that is, you want to start advocating for exceptions to be used as flow control.

Something to keep in mind is that all it's really doing is ({...keywordToColorMap})[inputKeyword] which just returns undefined if the key doesn't exist. It's analogous to an object lookup.

from ansi-styles.

My only hangup with it is that none of the other methods throw for incorrect input.

They should throw too, though.

since there are usecases where merely checking if a keyword is valid is useful.

Then there should be a separate method to check whether a keyword is valid. Please make the common-case user-friendly.

Unless, that is, you want to start advocating for exceptions to be used as flow control.

This is not about flow control. It's about input validation and not silently accepting invalid input.

from ansi-styles.

So I should add input validation to every single conversion method?

from ansi-styles.

Yes

from ansi-styles.

Closing in favor of https://github.com/Qix-/color-convert/issues/72.

from ansi-styles.