Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intermediate interface support for document builder #1275

Closed
cosmopetrich opened this issue Nov 26, 2024 · 6 comments · Fixed by #1276
Closed

Intermediate interface support for document builder #1275

cosmopetrich opened this issue Nov 26, 2024 · 6 comments · Fixed by #1276

Comments

@cosmopetrich
Copy link

cosmopetrich commented Nov 26, 2024

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
Copy link
Author

It looks like getInterfaceImplementors only searches through kindMap.list.OutputObject but not kindMap.list.Interface, which would explain the current behaviour.

export const getInterfaceImplementors = (typeMap: KindMap, interfaceTypeSearch: GraphQLInterfaceType) => {
return typeMap.list.OutputObject.filter(objectType =>
objectType.getInterfaces().filter(interfaceType => interfaceType.name === interfaceTypeSearch.name).length > 0
)
}

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
Copy link
Member

@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 #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
Copy link
Author

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 😅.

@jasonkuhrt
Copy link
Member

Hey just wanted to loop back here and say I've shipped an iceberg-of-a-PR #1272 and can now direct my attention to this issue. Unblocking users is my priority so I'll be working on this next. I would like to ship the feature this week.

@jasonkuhrt jasonkuhrt pinned this issue Dec 2, 2024
@jasonkuhrt

This comment was marked as resolved.

@jasonkuhrt
Copy link
Member

Thanks for your patience @cosmopetrich! If things aren't working in the newest release, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants