Relay: How to handle the "%future added value" in flow types generated enums

[HichamELBSI]

Hi everyone,

How to use a flow type generated enum for a known enum (see the use case below)

// ./src/events/__generated__/my_generated.graphql.js
...
export type EVENT_TYPES = ('activity' | 'formation' | '%future added value');
export type Event = {
     type: EVENT_TYPES
}
...

// ./src/components/icon.js
type IconProps = {
    type: 'menu' | 'arrow' | 'activity' | 'formation',
};

const Icon: React.ComponentType<IconProps> = ...

export default Icon;

// ./src/events/index.js

import type {Event} from './__generated__/my_generated.graphql.js';

type EventPageProps = {
     myEvent: Event
}

const EventPage = ({myEvent}: EventProps) => (
     <div>
               /* Flow error : 
                         Cannot create Icon element because string literal 
                         %future added value [1] is incompatible with string enum [2] in property type.     
              */
               <Icon type={myEvent.type} /> 
     </div>
)

[leebyron]

These are generated as a reminder that GraphQL services often expand in capabilities and may return new enum values. To be future-proof, clients should account for this possibility and do something reasonable to avoid a broken product. Flow types help us catch cases where that hasn't yet occurred.

[AugustinLF]

Hmmm, but couldn't you say that about any relay generated type? That we rely on the schema to be correct? If tomorrow the type property of Event becomes nullable, our code will fail where it shouldn't, right?

If the schema is updated because the the service expanded, it's up to me to refetch it and rerun the compiler, no?

[leebyron]

That is also true! I think this area of GraphQL is the least well defined at the moment however historically we've seen changes of what type a field returns (including becoming nullable) being considered as server breaking changes and thus avoided, while we've seen additions to enums not being considered server breaking changes and occurring (we now have a tool in graphql.js that detects this as a "dangerous" change). Another related example is a Union type adding a new possible type which can be returned - this hasn't historically been considered breaking but is dangerous for the same reason.

To bring the example closer to your code - if in the future you introduce a new kind of event and ship it, then your client may crash because it got undefined where it expected an icon. Relay's generated flow types are trying to help you out by pointing out this possible future scenario.

Right now this is always how this works, but I think if you really thought this wasn't useful then we would definitely entertain a PR which allows for configuration to exclude this "future added value" entry

[AugustinLF]

Thanks for your insight! It does indeed make sense to make such an option interesting, since in our case, we do have a total control on the server (so we would have to handle the new type of Event before the API is allowed to be updated), but it might not be the case for everybody.

I'll see if I can give a PR a try :)

[anfedorov]

if you really thought this wasn't useful then we would definitely entertain a PR which allows for configuration to exclude this "future added value" entry

It seems actively harmful if it breaks working code where other solutions exist. A type system enforcing defensive coding against "arbitrary future values" seems the opposite of what type systems usually do — enforce that there are no arbitrary values, but only those that are defined in the type system. That's the whole point of the

    default:
      throw ((impossible: empty) => {})(val)

construct, is it not?

[leebyron]

@anfedorov - what you say is definitely true when the source of truth for the Enum lives in the same codebase for which type-checking applies. At compile time all possible values for that Enum are known, so a type-checker should do exhaustiveness checking.

However this is not true when the source of truth for the Enum lives outside that codebase and the relationship between the two is one that is expected to be both long-lived and evolving. GraphQL's introspection allows you to capture a view of a type system at that specific moment in time, however there is nothing stopping that type system from evolving in the future. New values being added to enums is one such example of such an evolution. In this scenario a compile-time type checker can't know all possible values in the future. In the same way before a type-checker adds value by doing exhaustiveness checking, in this case a type-checker can add value by highlighting this real possibility of future unknown values.

[anfedorov]

Thanks for explaining, makes sense. It is true in our code base that "source of truth for the Enum lives in the same codebase" and we'd like to take advantage of relay-compiler 1.5 and also keep exhaustiveness checking. Is this possible sans @AugustinLF's #2368 or should we wait?