[types] Fix type errors in codebase

[MysteryBlokHed]

There are currently 176 type errors coming from the codebase itself. These are definitely worth fixing (not necessarily before v0.6.0, but at some point), so I'd like some feedback as to the best way to fix certain kinds of type-specific errors (i.e. type errors that aren't actually indicative of a problem with the JS logic).

A relatively common one is TS2322: Type 'number[]' is not assignable to type '[number, number, number]'. An example of code causing this can be seen here:

https://github.com/color-js/color.js/blob/c99665cb2dfb9e1cefb1f6c764a41b9af7cc89e2/src/adapt.js#L72-L74

Since the return value of multiplyMatrices is not specific enough, it causes the aforementioned error. There are a few ways I can think of fixing this.

The first would be to simply ignore the error:

// @ts-expect-error
return multiplyMatrices(env.M, env.XYZ);

The second would be to assert as the proper type ([number, number, number]):

return /** @type {[number, number, number]} */ (multiplyMatrices(env.M, env.XYZ));

And the third would just be to assert as any:

return /** @type {any} */ (multiplyMatrices(env.M, env.XYZ));

I find it annoying that the type assertions with JSDoc are so ugly, but I'm not sure whether it's worth completely ignoring type checks for a line instead with a @ts-expect-error directive. Thoughts?

[MysteryBlokHed]

cc @jgerigmeyer @lloydk

[jgerigmeyer]

My leaning would be option 2 (adding the correct type annotation) except in instances where it really can't be done accurately.

[lloydk]

My leaning would be option 2 (adding the correct type annotation) except in instances where it really can't be done accurately.

I'd go with option 2 as well

[lloydk]

I think we should try to fix as many type errors as possible before v0.6.0 so we catch any errors in the types that haven't been caught by the type tests.

[MysteryBlokHed]

Another issue: there are some instances in the code where we see something like this, where space is of type string | ColorSpace:

space = ColorSpace.get(space);

In most cases, TypeScript correctly realizes that the type is now narrow. However, there are some situations where it doesn't realize this. For example, here:

https://github.com/color-js/color.js/blob/c99665cb2dfb9e1cefb1f6c764a41b9af7cc89e2/src/toGamut.js#L261-L272

This function is defined within another function, and references the space variable from the local scope. However, since it is technically possible for this function to be called while space is still a string, line 263 causes errors.

One way I can think of fixing this would be to add an assertion function somewhere (probably a static method on the ColorSpace class) that does this:

function assertSpace(x: any): asserts x is ColorSpace {
    if (!(x instanceof ColorSpace)) {
        throw new TypeError("Type is not ColorSpace");
    }
}

And then add a call to that function where necessary:

function clip (_color) {
    // \/ Guarantees that `space` is still ColorSpace
    assertSpace(space);
    const destColor = to(_color, space);
    const spaceCoords = Object.values(space.coords);
    // ...
}

But it is somewhat annoying to add a function call when we as developers just know that space will never be a string when this is called.

The other option would be to add an inline typecast everywhere it's required, like this:

function clip (_color) {
    const destColor = to(_color, space);
    const spaceCoords = Object.values(/** @type {ColorSpace} */ (space).coords);
    // ...
}

That doesn't add any overhead, but it is a bit noisy to read/write.

This issue only happens a few times in the codebase, so it might just be worth adding the few typecasts required and then moving on, but I figured I should ask.

[lloydk]

I prefer the inline typecast