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.

Sign up for GitHub

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

Jump to bottom

Resolvers of merged types not running in nested query #1697

Closed
flux627 opened this issue Jun 27, 2020 · 6 comments · Fixed by #1699
Closed

Resolvers of merged types not running in nested query #1697

flux627 opened this issue Jun 27, 2020 · 6 comments · Fixed by #1699

Comments

@flux627
Copy link

flux627 commented Jun 27, 2020 •
edited

As an exercise, I took this federation-demo repo and tried converting it to use schema stitching provided by this library. See my branch here.

This query is working:

query {
  topProducts {
    upc
    shippingEstimate
  }
}

While this is not:

query {
  me {
    reviews {
      product {
        upc
        shippingEstimate
      }
    }
  }
}

And, it seems that it's not even attempting to delegate to the Reviews service to resolve the shippingEstimate field.

I thought that maybe I needed to do something special for nested queries, but other nested queries seem to be working just fine:

query {
  me {
    reviews {
      author {
        reviews {
          product {
            upc
          }
        }
      }
    }
  }
}

The above works just fine.

Is there something I'm missing here, or is this a bug?
@flux627
Copy link
Author

flux627 commented Jun 27, 2020

Interestingly, with this query, just adding the name field,

query {
  me {
    reviews {
      product {
        upc
        name
        shippingEstimate
      }
    }
  }
}

it does seem to attempt to delegate to the Reviews service, but I get this error:

RangeError: Maximum call stack size exceeded
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2773:19
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39
    at mergeDeep (<redacted>/federation-demo/node_modules/@graphql-tools/utils/index.cjs.js:2788:39

@yaacovCR
Copy link
Collaborator

huh! will investigate. thanks!

@yaacovCR yaacovCR mentioned this issue Jun 28, 2020
Merged
@yaacovCR yaacovCR added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Jun 28, 2020
yaacovCR added a commit that referenced this issue Jun 28, 2020
@yaacovCR
Fixes for complex type merging (#1699)
b8815ad 
= Fix cases where keys differ across subschemas
= Fix multiple rounds of type merging

Closes #1697.

Thanks to @flux627 for reporting.

Includes:
* refactor tests
* Shallow merge the field subschema map
* Add required fields for keys
* Add tests
@yaacovCR yaacovCR reopened this Jun 28, 2020
@yaacovCR
Copy link
Collaborator

Fixed (hopefully) by #1699, available as alpha.

Quickly update your package.json by running:

npx match-version @graphql-tools 6.0.12-alpha-e0bc578.0

@flux627
Copy link
Author

flux627 commented Jun 29, 2020

Seems to work, thank you!

@yaacovCR
Copy link
Collaborator

Thank you for reporting!

@yaacovCR yaacovCR removed the waiting-for-release Fixed/resolved, and waiting for the next stable release label Jul 3, 2020
@yaacovCR
Copy link
Collaborator

yaacovCR commented Jul 3, 2020

Release in 6.0.12, thanks again for reporting!

@yaacovCR yaacovCR closed this as completed Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes for complex type merging ardatan/graphql-tools
2 participants
@yaacovCR @flux627