Cache returns incorrect data when using aliases on different fields with the same name

[samkline]

Hi, we've noticed that when we use fragments to select different fields with the same name, Hermes does not behave correctly. It's easiest to illustrate with an example. This is from a black-box test I wrote around our cache:

it('should handle aliases on conflicting fields from fragments', () => {
    const query = gql`
      {
        foo {
          ... FirstFields
          ... SecondFields
        }
      }
      
      fragment FirstFields on First { id foo __typename }
      fragment SecondFields on Second { id bar: foo __typename }
    `;
    const cache = buildCache();
    cache.writeQuery({ query,
      data: { foo: [
          { id: 1, foo: 111, __typename: 'First' },
          { id: 2, foo: 222, __typename: 'First' },
          { id: 3, bar: 333, __typename: 'Second' },
        ] },
    });
    assert.deepEqual(
      cache.readQuery({ query }).foo,
      [
        { id: 1, foo: 111, bar: null, __typename: 'First' },
        { id: 2, foo: 222, bar: null, __typename: 'First' },
        { id: 3, foo: null, bar: 333, __typename: 'Second' },
      ],
    );
  });

And here's the test result:

      AssertionError: expected [ Array(3) ] to deeply equal [ Array(3) ]
      + expected - actual

       [
         {
           "__typename": "First"
           "bar": [null]
      -    "foo": [null]
      +    "foo": 111
           "id": 1
         }
         {
           "__typename": "First"
           "bar": [null]
      -    "foo": [null]
      +    "foo": 222
           "id": 2
         }
         {
           "__typename": "Second"
           "bar": 333
      -    "foo": 333
      +    "foo": [null]
           "id": 3
         }
       ]
      

This situation comes up when there are conflicting field types - e.g. we have some objects with a numeric ID, and others with a string ID. Using the @static annotation does work around the problem, but that's not possible in all cases.

[nevir]

Thanks for the detailed example here!

The trouble here is that hermes isn't currently aware of type unions (nor fragments with type conditions) :( So, when it sees the array there, it doesn't know to redirect fields for First typed results to FirstFields, nor Second typed to SecondFields.

It looks like the second fragment is winning (without any warning or error!), and it's interpreting the query as if it were:

{
  foo {
    __typename
    id
    bar: foo
  }
}

• The first two results get no value for foo, because hermes is expecting a value under bar, and ignores unknown fields in the payload
• The result for the last entry is "correct" hermes behavior for aliased fields, however, and probably unlikely to change (because hermes can return more fields than you ask for)

[nevir]

Until we support union types, I'm going to see if I can at least get an error in there to detect cases where fragments/queries disagree about field aliases

[nevir]

…and also warnings about type constraints not being supported.

[jamesreggio]

Just as a note, I added a (skipped, failing) test to the repo a while back that reflects this behavior: 01c7b34

We should re-enable it when we want to fix this.