Change "grouped field set" to be a map of ordered sets

[benjie]

Extension to #1039 Currently:

type GroupedFieldSet = Map<ResponseName, Array<FieldSelection{ResponseName}>>

I propose that we defined "field set" to be an ordered set (rather than list) of fields that all share the same response name. This makes a lot of the spec clearer, e.g. it's much clearer why ExecuteField() should receive a "field set" (since they all share the same response name) rather than simply receiving "fields".

Essentially I propose:

// Constraint: every FieldSelection in a FieldSet must have the same ResponseName
type FieldSet{ResponseName} = OrderedSet<FieldSelection{ResponseName}>

type GroupedFieldSet = Map<ResponseName, FieldSet{ResponseName}>

This also makes the term "grouped field set" a lot clearer.

It also increases efficiency; consider this query:

query Q {
  animal {
    ...Cat
    ...Dog
  }
  animal {
    ...Cat
    ...Dog
  }
  animal {
    ...Cat
    ...Dog
  }
}
fragment Cat on Animal {
  lives
}
fragment Dog on Animal {
  wagsTail
}

Prior to this change:

1. ExecuteRootSelectionSet uses CollectFields to produce this grouped field set:

   const rootGroupedFieldSet = {
     "animal": [
       FieldSelection`animal { ...Cat ...Dog }`,
       FieldSelection`animal { ...Cat ...Dog }`,       
       FieldSelection`animal { ...Cat ...Dog }`,
     ]
   }
   

2. CompleteValue calls CollectSubfields to produce this grouped field set (assuming Cat):

   const catGroupedFieldSet = {
     "lives": [
       FieldSelection`lives`,
       FieldSelection`lives`,
       FieldSelection`lives`,
     ]
   }
   

   Note: visitedFragments doesn't prevent this because it doesn't work across sibling selections, only nested selections

If we were to have deeper nesting, this duplication would get much worse. Memory wise, this isn't a big deal, but looping-wise we're looping a lot more than we need to.

By explicitly making this a set, we remove this duplication. The phrase "if not already present" makes it clear that the first field selection "wins", which is important for ordering.

[yaacovCR]

2. Note: visitedFragments doesn't prevent this because it doesn't work across sibling selections, only nested selections

I believe the current specification has visitedFragments working across sibling selections, but that there is a proposal to limit the use of visitedFragments to work only for nested selections, to prevent other problems:

• https://github.com/graphql/graphql-spec/pull/1045

For what it's worth, I support #1045 as well as this change. :)

[benjie]

This was merged into #1039 and since merged into the spec.