[Discussion] Enums/Unions and exhaustiveness

[alex35mil]

As a continuation of the discord discussion regarding enums/unions exhaustiveness.

Docs mention having exhaustive enums and unions is unsafe due to possible gaps between server/client releases. In the case of React Native or web apps with service workers, it makes sense. But in the case of traditional web apps, I would argue the risks are relatively low. And if an app implements version checking (eg, via headers), there is no risk at all.

Also, I have a case when the risk is minimal regardless of the nature of a client. On the backend, I have a macro that generates *Result unions. I'm fairly confident that these unions will never be extended, but still forced to handle #UnselectedUnionMember each time I dispatch a Result (quite often).

If in the case of Unions, #UnselectedUnionMember is an inconvenience. But in the case of enums, _ it might hurt. One of the main reasons we use GraphQL is the ability to sync interfaces between server and client, thanks to the GraphQL type system. I.e., when I extend an enum in the backend schema, I can be confident that after the schema sync, Rescript compiler would poke me in every single place where I need to review my code that depends on the extended enum. But when _ is added, it swallows all new constructors, so it requires manual searching to locate all the places where we depend on this enum to review them.

Proposal

Introduce PPX flags --exhaustive-enums --exhaustive-unions that would allow choosing a strategy of handling enums and unions on per application basis.

It makes sense to have a local attribute @exhaustive that makes unions/enums exhaustive for cases when in general, developers prefer to have a #UnselectedUnionMember seatbelt, but in some cases, it is not needed (like the example with generated Result unions).

What do you think?

[zth]

First; I agree this should be togglable. I think enums might already be possible to toggle globally via the Relay compiler and some option whose name I can't remember, but that's for "future proof enums". You can also set the behaviour per field via @rescriptRelayAllowUnsafeEnum.

Docs mention having exhaustive enums and unions is unsafe due to possible gaps between server/client releases. In the case of React Native or web apps with service workers, it makes sense. But in the case of traditional web apps, I would argue the risks are relatively low. And if an app implements version checking (eg, via headers), there is no risk at all.

Anecdotal, but I've actually worked at multiple places where the main product was a web app, where we'd regularly have a tail of 3+ month old web clients still making requests, because people never close the tab nor refresh the page.

But in the case of enums, _ it might hurt.

This was a trade off that was made a few years back. This is actually solved in V3 where enums are regular variants with a built-in catch all FutureAddedValue(_). So you get both, handling the catch all case and the compiler tells you where to handle new members.

So to sum it up, I agree, it should be togglable. We'd probably need a new option in the compiler config though.

[alex35mil]

Yeah, this is why we check backend version to prevent stale clients from pinging servers.

[zth]

Plan on seeing if I can tackle the union part next week. Are you on RescriptRelay v3 already?

[alex35mil]

Sorry, my notifications on GH are /dev/null. I'm actually on 1.0.4 at the moment. But TBH I'm thinking of switching to OpenAPI, as the amount of ceremonies when passing sum types through GraphQL is getting off the charts.

[zth]

Right, got it. We'll let this rest then.