Ensure that min/max of TimeScaleOptions can be a string

[etimberg]

Resolves #10134

[etimberg]

@kurkle @LeeLenaleee not sure if you have a better solution. It seems like TS doesn't override the min/max definitions from CartesianScaleOptions in the TimeScaleOptions definition.

The proposed solution here breaks new axis types and we have a test covering that

[StephanTLavavej]

It seems like TS doesn't override the min/max definitions from CartesianScaleOptions in the TimeScaleOptions definition.

Is this because TimeScaleOptions is defined as an intersection type with the & operator?

Chart.js/types/index.esm.d.ts

Lines 3178 to 3180 in aebbb5a

	export type TimeScaleOptions = CartesianScaleOptions & {
	min: string | number;
	max: string | number;

According to my understanding (as a TS novice with C++ background), if CartesianScaleOptions says that it has a min which is a number, and if an unnamed type (within the braces after the &) says that it has a min which could be a string or a number, then the intersection of those types has a min which is specifically a number.

(That is, properties that exist on both types are intersected. This might seem different from the behavior when the input types have distinct properties and the output type gains both properties - e.g. in the TypeScript example, Colorful with color and Circle with radius produces ColorfulCircle = Colorful & Circle with both color and radius. However, this can still be understood to be intersection behavior, because Colorful with the color property means "an infinite set of objects which have the color property and may have additional properties". Intersecting that with Circle, the "infinite set of objects which have the radius property and may have additional properties", produces a smaller infinite set of ColorfulCircles which have both properties (and possibly more).)

I am not sure what this implies for a fix. If any CartesianScaleOptions might actually be a TimeScaleOptions (i.e. substitutable, what I would call "base" and "derived" in C++), then it would seem that CartesianScaleOptions should specify string | number. Then LinearScaleOptions and LogarithmicScaleOptions could narrow that down to specifically number. Not sure if that would break other stuff.

[LeeLenaleee]

Instead of removing the min and max definition from the cartesianScaleOpts you could omit the min and max property when you assign the cartesianScaleOpts to the timeScaleOpts:

export type TimeScaleOptions = Omit<CartesianScaleOptions, 'min' | 'max'> & {
 // All timescale opts
}

Although seems like this has to be done for the categoryScaleOpts too then

[etimberg]

@StephanTLavavej that's a really helpful summary 😄. We are definitely going for CartesianScaleOptions as the base type so I went with the solution you suggested to have CartesianScaleOptions specify string|number and then narrow it down to number for the linear and logarithmic scale types.

I did also test the Omit<CartesianScaleOptions, 'min' | 'max'> and that worked as well so we also have it as a solution

[LeeLenaleee]

Since the base for min/max is now number | string it would be cleaner if we removed them from the CategoryScaleOptions and TimeScaleOptions

[LeeLenaleee]

Just tested this and not sure if this is the best way to fix it.
If I specify the y scale with only a min or max prop I can assign a string to them without issue, TS only starts to complain when I explicitly specify that the scale is linear.

So maby the Omit route is better since then it will also throw an error if you dont explicitly specify that the y scale is of type linear

[etimberg]

This sounds like a really good reason at v4 to require type to be specified and remove the default axis types once any settings are provided. All this pain to prevent one line of config.

I will push up the Omit solution now