Allow to extend Series in types

[zefirka]

Hi @leeoniya! I've noticed that there are a lot of uPlot plugins that I wrote depends on solution where I just put some property into Series at initialization time and then I just read it in plugin or callback. For example I'm putting into series information about series unit type for correct formatting in units plugin, or putting into series reference to original series data to show it in tooltip after data transformation along with transformed series values e.g. after normalization.

So I thought it would be convenient to allow somehow to extend Series type. I can suggest to make Series type generic:

type Series<T = {}> = {...} & T

But it immediately makes to turn all series containing types in generics too, which is pretty much annoying to handle:

declare class uPlot<T = {}> {
	constructor(
		opts: Options<T>,
		data?: Data,
		targ?: HTMLElement | ((self: uPlot, init: Function) => void)
	);
	...

Other way is to do: class uPlot<S extends Series>, then we'll not touch Series. This approach too requires to update all Series-containing interfaces, but it's a little bit easier to handle.

What do you think?

[leeoniya]

Series, like many of uPlot's types, are interfaces, which can be extended without problems:

[zefirka]

Sure, but in your example uPlot itself doesn't know about MySeries and all inner callbacks and interfaces will alarm you about type inconsistency when you'll try write some plugin which reads series.foo or use some callback. You can see simplified example here.

Certainly many corners can be circumvented by typecasts but I think that developers try to avoid it. If we extend all Series containing interfaces the typings may come more complex, but developer experience will significantly be improved. Moreover, we can do it by saving total backward compatibility.

[zefirka]

I've made a little research and came to this:

• Example with extending <S = {}>
• Example with extending <S extends Series = Series>

As you can see example <S extends Series = Series> doesn't work with straightforward approach because of Series should know about type which extends itself. We can use some new types first contains only fields in Series that doesn't require extension and second type to combine all other fields. And define Series as a union of these types. You can look at example here. But this approach will require types refactoring.

I can handle both options (example 1 or example 3), but I think that example 3 is preferable.

[leeoniya]

what's the benefit of splitting the data and the handlers instead of using Partial<Series> or Pick<Series, ...> externally?

also, if we go this route, i would want all the top-level major interfaces to be extendable. at least Scale, Axis, Series, Band. not sure if making Options itself extendable would make this whole thing even more twisted, but that would be good as well.

if you wanna PR it, i'll try it out and see how it goes.

[zefirka]

I'm not sure that Partial<Series> will work well. I think Pick or splitting interfaces is more explicit and reusable, but not insisting for splitting.

also, if we go this route, i would want all the top-level major interfaces to be extendable. at least Scale, Axis, Series, Band. not sure if making Options itself extendable would make this whole thing even more twisted, but that would be good as well.

I was experimenting with types and think that we'll have to give the opportunity to reach out extended Series more or less everywhere (options - for callbacks, hooks - for plugins). I don't think that there is a reason to make Options extendable in sense of extendable Series, but in my experiments Options necessarily becomes Options<S extends SeriesData = SeriesData> to pass S to Series, Scales and all other interfaces where callbacks with self: uPlot argument exists.

if you wanna PR it, i'll try it out and see how it goes.

Great! The job is mostly routine, and if you'll grant permission I can make PR cuz I've already started experimenting and found that the result is worth it. It's really very nice when you as developer once say that you want extend uPlot series and in all your plugins and callbacks you can reach all properties in extended series.

[leeoniya]

let's leave them merged and rely on Pick, i don't want to bloat the exports for now.

i'm still a bit nervous about this changeset size/complexity, so don't be offended if this doesn't get merged. worst case scenario is you'll just use your own typings defs -- a nice benefit of having only external types and not generated from the source :)

[leeoniya]

someone also mentioned this, if you want to consider this route:

there is also module augmentation, that allows you to add to an extend interfaces used by modules https://www.typescriptlang.org/docs/handbook/declaration-merging.html#module-augmentation
(material ui promotes this as how to customise the theme https://material-ui.com/guides/typescript/#customization-of-theme )

[zefirka]

Hi @leeoniya! I've made some experiments, and you know, once you started to extending Series it inevitably brings a huge amount of changes in typing if you want to work well done. So, I'm not pretty sure that there is a really necessity to bring that to uPlot, at least now. And I decided to use .d.ts no my side.

But I prefer not close this issue, because I think in time a lot of people may go through this way and maybe there will be some solution someday.

[leeoniya]

the module augmentation i mentioned above appears to work great, can you try it?

https://codesandbox.io/s/zen-clarke-zcl58?file=/src/index.ts

[leeoniya]

i think this is resolved with module augmentation.