-
Notifications
You must be signed in to change notification settings - Fork 357
Deeper GraphQL and Recursively nested schemas #171
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
src/plugins/graphql.js
Outdated
| if (field.type && field.type._fields) { | ||
| wrapFields(field.type._fields, tracer, config, responsePathAsArray) | ||
|
|
||
| if (field.type && !field.type._datadog_patched) { |
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.
The _datadog_patched property should be on the resolve function directly. This will avoid the same issue when a function is shared between 2 types.
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.
Yeah, _datadog_patched would never get set on something like a list so is a pointless check here. Though isn't it needed on the type to stop recursion.
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.
You're right that this may be a problem. Would you be able to write a test case for this issue? Or did it end up being a non-issue?
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.
I added a recursive type to the test schema, so because the tests are passing its working. But a dedicated test would be much better.
|
The tests are a bit flaky at the moment. I'll rerun any build that fails because of that. |
This PR fixes the
graphqlintegration stoping a type being instrumented more than once, for allowing recursive types and will now instrument resolvers on types within lists.