-
Notifications
You must be signed in to change notification settings - Fork 6
fix: always create field edges to current graphs field type #151
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
Conversation
@provides(fields: "bbbbbbbbbbbbb { aaaaaaaaaaaa { someField } }") | ||
} | ||
|
||
type AAAAAAAAAAAAA @key(fields: "id") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming the type in all subgraphs fixes the issue... So it must be some issue with the order of things...
- AAAAAAAAAAAAA
+ AAAAAAAAAAAA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, if the field had a shorter name we attempted to create a edge from BBBBBBBBBBBBB.aaaaaaaaaaaa
to SomeInterface/b
instead of AAAAAAAAAAAAA/
. 🤡
The changes in src/supergraph/validation/rules/satisfiablity/graph.ts
should ensure we always create the field edges to the subgraph defined field type instead.
type Post @key(fields: "id") { | ||
id: ID! | ||
author: User @provides(fields: "userId") | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -1381,7 +1381,10 @@ export class Graph { | |||
} | |||
} | |||
|
|||
const outputTypeName = stripTypeModifiers(field.type); | |||
// We always want to connect it to the field type within our graph not the field type from another graph/supergraph | |||
const fieldType = field.byGraph.get(this.id)?.type ?? field.type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @theguild/[email protected] ### Patch Changes - [#151](#151) [`f9b9908`](f9b9908) Thanks [@n1ru4l](https://github.com/n1ru4l)! - Fix issue where the satisfiability check raised an exception for fields that share different object type and interface definitions across subgraphs. - [#152](#152) [`e4440a1`](e4440a1) Thanks [@n1ru4l](https://github.com/n1ru4l)! - Fix issue where scalar type marked with `@inaccessible` does not fail the composition if all usages are not marked with `@inaccessible`. Composing the following subgraphs resulted in an invalid supergraph instead of failing the composition. ```graphql # Subgraph A extend schema @link( url: "https://specs.apollo.dev/federation/v2.9" import: ["@inaccessible"] ) type Query { a: Foo } scalar Foo ``` ```graphql # Subgraph B extend schema @link( url: "https://specs.apollo.dev/federation/v2.9" import: ["@inaccessible"] ) type Query { b: String } scalar Foo @inaccessible ``` Now it correctly raises a composition error with the message `Type "Foo" is @inaccessible but is referenced by "Query.a", which is in the API schema.`. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
No description provided.