-
Notifications
You must be signed in to change notification settings - Fork 2k
Force inclusion of certain fields #421
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
Comments
I also need this to implement "greedy" queries.. |
I don't think this is a good idea since it reduces a client's ability to predict the shape of the response and could result in accidental over-fetching for existing clients if new types or fields are added which overlap the forcefully included fields. Instead I think a cache identifier is more a clear special case and deserves to be treated as such. I think GraphQL is more likely to see specific support for cache identifiers rather than for a query's response to be shaped by anything other than the query itself. |
I would be very excited to see something like |
Another relevant issue on the graphql repo: graphql/graphql-spec#188 |
We should definitely continue discussing the possibility of an @calebmer It seems like the main motivation for this issue was to avoid the need for preprocessing, and that the main use case could be addressed by the |
Yeah, it's trivial to write a GraphQL transform that would add |
Ok. I'm going to go ahead and close, @calebmer feel free to reopen or comment if there is more to discuss! |
@leebyron, @stubailo, @josephsavona I think we can all agree a global First reason why I think force inclusion of an Falcor doesn’t need to add an
So in my mind, Falcor is better for maintaining server state in complex apps off the shelf, and GraphQL provides a much better experience when you make single requests. After coming to this conclusion the question then becomes, well how do you fix this? Because I’m definitely not using Falcor anytime soon, the tooling just isn’t as good as GraphQL 😉. My answer would be to implement greedy The Furthermore, a transform to add Continuing on as devil’s advocate, now I want to ask a question 😉 I get that overfetching is an anti-pattern that GraphQL is trying to avoid, however how bad is it really to always include a couple of relatively small string field in the payload? Especially when the upsides are fairly large for a better DX. I don’t think it’s that great of a cost especially given that teams working with GraphQL have already decided it’s an ok practice to potentially include more than you need in the following ways:
Given its already common practice to give more than the client asked for (whether it be in duplicate data, extra data, or arbitrary data) is there a reason to hold this tenet when there may be benefits in selectively breaking it? (with little drawbacks that I see) Of course I’m operating under the assumption that the real reason we don’t send extra data is to reduce network payload size, I may be wrong so please let me know 👍 I love GraphQL, I think it has a really promising future, the I’ve seen where this could be useful. If you read my word wall and decide this is not worth exploring or at least not worth a second thought, I won’t be hurt. Keep up the great work! 👍 |
This is not true, the way Apollo currently does query transformations is by applying them recursively to a selection set. You don't need the schema to do that. 😄 |
With regards to Apollo, the reason why it would be beneficial to have const CacheableGraphQLObjectTypenames = [/* ... */]; // these types have an `id` field
const client = new ApolloClient({
dataIdFromObject: ({ __typename, id }) =>
(CacheableGraphQLObjectTypenames.includes(__typename) ? `${__typename}:${id}` : null),
networkInterface: createNetworkInterface(process.env.GRAPHQL_API_ENDPOINT),
queryTransformer: addTypename,
}); would change to this: const client = new ApolloClient({
dataIdFromObject: ({ __id }) => __id,
networkInterface: createNetworkInterface(process.env.GRAPHQL_API_ENDPOINT),
queryTransformer: addId,
}); |
Was there ever any progress on adding greedy queries / fields that are always returned? |
@cantino Did you find anything out here? |
I don't think so. |
Is there a way to force
graphql-js
to always include certain fields in it’s output? For example anid
field? In order for Relay to always get theid
field, you need to add in a preprocessing step so Relay can insert theid
field into queries. It seems like a much simpler solution would to be just let GraphQL servers forcefully always include anid
field if available. Doesgraphql-js
currently allows this, and may this be allowed in the future?The text was updated successfully, but these errors were encountered: