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(stitchSchemas): subschemas with object inputs and mergeTypes as true causes crash #1631

Merged

Conversation

nicolas-cherel
Copy link
Contributor

NOTE I've no idea if that fix is sufficient or if support of input object is otherwise supported

Without the proposed fix in typeCandidates.ts the added test case triggers the following error from expected unreachable code:

 FAIL  packages/stitch/tests/alternateStitchSchemas.test.ts (11.775 s)
  ● mergeTypes › can merge types

    Type ObjectInput has unknown GraphQL type.

      351 |   } else {
      352 |     // not reachable.
    > 353 |     throw new Error(`Type ${typeName} has unknown GraphQL type.`);
          |           ^
      354 |   }
      355 | }
      356 |

      at merge (packages/stitch/src/typeCandidates.ts:353:11)
      at packages/stitch/src/typeCandidates.ts:222:27
          at Array.forEach (<anonymous>)
      at Object.buildTypeMap (packages/stitch/src/typeCandidates.ts:212:31)
      at Object.stitchSchemas (packages/stitch/src/stitchSchemas.ts:98:19)
      at Object.<anonymous> (packages/stitch/tests/alternateStitchSchemas.test.ts:1989:37)

@nicolas-cherel nicolas-cherel changed the title fix(stitchSchemas): subschemas with object inputs and as true causes crash fix(stitchSchemas): subschemas with object inputs and mergeTypes as true causes crash Jun 10, 2020
@nicolas-cherel
Copy link
Contributor Author

rhaar borched the commit message :)

@yaacovCR
Copy link
Collaborator

@nicolas-cherel are you able to try this out using canary above?

Thanks for kicking this off!

@nicolas-cherel nicolas-cherel force-pushed the pr/fix-mergetypes-with-input-object branch from 09ccadd to ee7727e Compare June 11, 2020 08:55
@nicolas-cherel
Copy link
Contributor Author

rebased with new upstream

@yaacovCR
Copy link
Collaborator

Looks like my changes got borched...

@nicolas-cherel
Copy link
Contributor Author

ho sorry ! I cherry picked what I found it the previous sha1 log

@yaacovCR
Copy link
Collaborator

lol, simultaneously I was going to merge your initial test and do my changes as a separate PR.

@yaacovCR yaacovCR force-pushed the pr/fix-mergetypes-with-input-object branch from d355918 to ee7727e Compare June 11, 2020 14:08
@yaacovCR yaacovCR merged commit 6059c40 into ardatan:master Jun 11, 2020
@theguild-bot
Copy link
Collaborator

The latest changes of this PR are available as alpha in npm: 6.0.10-alpha-6059c40.0

Quickly update your package.json by running:

npx match-version @graphql-tools 6.0.10-alpha-6059c40.0

@yaacovCR
Copy link
Collaborator

Thanks again @nicolas-cherel for your work on this!

@ardatan ardatan added the bugfix label Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants