-
-
Notifications
You must be signed in to change notification settings - Fork 809
-
-
Notifications
You must be signed in to change notification settings - Fork 809
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
selectionSet doesn't work when using WrapFields transforms #1725
Comments
On the bleeding edge! Hopefully this will be fixed by #1718 |
Wow! I don't know nearly enough about GraphQL to plan a PR so I was already planning how to work around this! I'll test that change on Monday, thank you! |
Sadly it doesn't seem to be resolved with #1718. The selectionSet isn't added to the query properly, I verified this by removing the schema mocks, and adding the resolvers: resolvers: {
Query: {
user: { resolve: () => ({}) },
},
TestUser: {
handle: { resolve: () => { console.log('handle requested'); return 'foo'; } },
},
}, When the WrapFields transform is removed, you'll always get the log entry, but nothing when it is present. (For what it's worth I'm basically trying to replicate the namespaces and links from |
Is there a problem with first argument to WrapFields? I think you should be using the renamed query type name |
I don't think so, (I also confirmed I am definitely loading the new version of graphql-tools by adding a console.log in some of the new functions added.) That said I can take out the import { ApolloServer } from 'apollo-server';
import { stitchSchemas } from '@graphql-tools/stitch';
import { WrapFields } from '@graphql-tools/wrap';
import { makeExecutableSchema } from '@graphql-tools/schema';
const test = makeExecutableSchema({
typeDefs: `
type Query {
user: TestUser
}
type TestUser {
handle: String
}
`,
resolvers: {
Query: {
user: { resolve: () => ({}) },
},
TestUser: {
handle: { resolve: () => { console.log('handle requested'); return 'foo'; } },
},
},
});
const schema = stitchSchemas({
subschemas: [{
schema: test,
transforms: [
new WrapFields(test.getQueryType().name, ['wrapped'], [`WrappedQuery`]),
],
}],
typeDefs: `
extend type TestUser {
allInfo: String
}
`,
resolvers: {
TestUser: {
allInfo: {
selectionSet: `{ handle }`,
resolve(info) {
return JSON.stringify(info);
},
},
},
},
});
const server = new ApolloServer({
schema,
introspection: true,
});
export default () => {};
server.listen().then(({ url }) => {
// eslint-disable-next-line no-console
console.log(`🚀 Server ready at ${url}`);
}); Generated schema: directive @specifiedBy(url: String!) on SCALAR
type Query {
wrapped: WrappedQuery
}
type TestUser {
handle: String
allInfo: String
}
type WrappedQuery {
user: TestUser
} Queries: { wrapped { user { allInfo } } } {
"data": {
"wrapped": {
"user": null
}
}
} {
wrapped {
user {
handle
allInfo
}
}
} {
"data": {
"wrapped": {
"user": {
"handle": "foo",
"allInfo": "{\"handle\":\"foo\"}"
}
}
}
} |
You are, of course, correct. My investigation demonstrated that this is because although WrapFields works for root fields, it actually works in a different way, specifically with regard to transforming the transmitted field nodes. When wrapping non-root fields, transforming/hoisting the transmitted field nodes is performed using the MapFields transform. When wrapping root fields, which perform the delegation rather than simply reading the result, the transmitted field nodes are adjusted simply by adding a wrapping field with a resolver that returns an empty object and the resolver that performs the delegation is retained, but moved down a level such that the hoisting is not directly performed, or, more accurately, performed by graphql-js. And, basically, the mechanism in which a proxied request is created still works, but the mechanism for adding selection sets does not, because the hoisted query is now of the new WrappedQuery type, to use above example, not the Query type, and the selection set adder transform is not aware of this. I attempted a solution in which the parent type is added as a parameter to that transform, which works, but this seems to break type merging as well. It seems like the most consistent solution may be to change the wrapping mechanism to move the delegation to the new root field, so the root fields become just a not so special case of all object fields. Rewriting root fields basically just requires saving the original schema config within a wrapped schema, and recalling the proxy resolver generator functions. That is the avenue I am working on (slowly) but there may be other solutions that work in the meantime. I hope this investigation helps anybody who might want to pick this up. |
Thanks for the detailed explanation; I'm not really confident enough in how graphql works to do a PR right now but hope to revisit t some point in the future if it hasn't been solved by then! |
Actually, I was able to work out a fix with the first approach. |
Visitors visiting the proxying ast cannot rely on the operation being valid according to the source schema in all cases. But, the proxying document's root field nodes represent values with the expected return type, and so the document can be traversed according to the source schema from that point and below. See #1725
Fix from #1757 is available as alpha in npm: Quickly update your package.json by running:
Hopefully this works for you! |
That fixes it! 😄 |
Awesome! Please consider visiting https://github.com/sponsors/yaacovCR to support my open-source development efforts. :) |
Using
selectionSet
withstitchSchemas
does not work when a schema has aWrapFields
transform.For example,
query { wrapped { user { name, allInfo } } }
should return{"name":"Hello World","handle":"Hello World"}
but insteadhandle
is missing.Removing the
WrapFields
transform (and removingwrapped
from the query) causes the correct result.(I'm not sure how feasible this is to fix, but I spent 2 hours debugging this, so hopefully it can at least save others the trouble.)
The text was updated successfully, but these errors were encountered: