graphql-tools icon indicating copy to clipboard operation
graphql-tools copied to clipboard

stitchSchemas() removes fields that are annotated with @computed from stitchedSchema causing our server to fail

Open tommysun777 opened this issue 3 years ago • 4 comments
trafficstars

Issue workflow progress

Progress of the issue based on the Contributor Workflow

  • [ ] 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

    Make sure to fork this template and run yarn generate in the terminal.

    Please make sure the GraphQL Tools package versions under package.json matches yours.

  • [ ] 2. A failing test has been provided
  • [ ] 3. A local solution has been provided
  • [ ] 4. A pull request is pending review

Describe the bug

We are using @computed in one subschema referencing a field from another

Our subschema 1:

scalar ItemId
scalar ItemId2
scalar ItemId3
scalar AField

type Query {
  _service: _Service!
  _item(input: ItemInput!): Item! @merge
}

input ItemInput {
  itemId: ItemId!
  itemId2: ItemId2!
  aField: AField
}

type Item @key(selectionSet: "{ itemId, itemId2 }") {
  itemId: ItemId!
  itemId2: ItemId2!

  giftOptionsList: [GiftOptions] @computed(selectionSet: "{ itemId, aField }")
}

Our subschema 2:

scalar ItemId
scalar ItemId2
scalar ItemId3
scalar AField

type Query {
  item(
      itemId: ItemId
      itemId3: ItemId3
  ): RetailLineItem
}

type Item @key(selectionSet: "{ itemId, itemId2 }") {
  itemId: ItemId!
  itemId2: ItemId2!
  itemId3: ItemId3!

  aField: AField

  giftOptionsList: [GiftOptions] @computed(selectionSet: "{ aField }")
}

type GiftOptions {
	someOptions: [string]
}

What we are seeing right now is that after stitchSchema() is called to stitch both, it actually removes the giftOptionsList: [GiftOptions] @computed(selectionSet: "{ aField }") and removes the fields in type GiftOption making it an empty type breaking our GraphQL server (ApolloServer)

To Reproduce just run stitchedSchema() then feed the stitchedSchema to ApolloServer

(node:32331) UnhandledPromiseRejectionWarning: Error: Type GiftOptions must define one or more fields.

Environment:

  • NodeJS 14

Thank you!

tommysun777 avatar Jun 30 '22 16:06 tommysun777

Thanks for opening this issue! Could you create a reproduction on CodeSandbox or Stackblitz? So we can debug the issue easily. Thanks!

ardatan avatar Jun 30 '22 16:06 ardatan

I bumped into this issue as well. This does not seem to be an issue with the @computed directive (ie it persists when you move directives to merge config) but with the isolateComputedFieldsTransformer on fields returning a composite type. I'd guess there are three cases to consider:

  1. Computed field returning an umerged type (the issue reported above). This might get worse if the type is used in plain (not computed) fields as it would need to be in both sub-schemas but there is no obvious entry point.
  2. Computed field returning a merged type.
  3. Computed field returning an interface/union.

I'll try to come-up with test cases in the style of isolateComputedFieldsTransformer.test.ts but that is unlikely to happen this week.

asodeur avatar Jul 26 '22 06:07 asodeur

Just recently found some time to look into this again. #5162 could hopefully considered a PoC in it's current state. @ardatan Would it be possible to have a look at this to double check the general approach looks ok before I invest into clean-up and test coverage? Expected test results might change significantly depending on how unmerged composite types returned from computed fields are treated by the isolateComputedFieldsTransformer.

asodeur avatar Apr 18 '23 09:04 asodeur

Your approach looks good to me! Great work @asodeur <3

ardatan avatar Apr 18 '23 11:04 ardatan

Merged and released! Thank you for the PR!

ardatan avatar Apr 08 '24 11:04 ardatan