[RFC] What types should we make private/public?

[m4theushw]

At this moment, we're exporting all the types we use internally, but should we continue with this approach or make some of them private? This discussion started in #1778 (comment).

In the core, the same problem was raised, but we agreed to keep the accessory types public. The rationale was that getting them via props (e.g. GridColDef['cellClassName']) created more complexity than using an export: mui/material-ui#26187 (comment)

The main reason for me to keep them public is documentation. TypeDoc only sees what is exported. If we make some types private, we won't be able to describe the real values accepted by a prop, like below:

[oliviertassinari]

This discussion started in #1778 (comment)

@m4theushw I have resolved the comment as it's not really important. The biggest value I saw was an exported type that could have been inlined, a type that is currently not easy to discover as a developer (maybe a better doc can solve this?).

In the core, the same problem was raised, but we agreed to keep the accessory types public.

In mui/material-ui#26187, for me, the conclusion was that: it's better without these types but it's a breaking change, we have already done too many for v5. Regarding Sebastian's point, yeah, it's definitely a downside, but what's the alternative? Expose a type for all the props of the components? Does it scale? I fail to envision it.

The main reason for me to keep them public is documentation.

I don't understand this part. Shouldn't all the public APIs be documented and vis-verca?
Regarding TypeDoc, I assume that if a method is using the type in its signature, it can access it? But yeah, spectrum makes it public:

---

@m4theushw What's the advantage of using a GitHub discussion over GitHub issues? My assumption is that the discussion label is enough to isolate the different issues. Maybe you are looking for the threaded feature? However, how do we close GitHub discussions (it seems that only the author can)?

[m4theushw]

In mui/material-ui#26187, for me, the conclusion was that: it's better without these types but it's a breaking change, we have already done too many for v5. Regarding Sebastian's point, yeah, it's definitely a downside, but what's the alternative? Expose a type for all the props of the components? Does it scale? I fail to envision it.

@oliviertassinari If we start making types inline first, and only creating aliases for when there's two or more usages of them, then it will scale well. The inverse is not true, as once a type is exported and we don't want it anymore that will require a major release to remove it.

I don't understand this part. Shouldn't all the public APIs be documented and vis-verca?
Regarding TypeDoc, I assume that if a method is using the type in its signature, it can access it? But yeah, spectrum makes it public:

It can access but it will not generate documentation for the type if it's not exported. It works in a lazy way, doesn't explore the type to discover more types.

What's the advantage of using a GitHub discussion over GitHub issues? My assumption is that the discussion label is enough to isolate the different issues. Maybe you are looking for the threaded feature? However, how do we close GitHub discussions (it seems that only the author can)?

We decided to use it during the retrospective, to avoid having off-topic discussions in PRs. We could open RFCs issues as well. I can't close this discussion because it's open ended. To close it, its category should be one that follows the question / answer format.

[oliviertassinari]

I like that.

I have changed the type of the category to "Question / Answer" as we are looking for a decision on how to design the public API 😁

[DanailH]

Ok, as far as I can see we at least can agree on one point for now - we need to have documentation via TypeDoc, so not exposing anything is not an option, but we can have inline types if they are used in only one place. So basically if we are going to use the type in a single place then inline it, if it's used in more than one place then abstract it. If we can agree to that it would be a great start.

[dtassone]

Not sure I totally agree.
A counter-example would be for React component props.
I would expect them to be used once in some cases, and be exported in their own type. Don't you think?

[m4theushw]

I would expect them to be used once in some cases, and be exported in their own type. Don't you think?

But you still can get the type for a prop that uses inline types with Component['propName'].

[dtassone]

IMO it helps users to have all types available as

1. it allows them to apply internal types in their own code. Not publishing types for me means adding holes to this matter.
2. If they use our type, then they will get compile issue if we did a change and so migrate in an easier way. If they use their own type mapping ours then, they might get no issues or runtime issues.
3. Definition type is a sort of documentation, I always control-click to go to the type def to understand what parameters a function might need. So if the type is missing there, and it's mapped to any, it won't be that convenient.

Another option would be to mark some types or props as unstable or experimental in typeDoc so ppl are aware that it can change soon.

[oliviertassinari]

2. If they use our type, then they will get compile issue if we did a change and so migrate in an easier way

If we assume that the definition of a "private type" is a type that describes a data structure that is never exposed publicly, then I fail to understand the case consider. Won't this case never happen? My assumption is that it will always be: we change an internal type, a developer was using it, it breaks his code, he realizes that the type he used has nothing to do with his use case, and create a new custom type? The output being: a new type, not a new implementation.

3.Definition type is a sort of documentation

I think that we all agree on this one. If a method is public, it needs types and these types should be public. It doesn't seem to rule out: inlining nor not exposing internal types, not encouraging to use Foo['bar'] to access the type.

[dtassone]

Quick one to add to the debate:
Would you expose getInitialGridFilterState function ?

[oliviertassinari]

I think what we want is the initial value of the controlled mode of a filter model. Not what we store in the state. IMHO making the assumption that it's the same can break (maybe we will add a cache?).

👍 for a new getInitialGridFilterModel and make getInitialGridFilterState private

[dtassone]

the model and the state are the same

[oliviertassinari]

Actually, it makes me think a lot about #820. If we refactor the state it only stores the raw input data (no cached values or derived states), with direct mapping (and a commitment to keep it) to the controllable state, then yes, maybe getInitialGridFilterState can work. Still, the name could be confusing, the developer wants its initial filter model, not the initial filter state.

[dtassone]

well it depends what they do. Both can be useful. One for the case above, and the other for the case you described

[flaviendelangle]

This was resolved.

We have a new getDefaultFilterModel methods and the state init methods do not exist anymore.