Fix potential bug when an @interfaceObject has a @requires #2524
Conversation
👷 Deploy request for apollo-federation-docs pending review.Visit the deploys page to approve it
|
🦋 Changeset detectedLatest commit: eb7a22d The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| @@ -1598,9 +1598,18 @@ function addOneToKeyedUpdates(keyedUpdates: MultiMap<string, SelectionUpdate>, s | |||
| } | |||
| } | |||
|
|
|||
| function maybeRebaseOnSchema(toRebase: CompositeType, schema: Schema): CompositeType { | |||
There was a problem hiding this comment.
This behavior is slightly different than I would expect. If the type is undefined in the new schema, why return the original type rather than, say, undefined?
There was a problem hiding this comment.
I guess that was me being overly cautious and defaulting to what was essentially the prior behaviour before this method was added if something unexpected happened. But truly, we should always find the type or we're having a bug, so I ended up changing this to an assertion. Probably better to get a signal early if we introduce/have such a bug (returning undefined would push the problem to the caller, which in that case wouldn't have much better it can do than failing, so having an assertion directly kept the code simpler; this is all non-exported methods after all).
When an `@interfaceObject` type has a field with a `@requires` and the query requests that field only for some specific implementations of the corresponding interface, then the generated query plan was sometimes invalid and could result in an invalid query to a subgraph (against a subgraph that rely on `@apollo/subgraph`, this lead the subgraph to produce an error message looking like `"The _entities resolver tried to load an entity for type X, but no object or interface type of that name was found in the schema"`). The underlying reason is that the plan was mistakenly requesting the required fields for any object of the interface (corresponding to the `@interfaceObject` in question), instead of only requesting them only for only the implementation types it needed to. Not only is that inefficient in principle, but this lead to invalid fetches because the "rewrite" logic used to fixup the `__typename` for `@interfaceObject` under the hood was only rewriting the types of the implementation types in question (only expecting those to need rewrite, which technically was correct) but was mistakenly getting values for other implementation types.
pcmanus
force-pushed
the
bug-itfobj-requires
branch
from
9f67428
to
eb7a22d
Compare
When an `@interfaceObject` type has a field with a `@requires` and the query requests that field only for some specific implementations of the corresponding interface, then the generated query plan was sometimes invalid and could result in an invalid query to a subgraph (against a subgraph that rely on `@apollo/subgraph`, this lead the subgraph to produce an error message looking like `"The _entities resolver tried to load an entity for type X, but no object or interface type of that name was found in the schema"`). The underlying reason is that the plan was mistakenly requesting the required fields for any object of the interface (corresponding to the `@interfaceObject` in question), instead of only requesting them only for only the implementation types it needed to. Not only is that inefficient in principle, but this lead to invalid fetches because the "rewrite" logic used to fixup the `__typename` for `@interfaceObject` under the hood was only rewriting the types of the implementation types in question (only expecting those to need rewrite, which technically was correct) but was mistakenly getting values for other implementation types.
When an
@interfaceObjecttype has a field with a@requiresand the query requests that field only for some specific implementations of the corresponding interface, then the generated query plan was sometimes invalid and could result in an invalid query to a subgraph (against a subgraph that rely on@apollo/subgraph, this lead the subgraph to produce an error message looking like"The _entities resolver tried to load an entity for type X, but no object or interface type of that name was found in the schema").The underlying reason is that the plan was mistakenly requesting the required fields for any object of the interface (corresponding to the
@interfaceObjectin question), instead of only requesting them only for only the implementation types it needed to. Not only is that inefficient in principle, but this lead to invalid fetches because the "rewrite" logic used to fixup the__typenamefor@interfaceObjectunder the hood was only rewriting the types of the implementation types in question (only expecting those to need rewrite, which technically was correct) but was mistakenly getting values for other implementation types.