Hmm. I'm not sure I like this.

Could we instead make .round() use a simple Math.round() function on the components, removing the precision argument, and then introduce a toFixed() method that takes a precision argument and returns an array of strings whereby each component has had .toFixed() called on it?

That would satisfy all the usecases without being incredibly broken. I can't imagine a usecase where "round too precision and then perform some more math on it" makes any amount of sense, but the "round to fixed precision as a string" definitely does.

It'd still be a breaking change but at least more amicable for people to migrate to this less broken behavior.

I can't imagine a usecase where "round too precision and then perform some more math on it" makes any amount of sense

It happens indirectly through normal colour manipulation. Take the rgb implementation in your library: you round because, well, rgb colours don't need to be kept at high precision. But then if you lighten or darken that colour a number of times (e.g. you generate a palette from a single rgb), those computations will be done on a less-accurate value and will accumulate accuracy errors every time they're rounded (i.e. every time a rgb is created from another rgb).

To make sure I understand you well, you'd like me to:

• Reintroduce round() but make it round to integer without a precision argument
• Add toFixed which returns strings with a precision argument
• Keep the changes made to string output (i.e. make it use toFixed internally)?
• Reintroduce the round() calls where they were previously?

But then if you lighten or darken that colour a number of times (e.g. you generate a palette from a single rgb), those computations will be done on a less-accurate value and will accumulate accuracy errors every time they're rounded (i.e. every time a rgb is created from another rgb).

Right, I agree. I was referring more to the fact that .round() allows rounding to an arbitrary decimal precision using toFixed(), but the current implementation then parses that back into integers and stores them internally, for which I struggle to find any valid usecase.

I can't imagine a case where that's useful outside of toString()-like behavior, as performing math on e.g. 5.4 instead of 5.432483 doesn't have any benefit from what I can think of, so the toFixed() call should only be used if returning strings, because otherwise what's the point.

round() should just do that - round, internally, to an integer, using Math.round(). Then toFixed() can return an array of values for each channel that have had toFixed() called on them - i.e., an array of strings. In fact I'd be hesitant to even include toFixed() at all. toString() could probably take an optional argument for the precision and it'd use toFixed() before handing them to color-string or something like that.

That'd satisfy most of the usecases, I think.

tl;dr any op that results in a string (e.g. toFixed()) should not affect internal state whatsoever, and should only be used in the context of stringizing the color value. .round() shouldn't take an argument and should just be effectively (pseudocode) this.channels = this.channels.map(Math.round). Nothing more.

Then no more accumulated errors.

@Qix- I've reworked the commit so that now, round() uses Math.round() and a toFixed method returns a string array. I've also added more unit tests and documented rounding limitations.

Hi @Qix-, do you know when you'll have time to review this PR again? I'd love to get rid of the hotfix I maintain on my end as a consumer of the library :)