Apollo-tooling: Making array object types optional may have been a mistake in flow types generation.

[epicallan]

In version 0.15 we had this in the test snapshots

{
      friends: ?Array< ( {
          __typename: \\"Human\\",
          // The name of the character
          name: string,
        }

In the most recent version of the lib you have flow types as such

friends: ?Array< ?( { __typename: \\"Human\\", // The name of the character name: string, } >,

the new change that makes the array object optional just introduces a lot of type / value checking complications.

When I cross check in the typescript generated types, the array object is not optional. The type is similar to what we had in v0.15 in flow types.

This makes me wonder whether this new change in v0.16 was intentional ? If it wasn't could we revert to the earlier version for this. I can put together a PR

[dperetti]

100% agree ! It's even worse when you have a Relay interface and you need to traverse edges and nodes, all optional (for no obvious reason)... and Flow gets lost even if you unwrap the values properly.

[dperetti]

This is crazy, it makes the whole thing barely usable !

[epicallan]

Its strange it hasn't got any attention from the library authors, maybe submitting a PR would do. Personally i just rolled back at 0.15 and I have not attempted to update since then.

[dperetti]

Now we know who the two users of apollo-codegen are ! 😅

[martijnwalraven]

@dperetti @epicallan: Yeah, a PR would be great!

[epicallan]

@martijnwalraven I have submitted PR #263

[martijnwalraven]

@epicallan: Looking at your PR, I realized this is actually expected behavior. Whether array elements are non-nullable is part of the schema, and in this case friends is defined as [Character], meaning an optional array of optional elements.

Since the server is allowed to send you null for any of the elements, that is something you want to make explicit in the generated types so you can deal with it appropriately. In many cases of course, an array element being null doesn't make sense. But that means the type should be fixed in the schema, changing friends to [Character!] here for example.

[epicallan]

Thanks @martijnwalraven that makes sense.

[lydell]

I consider it a feature that apollo-codegen follows the schema definition and adds ? to everything that can be nullable (so IMO this issue can be closed). I'm fine with the extra null-checking in the code I've written so far.

[martijnwalraven]

@lydell: Good to hear, closing this issue!