graphql-spec icon indicating copy to clipboard operation
graphql-spec copied to clipboard

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

Open benjie opened this issue 6 months ago • 1 comments

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.

benjie avatar Apr 25 '25 08:04 benjie