✅ Deploy Preview for colorjs ready!

Name	Link
🔨 Latest commit	68d9f69
🔍 Latest deploy log	https://app.netlify.com/sites/colorjs/deploys/6775a8f52c5c3e000895c4c8
😎 Deploy Preview	https://deploy-preview-628--colorjs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Two fixes that come out of this discussion: #593

1. The code finds a match for a custom parse() function, but only exited the inner of two loops. Here it exits early via break loop;

I added a suggestion for a more clear name, since both are loops.
Btw this is an API change too: previously the last parseable format won, now the first one wins. I think it's a reasonable tradeoff, just want everyone to be clear that this is what's happening.

This was not only causing unnecessary iterations of the loop, it was causing issues with user ColorSpaces with {type:"custom"}.

Why was it causing issues with custom formats? If changing the order fixes these issues, it sounds like a fix by coincidence, not a robust fix.

2. The addition of ?. in this line:

let color = format.parse?.(env.str);

so that "custom" ColorSpace formats don't require a parse property. The discussion linked above creates a custom ColorSpace with a default format that has a custom serialize() property/function, but doesn't need a parse property.
The current code requires it to have a dummy parse() property/function that returns falsy. This change removes that requirement and prevents the throwing of unnecessary errors.

Rather than encoding format-related logic at the point of usage, it seems better to include a parse() function in the base Format class: https://github.com/color-js/color.js/blob/main/src/Format.js

It is worth noting that there is only one built-in space with custom formats: sRGB, and it's the near the end of ColorSpace.all. Is there a need to iterate over any other color spaces?

We can't know what plugins people may have, and it doesn't seem like a good idea to couple this kind of information there unless we can generate it (but then cache invalidation becomes a concern).

I added a suggestion for a more clear name, since both are loops.

I committed your suggestions. It's still failing the lint tests. I don't think it has anything to do with these 3 commits.

Why was it causing issues with custom formats? If changing the order fixes these issues, it sounds like a fix by coincidence, not a robust fix.

It has been a while since then... I know I had to do a lot of fiddling around to get my color space to work. I should have been more specific in the original PR, from whence that text originates.

Rather than encoding format-related logic at the point of usage, it seems better to include a parse() function in the base Format class: https://github.com/color-js/color.js/blob/main/src/Format.js

I think that's above my pay grade at this time.

Yeah don't worry about the lint tests right now.

I think that's above my pay grade at this time.

It's literally just adding this in the Format class:

parse() {
	return null;
}

You'd need to test whether it works though.

I created Format.prototype.parse() and removed the ?. that I had added in parse.js. In my test it works. I no longer specify a parse property in my format. I dummied up a JSDoc comment. Please edit as appropriate.

Why was it causing issues with custom formats? If changing the order fixes these issues, it sounds like a fix by coincidence, not a robust fix.

After stepping through the different versions of this, there is nothing my current code that depends on this change. I have been importing /dist all along. In my case, the extra iterations are pointless, but I'm returning null from parse(). If you were to inherit from srgb, as I do, and, for whatever reason that I can't foresee, you want to override the parsing of named or hex colors or whatever other format, then this PR creates a bug instead of fixing one. As for non-srgb "custom" spaces I don't know what they would be or if they would inherit from other non-srgb "custom" spaces. AFAICT the only way to ensure that a unique version of parse() runs is to use a unique formatId.

If there was or is another issue, maybe a description of it is buried in the original discussion #593. Otherwise I can understand if you'd prefer to back out the changes to parse.js and let this PR be about the default parse() method.

It occurs to me that if you want to preserve the behavior of using the last match, then it would be best to iterate over the registry in reverse. All the user color spaces are at the end of the list, and srgb is much closer to the end of the built-in spaces than the front. Not that it's a huge list, but it seems silly to iterate over all those built-in spaces that will never be tagged as "custom".