graffle-js / graffle

Simple GraphQL Client for JavaScript. Minimal. Extensible. Type Safe. Runs everywhere.
http://graffle.js.org/
MIT License
5.89k stars 310 forks source link

Intermediate interface support for document builder #1275

Open cosmopetrich opened 5 days ago

cosmopetrich commented 5 days ago

Perceived Problem

Please forgive me if I am mistaken, but it appears that Graffle's generated client does not directly support 'intermediate' interfaces when using the document builder. As an example, consider the schema below.

interface Grandparent {
  a: Int
}

interface Parent implements Grandparent {
  a: Int
  b: Int
}

type SiblingX implements Grandparent & Parent {
  a: Int
  b: Int
  x: Int
}

type SiblingY implements Grandparent & Parent {
  a: Int
  b: Int
  y: Int
}

type Query {
  grandparents: [Grandparent]
}

In raw GraphQL it is possible to write a query fragment on the "middle" interface, Parent.

query test {
  grandparents {
    ...on Parent {
      b
    }
  }
}

However, given the Schema above, graffle@8.0.0-next.133 would generate GrandParent.__on_SiblingX/GrandParent.__on_SiblingY properties but not a GrandParent.__on_Parent property. This makes obtaining b more verbose than it otherwise would be.

// Actual invocation as of graffle@next-133
graffle.query.grandparents({
  ___on_SiblingX: {
    b: true
  },
  ___on_SiblingY: {
    b: true
  },
  // Repeat for every other 'Sibling' type
});

// Ideal invocation
graffle.query.grandparents({
  __on__Parent: {
    b: true
  },
});

Is this something that the document builder could feasibly support? Or have I just missed an obvious way to do it?

In any case, thankyou for your work on Graffle. It has been fantastic to use so far.

cosmopetrich commented 5 days ago

It looks like getInterfaceImplementors only searches through kindMap.list.OutputObject but not kindMap.list.Interface, which would explain the current behaviour. https://github.com/graffle-js/graffle/blob/a78ebbeb31805701c82f54b038d2c81eba52a74d/src/lib/grafaid/schema/KindMap/_.ts#L111-L115

Munging that to the below causes the __on_* interface parameters to be generated, though I'm not currently able to run Graffle's test suite or the full build process to check if there's any nuance that I'm missing.

export const getInterfaceImplementors = (typeMap: KindMap, interfaceTypeSearch: GraphQLInterfaceType) => {
  const allTypes: Array<Grafaid.Schema.ObjectType|Grafaid.Schema.InterfaceType> = [...typeMap.list.OutputObject, ...typeMap.list.Interface]
  return allTypes.filter(type =>
    type.getInterfaces().filter(interfaceType => interfaceType.name === interfaceTypeSearch.name).length > 0
  )
}
jasonkuhrt commented 4 days ago

@cosmopetrich Hey thanks for raising this, indeed its a missing feature! I'm glad there's a user use-case for it as I'm aware of the feature but don't have direct use-cases for it myself.

I'll take this work on after I resolve the current modular transport https://github.com/graffle-js/graffle/pull/1272.

Meanwhile, a PR is welcome but I'm not sure how deep this feature will run yet in the generator and type system so it may be hard for someone else than me to jump in and ship this.

cosmopetrich commented 4 days ago

Yeah I suspect it isn't at all a common requirement. The only reason I came upon it is that I'm operating against a schema for something that makes heavy use of polymorphism. Even in my case it's far from a huge problem as there are currently only a small handful of siblings for any one interface.

The change is definitely more involved than the kludge in my earlier comment, which triggered some Typescript errors when I got a chance to test the generated client. I'll look further into it if I am able, though my weak JS/TS is probably a bigger hurdle than the complexity of the generator 😅.