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

fix: select node when a type is duplicated #745

Closed
wants to merge 4 commits into from

Conversation

Eomm
Copy link
Contributor

@Eomm Eomm commented Mar 1, 2022

Fixes mercurius-js/mercurius-gateway#31
Supersedes #744

When a Type is duplicated across multiples node, Mercurius define its service as null

typeToServiceMap[typeName] = null

Instead, its value should be dynamically chosen by the node that is running the query/mutation.

This is a draft PR because there is one test failing due to the service resolution that I did not solve yet: sometimes the duplicated logic should be ignored, based on the input query.

@Eomm
Copy link
Contributor Author

Eomm commented Mar 3, 2022

I think the logic that must be implemented to solve the failing tests is:

given one single type PageInfo, the gateway's resolver function must choose to execute:

  1. the simple resolver (aka (parent) => parent[path])
  2. the external resolution (aka calling another gateway's node)

The choice can be made based on the node's gql type, which is unknown at the moment because when the types are the same, it is merged/overwritten.

Right now, my change runs the 1. option every time, for this reason, some tests are failing.

duplicatedTypes[key][0] ||
null
return duplicatedType
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this to a top-level function?

@mcollina
Copy link
Collaborator

mcollina commented Mar 3, 2022

given one single type PageInfo, the gateway's resolver function must choose to execute:

Can you link to the resolver that is being called in this case?

@brandomeniconi
Copy link

Hi, we have the same issue as we are developing our new federated schema.
The only workaround we found is to prefix the duplicated types (es: UserPageInfo) so they are unique and don't get a null resolver, not ideal.

We already reversed most of Mercurius federation logic code to verify the bug, but there are still some grey areas. If you can guide us in what's missing, we can allocate some dev time to help close this.

@Eomm
Copy link
Contributor Author

Eomm commented Nov 3, 2022

@brandomeniconi I think to implement this logic #745 (comment)

The steps are:

  • when the gateway merges the same types UserPage, it should keep all the versions for each node
  • when the gateway has a type, it should use the type defined by the node itself

@Eomm Eomm closed this Jun 10, 2023
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

Successfully merging this pull request may close these issues.

Alias fails on federation if the type is declared in multiple nodes
3 participants