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

delegateToSchema with abstract types does not work as expected #1920

Closed
saerdnaer opened this issue Aug 18, 2020 · 10 comments
Closed

delegateToSchema with abstract types does not work as expected #1920

saerdnaer opened this issue Aug 18, 2020 · 10 comments

Comments

@saerdnaer
Copy link
Contributor

saerdnaer commented Aug 18, 2020

How to stitch from an abstract to another abstract type?
In this case from AudioBrodcastService to MangoBroadcastInterface which are both implemented by MangoBroadcastService.

Demo-Repo: https://github.com/saerdnaer/stitching-demo/blob/master/src/schema/index.ts#L199-L216

I am trying to reduce and modernise an existing relay classic endpoint with following schema structure:

schema {
	query: RootQuery
	…
}

type RootQuery {
	node(id: ID!): Node
	nodes(ids: [ID]!): [Node]
	viewer: Viewer
}

type Viewer implementes Node {
	broadcastServices(after: String, first: Int, before: String, last: Int, filter: BroadcastServiceFilter, orderBy: BroadcastServiceSortOrder): BroadcastServiceConnection
	broadcastService(id: ID!): BroadcastServiceInterface
	broadcastEvent(id: ID!): BroadcastEventInterface
	…
}

which I transform into

schema {
  query: Query
}

type Query {
  node(id: ID!): Node
  nodes(ids: [ID]!): [Node]

  broadcastServices(after: String, first: Int, before: String, last: Int, filter: BroadcastServiceFilter, orderBy: BroadcastServiceSortOrder): BroadcastServiceConnection
  broadcastService(id: ID!): BroadcastServiceInterface
  broadcastEvent(id: ID!): BroadcastEventInterface
}

and then extend with

extend type Query {
	audioBroadcastService(id: AudioBroadcastServiceId!): AudioBroadcastService
	audioBroadcastServices(after: String, first: Int, before: String, last: Int, filter: MangoBroadcastServiceFilter, orderBy: MangoBroadcastServiceSortOrder): AudioBroadcastServiceConnection
}
interface AudioBroadcastService {
	id: ID!
	name: String
	description: String
	colors: JSON
	url: String
	epg(dayOffset: Int, day: Day, slots: [MangoEPGSlotKey]): [MangoEPGEntry]
}
extend type MangoBroadcastService implements AudioBroadcastService {
	colors: JSON
	description: String
	url: String
}
type AudioBroadcastServiceConnection {
	count: Int
	nodes: [AudioBroadcastService!]
	pageInfo: MangoPageInfo
}
enum AudioBroadcastServiceId {
	Bayern_1
	Bayern_2
	Bayern_3
}

When I now try to query all audioBroadcastServices I do not get any results, because the transform inside of delegateToSchema (see link in first line this description) remove all attributes as it does not realise that they also exist on MangoBroadcastService:

image

Request from delateToSchema for audioBroadcastServices{…} from gateway to source service:

{ 
viewer {
    __typename
  }
}

Request for broadcastServices{…} from gateway to source service:

{
viewer {
  broadcastServices: allBroadcastServices {
    nodes {
      id
      name
      __typename
      __typename
    }
  }
}

This bug is probably related to #751

@yaacovCR
Copy link
Collaborator

Have not fully looked at this, but seems similar to #1911, potentially fixed by #1912, check out canary there, maybe?

@saerdnaer
Copy link
Contributor Author

I upgraded to 6.0.19-alpha-fe84638d.0, but the problem still persists.

I think the special here is that the transform has to cast AudioBrodcastService to MangoBroadcastInterface, which are both implemented by MangoBroadcastService.

@yaacovCR
Copy link
Collaborator

I am basically stumped at first blush, as often. If you are able to minimize your reproduction, I can take a look. Right now, it is too extensive for me to quite wrap my head around

@yaacovCR
Copy link
Collaborator

Just looking at this again from a different angle, possibly the HoistField transform might help you to hoist fields from viewer up a level, if that is what you are trying to do.

If you could prune the gateway schema and source subschema to the minimum necessary to see the problem, that would help my aging brain!

Would love to help!

@yaacovCR
Copy link
Collaborator

And by minimum necessary, I think you need:

  1. a source subschema with: a single root query field, an interface type with one field, and an implementing object type with an additional field
  2. a gateway schema that adds: a new interface and a new root field with the new root field, and a resolver with the failing delegation.
  3. the query you are running
  4. the result you are expecting
  5. the result you are getting

You can help a fix move along by providing the above, in order of my personal preference:

  1. a PR with a failing test
  2. a code sandbox
  3. or a separate github repository, leaner than the one linked above

Really would love to help!

@saerdnaer
Copy link
Contributor Author

I created a simplified example which worked as expected, the problem is my approach to transform the classic schema into modern one: As the individual requests do not get properly translated from the gateway schema to the service schema – would have to wrap most requests with a selectionSet? named viewer – I my first approach I tried to do this by modifying the executor.

The HoistField transform has the problem this it only moves a single field and drops all arguments of the hoisted field. In my example item(id: ID!): ItemInterface gets transformed to item: ItemInterface

I would require a HoistType transform (basically a inverted version of WrapType) which merges all fields from a childType (in my case Viewer) into a targetType (in my case Query).

I cleaned the demo repo: https://github.com/saerdnaer/stitching-demo/tree/b9c3942903da2ec8a4b3838dd233c0e601ac275c/src/schema and tried to show my first problem with HoistField in following test: https://github.com/ardatan/graphql-tools/pull/1963/files

@saerdnaer
Copy link
Contributor Author

@yaacovCR delegateToSchema() does not support to extract wrapped fields? E.g. allowing using fieldname: ['viewer', 'item'] as parameter would also solve my problem...

@yaacovCR
Copy link
Collaborator

See #2040 in terms of allowing arguments

Turns out hoisting fields into a root type is more complicated than hoisting regular fields because you have to create new proxying resolvers. So it turns out that we do not yet support hoisting fields into new root types...

That is the next challenge, and then we can expand on this to hoist a type.

@yaacovCR
Copy link
Collaborator

yaacovCR commented Oct 6, 2020

Hoisting fields into root types will now be available on v7, check out #1935 for instructions on how to try the canary release.

Last remaining bit of functionality you requested is to be able to hoist multiple fields at once. That will be less of a priority, as you can programmatically just add multiple transforms in the meantime.

@yaacovCR yaacovCR added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Oct 6, 2020
@yaacovCR
Copy link
Collaborator

Released in v7

@yaacovCR yaacovCR removed the waiting-for-release Fixed/resolved, and waiting for the next stable release label Oct 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants