feat(server fragments): server predefined fragments #576
Conversation
added function addedServerFragments that concats relevent fragments from the server to req query. added function findFragments that return relevant fragments for current request. added function sliceFirstWord. added serverFragments option. resolves graphql#575
returned build to previous version. added serverFragments to OptionsData in index.js. added serverFragments to OptionsData in types/index.d.ts
|
nice! so we have a standard for this using graphql-config that is used pretty widely for loading additional fragments, for example with playground or graphiql. maybe we can build off of that so folks wont have to maintain duplicate configuration for this and the likely soon to be standard graphql-config? |
| * | ||
| * @returns { Set<string> } - relevant fragments for current request. | ||
| */ | ||
| function findFragments(serverFragments: string, query: string, fragmentsInUsed: Set<string>): Set<string> { |
There was a problem hiding this comment.
it would be much simpler and more concise to use the graphql visit() function here I think
There was a problem hiding this comment.
https://graphql.org/graphql-js/language/#visit just parse the operation string to AST, and visit all the FragmentSpread node types i think it would be for this case.
import { visit, print } from 'graphql'
function (serverFragments: string[], query: string) {
const operationAST = parse(query)
const serverFragmentString = ''
visit(operationAst, {
FragmentSpread(node) {
if(serverFragments.includes(node.name.value)) {
serverFragmentString += print(node)
}
}
})
return serverFragmentString
}There was a problem hiding this comment.
I could be wrong though, about which option is more performant, as parsing each query and then re-printing the nodes entails another expense, though much of this could be memoized, eventually cached even?
| }) | ||
|
|
||
| return `${fragmentsInUsed}\n${query}`; | ||
| } |
| let fragmentsInUsed = ''; | ||
|
|
||
| [...(findFragments(serverFragments, query, new Set()))].forEach(fullFragment => { | ||
| fragmentsInUsed += `fragment ${fullFragment} `; |
There was a problem hiding this comment.
itll probably be easier and more perfomant to generate this from AST as well
|
also https://travis-ci.org/graphql/express-graphql/jobs/618258347#L236 this is looking good though! i'll gladly help you get it ready for @IvanGoncharov to give this a final pass as per my earlier comment re: graphql-config, it will make sense as an abstraction around this interface. |
|
@Ariel-Dayan @acao Don't want to be a party killer but I don't think it's the right feature for the reference implementation of GraphQL Server.
This is solved by persistent queries
It brake assumption for client-side tools they since they were compiled/deployed with different sets of fields and this invalidates GraphQL type-safety/"predictable payload" principle. At the same time if you want you can add this feature quite easily in your code without modifying import { parse } from 'graphql';
// ...
app.use(
'/graphql',
graphqlHTTP({
schema: MySessionAwareGraphQLSchema,
graphiql: true,
customParseFn(source) {
const ast = parse(source);
// code that inject fragments into ast
const serverFragmentString = ''
visit(ast, {
FragmentSpread(node) {
const name = node.name.value;
if(serverFragments.includes(name)) {
serverFragmentString += serverFragments.get(name);
}
}
});
return concatAST(ast, parse(serverFragmentString));
}
}),
); |
added function addedServerFragments that concats relevent
fragments from the server to req query.
added function findFragments that return relevant fragments
for current request.
added function sliceFirstWord.
added serverFragments option.
resolves #575