.clone does not preserve validity

[Elberet]

Summary:

import Tinycolor from 'npm:tinycolor2';
let invalidColor = new Tinycolor('');
let clonedColor = invalidColor.clone();
let shouldBeTrue = invalidColor.isValid() === clonedColor.isValid();
// however: shouldBeTrue === false!

When an invalid color is cloned as demonstrated, Tinycolor returns a new color using the other color's string representation (tinycolor.js#L213). However, since clone() does not check the subject's validity and toString() ends up outputting an RGB hex-string using the default rgb-values - again, without first checking the object's validity -, the result is typically a valid #000000 color.

While it is acceptable for clones to represent the same state while not being identical internally, the validity of the color object is very much an important part of it's state and should be preserved in clones.

[CrandellWS]

I can not make it work the way you show things

This is because how it is cloned...

clone: function() {
        return tinycolor(this.toString());
    }

What does seem to get the results you want is to actually pass the color object in as the parameter...

var invalidColor = new tinycolor('');

var clonedColor = invalidColor.clone();
var shouldBeTrue = invalidColor.isValid() === clonedColor.isValid();// not true


var copiedColor = new tinycolor(invalidColor);
var isTrue = invalidColor.isValid() === copiedColor.isValid();// true

[Elberet]

This is still an error in TinyColor, we're now just arguing whether it's in the code or in the documentation:

The code arguably subverts the expectations associated with the word "to clone", which I commonly understand to mean "to make an identical copy, independent of but indistinguishable from the original in all its observable properties".
Alternatively, the documentation neglects to mention this behaviour of .clone() and should instead recommend to use new TinyColor(otherColor) as the preferred way to copy colors, at which point .clone() should probably become deprecated.

Personally, I'd be fine with either option, but no clone() is better than a broken clone().

Actually... see tinycolor.js#L22: new TinyColor(color) where color is an instance of TinyColor does not create a copy at all, so .clone() is still the only provided way of copying a color object.

[CrandellWS]

I did not think that was gonna work and well idk... I think it should not work but it seems to

I think though it may be a problem with changing the color...I will have to check...

right that is not gonna help anything... my bad...

This could be improved for optimization though it at least works as desired...