Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse(): thow an error when parsing is unsuccessful #148

Open
meodai opened this issue Oct 6, 2021 · 2 comments
Open

parse(): thow an error when parsing is unsuccessful #148

meodai opened this issue Oct 6, 2021 · 2 comments
Labels
Feature New feature or request

Comments

@meodai
Copy link
Contributor

meodai commented Oct 6, 2021

It would be very practical, if parse would throw an error if it was not successful in parsing a color.

try {
  const color = culori.parse('something');
  console.log({color}); // {{ color: undefined }}
} catch (error) {
  console.error(error);
}

The only way I can do this now is check for undefined.
I'd find it very practical to get something like:

throw new Error(
   `parse() must be an object or a valid color string but \`${arguments}\` were given.`
);
@danburzo
Copy link
Collaborator

danburzo commented Oct 6, 2021

Hi @meodai, thanks for bringing this up.

I was just thinking the other day that I should make explicit in the API Reference that it's a convention throughout the Culori API for methods to return undefined when they can't produce a valid color, and that a chain of methods, eg. formatHex(hsl(rgb('tomato'))) should not break on unparseable strings.

This makes it nicer to code in environments where you primarily work with expressions, such as Observable notebooks, but also in regular code, where you can casually insert your own parsers or other fallbacks:

const myColor = culori.parse(string) || myFancyParser(string) || culori.rgb('#0000');
if (myColor) {
  // do something with `myColor`
}

I don't believe parsing an invalid string should throw (and it wouldn't have much useful feedback other than that the string is not understood), but I will look into throwing TypeErrors when the API is invoked with the wrong types of arguments. (Also JSDoc / TS type definitions #128 may help with IDE autocompletion).

It would also maybe make sense to have a way to switch the whole API from the return undefined paradigm to be throw-based.

@meodai
Copy link
Contributor Author

meodai commented Oct 6, 2021

@danburzo ah ok I get it. But maybe having a isValid() method then?
I am working on a class that uses culori, I want to make sure all the colors that are passed in, can be parsed.

isValid('red') // => true
isValid('jeronimo') // => false

or maybe validate(color): that would return an error code and what part did not work

validate(rgb(260,0,0)) => error: RGB component range is between 0 - 255.

PS: Don't see this as an issue, more of a suggestion, I am simply testing for undefined now

@danburzo danburzo added the Feature New feature or request label Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants