normalizer fails for simple lists that are not ordered

[valkum]

Hello,

with the current main version, we observe an (it seems) undocumented behavior when using simple lists that are not ordered.

This happens when the same node with a list field has different orders of elements in these lists. We have some lists that are small and where pagination doesn't make sense; thus they are not a connection.

Simple example:

{
  "viewer": {
    "id": "user",
    "orgs": [
      {
        "id": "org1"
      },
      {
        "id": "org2"
      }
    ]
  },
  "latestComment": {
    "author": {
      "id": "user",
      "orgs": [
        {
          "id": "org2"
        },
        {
          "id": "org1"
        }
      ]
    }
  }
}

This would result in

ected warning in callback: RelayResponseNormalizer: Invalid record. The record contains references to the conflicting field, node and its id values: org1 and org2. We need to make sure that the record the field points to remains consistent or one field will overwrite the other.

Simple test case based on existing types in the test package.

it('should not warn in __DEV__ if a single response contains conflicting linked fields which are not sorted', () => {
    const BarQuery = graphql`query RelayResponseNormalizerTestListQuery {
      me {
        neighbors {
          id
          neighbors {
            id
          }
        }
        parents {
          neighbors {
            id
          }
          id
        }
      }
    }`;

    //
    // we -> neighbors -> Mother and Father -> neighbors -> 4 and 5
    //
    const payload = {
      me: {
        id: '1',
        __typename: 'User',
        parents: [
          { // Father
            id: '2',
            neighbors: [
              {
                id: '4'
              },
              {
                id: '5'
              },
              {
                id: '1'
              }
            ]
          },
          { // Mother
            id: '3',
            neighbors: [
              {
                id: '4'
              },
              {
                id: '5'
              },
              {
                id: '1'
              }
            ]
          }
        ],
        neighbors: [
            { // Father
              id: '2',
              neighbors: [
                {
                  id: '4'
                },
                {
                  id: '5'
                },
                {
                  id: '1'
                }
              ]
            },
            { // Mother
              id: '3',
              neighbors: [
                {
                  id: '4'
                },
                {
                  id: '5'
                },
                {
                  id: '1'
                }
              ]
            }
          ]
        }
      };
    const recordSource = new RelayRecordSource();
    recordSource.set(ROOT_ID, RelayModernRecord.create(ROOT_ID, ROOT_TYPE));
    
    normalize(
      recordSource,
      createNormalizationSelector(BarQuery.operation, ROOT_ID, {
      }),
      payload,
      defaultOptions,
    );
  });

[captbaritone]

Thanks for the report. We currently expect all GraphQL results to be stable within a single request. GraphQL Spec presumably allows this degree of instability, so I see where you're coming from. This warning is generally quite useful. Do you have a suggestion on ways we could enable your use case without sacrificing the warning for the more common case where results are stable?

[valkum]

Thanks for your fast answer. For once, I guess it would be beneficial if this requirement would be stated in the docs. It also would be very helpful if the warning would be a bit more verbose, or in the best case would link to a place in the docs.
While debugging this, the We need to make sure that the record the field points to remains consistent or one field will overwrite the other. wasn't quite helpful.

Regarding the stable response:
Having this requirement with your own GraphQL server is no problem (performance-wise we only emit small lists as non-connections, and thus it's ok to sort them server-side).
But if you want to use Relay with a third-party GraphQL API, these kinds of Relay-specific contracts might be hard to enforce.

I am not 100% familiar with Relay internals, so the following statements might be wrong.
I think the order of items in the store is the only thing that is problematic here, right?
Fields of items (e.g. additional fields of the org node type in the example) would still be normalized correctly (based on their ID). But because the list fields (e.g. orgs) are flattened to a single list in the store, there is no authoritative order.
The only thing I can think of would be the following optimization (based on my assumption that the list record (e.g. orgs), is the problem):

If you resolve the same field twice in a query, normalize the first occurrence. If the second occurrence has the same nodes, based on a set derived from the list, don't overwrite the list record during normalization. Continue with the normalization of the items in the list.