[charts] Improve exposed types

[bernardobelchior]

At the moment, the exposed types aren't very user-friendly.

From my point of view, there's two problems we need to solve:

1. Expose simpler types, and
2. Expose flatter types

Expose Simpler Types

For example, if you want to define a variable that satisfies the x-axis of a bar chart, you can't do:

const xAxis = { } satisfies XAxis;

Instead, you can do:

const xAxis = { } satisfies MakeOptional<AxisConfig<ScaleName, any, ChartsXAxisProps>, 'id'>[]

Which is the type that is defined in UseChartCartesianAxisParameters. The problem is that MakeOptional is internal, so external users can't access it properly.

Another cleaner but less beginner-friendly option is:

const xAxis = {} satisfies UseChartCartesianAxisParameters['xAxis'][number];

But this is, in my opinion, far from ideal.

Expose flatter types

Currently, if you Cmd/Ctrl+Click (at least on my IDE) in BarChart's xAxis prop, you get redirected to this interface definition:

export interface UseChartCartesianAxisParameters {
  /**
   * The configuration of the x-axes.
   * If not provided, a default axis config is used.
   * An array of [[AxisConfig]] objects.
   */
  xAxis?: readonly MakeOptional<AxisConfig<ScaleName, any, ChartsXAxisProps>, 'id'>[];
  // ...
}

This type isn't very beginner friendly and requires understanding of four separate types: MakeOptional, AxisConfig, ScaleName and ChartsXAxisProps.

In my opinion, we should try to make the exposed types flatter and derive our internal representation from them, instead of the other way around.

Another option would be to see if there's some TypeScript utility that allows us to flatten types when building. That way we could keep our current state, but the external facing types would be easier to read. This would likely require less work with refactoring types, but I'm not sure if such a tool exists.

@alexfauquette @JCQuintas curious to hear your thoughts.

[JCQuintas]

I agree with this 😄

Another issue we currently face is that it is quite cumbersome to know and maintain which types are exported, so if we go this way, we can have an improved pattern for these cases.

Eg: Xxx.types.ts vs Xxx.internal.types.ts

[bernardobelchior]

I agree with this 😄

Another issue we currently face is that it is quite cumbersome to know and maintain which types are exported, so if we go this way, we can have an improved pattern for these cases.

Eg: Xxx.types.ts vs Xxx.internal.types.ts

With a quick search I found this. It seems we could use annotations such as @internal to verify that we don't accidentally expose internal types. Another good check would be to ensure that we don't export types that rely on @internal types.

[flaviendelangle]

We are generating one JSON file per package that list all the exports (all the root ones).
It's not super advanced and clearly third party solutions would do a beter job here, but that's what I'm using to track new exports / removed exports in PRs.

For the file structure solution, Base UI namespace solution works nicely to show what is exported just by looking at the code (everything in the namespace is exported, everything outside is private).
But if we don't want to migrate to namespaces just yet (it's a massive BC), splitting in two files as @JCQuintas described is probably the easies solution.

[bernardobelchior]

We are generating one JSON file per package that list all the exports (all the root ones). It's not super advanced and clearly third party solutions would do a beter job here, but that's what I'm using to track new exports / removed exports in PRs.

Yeah, I think tracking added and removed exports is solved already by that, but I think there's still some improvements that we can do in other areas.

For the file structure solution, Base UI namespace solution works nicely to show what is exported just by looking at the code (everything in the namespace is exported, everything outside is private). But if we don't want to migrate to namespaces just yet (it's a massive BC), splitting in two files as @JCQuintas described is probably the easies solution.

Yeah, that works. I wonder if we can use a lint rule for this as well.

[flaviendelangle]

Yeah, that works. I wonder if we can use a lint rule for this as well.

A lint rule that forbid importing files from ***/**.internal.types.ts in index.ts files should be easy to add and do most of the job.

[alexbalonperin]

Your example is spot on. I ended up creating an alias for that exact type in my code so that I don't have to import every subtypes every time. It would be nice to expose a simpler type even if it's just a type alias provided by the lib. It would just feel a lot cleaner too.

Also, it would be great to avoid using "any" at all. Just using a type parameter T (or a more explicit name) would already ensure that users don't mistakenly modify the type midway in their code and end up with a runtime exception. Let's say that the type for axis elements is type AxisElement = { label: string, value: O } where O is whatever object. If T is the type parameter for the x axis element, the user can't pass a string for the axis elements by mistake. Now the tradeoff with this approach is that you are fixing the type to be the same for all x axis elements both within the same axis and across the different x axis. I personally think this is worth it but I can see how people might argue against it. Maybe there is some type gymnastics that would allow this use case. 🤔