You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hi, first of all thanks for providing this library.
I'm a consumer of graphql-eslint, which depends upstream on graphql-tag-pluck. It uses graphql-tag-pluck in order to run rules that require knowledge about GraphQL definitions in other files, e.g. to check if a field in a query is ever used. Our ESLint runs are taking a long time (>90 seconds) to complete, so I've been running some profiles in an effort to speed things up.
The reason I'm making this issue is that, on an M3 Macbook Pro, our project spends about 29% of its time (about 27 seconds) inside of gqlPluckFromCodeStringSync, which in turn spends almost all of its time parsing and traversing the file with babel-traverse. Our project is fairly large, so this function gets called on almost 4000 files currently, and I'd expect that number to increase over time.
My understanding of graphql-tag-pluck's gqlPluckFromCodeStringSync is that it checks the file's imports for any imports from a user-customisable set of modules (e.g., apollo-server-fastify, react-relay/hooks etc. I think there are also magic GraphQL comments, but same principle), and uses that name to look for any invocations of the gql function (or whatever the user has imported it as). Then it returns whatever is in those invocations (reformatted slightly). Because this process uses babel, it's quite heavyweight.
I was able to cut our total ESLint run time by about 15-16 seconds by first searching the file for any of the module names using a Regex, and returning an empty array if it didn't match any. This means files that definitely don't have any GraphQL code inside them can avoid invoking Babel entirely, reducing the time taken to process those files by a couple of orders of magnitude. In our project, that's about two-thirds of the files. Here's my code (this is just me hacking on the release .cjs file for graphql-tag-pluck in our node_modules):
constdefaultModules={modules: [{name: 'graphql-tag',},// ...snip// This is the list of default modules taken from `graphql-tag-pluck/src/visitor.ts`.// If I made a PR I'd change this to also take any user-provided modules,// and also take into account magic comments, etc. ],};constmoduleList=[
...newSet(defaultModules.modules.map((m)=>m.name).filter(Boolean)),];constGQL_MODULE_REGEX=newRegExp(moduleList.join('|'));/** * Synchronously plucks GraphQL template literals from a single file * * Supported file extensions include: `.js`, `.mjs`, `.cjs`, `.jsx`, `.ts`, `.mjs`, `.cjs`, `.tsx`, `.flow`, `.flow.js`, `.flow.jsx`, `.vue`, `.svelte` * * @param filePath Path to the file containing the code. Required to detect the file type * @param code The contents of the file being parsed. * @param options Additional options for determining how a file is parsed. */constgqlPluckFromCodeStringSync=(filePath,code,options={})=>{validate({ code, options });if(!GQL_MODULE_REGEX.test(code)){// The file doesn't contain any invocations of any of the module names,// so it definitely doesn't contain any gql.return[];}constfileExt=extractExtension(filePath);if(fileExt==='.vue'){code=pluckVueFileScriptSync(code);}elseif(fileExt==='.svelte'){code=pluckSvelteFileScriptSync(code);}returnparseCode({ code, filePath, options }).map((t)=>newgraphql_1.Source(t.content,filePath,t.loc.start));};
Before I make a PR, I'd just like to sense-check: is this a stupid change to make? I don't think it'd have any effect on behaviour, and it should speed up anyone depending on graphql-tag-pluck considerably. But I had also never heard of this library until this afternoon, so I may well be missing something that would make this non-viable.
Thanks!
The text was updated successfully, but these errors were encountered:
Hi, first of all thanks for providing this library.
I'm a consumer of
graphql-eslint
, which depends upstream ongraphql-tag-pluck
. It usesgraphql-tag-pluck
in order to run rules that require knowledge about GraphQL definitions in other files, e.g. to check if a field in a query is ever used. Our ESLint runs are taking a long time (>90 seconds) to complete, so I've been running some profiles in an effort to speed things up.The reason I'm making this issue is that, on an M3 Macbook Pro, our project spends about 29% of its time (about 27 seconds) inside of
gqlPluckFromCodeStringSync
, which in turn spends almost all of its time parsing and traversing the file withbabel-traverse
. Our project is fairly large, so this function gets called on almost 4000 files currently, and I'd expect that number to increase over time.My understanding of
graphql-tag-pluck
'sgqlPluckFromCodeStringSync
is that it checks the file's imports for any imports from a user-customisable set of modules (e.g.,apollo-server-fastify
,react-relay/hooks
etc. I think there are also magic GraphQL comments, but same principle), and uses that name to look for any invocations of thegql
function (or whatever the user has imported it as). Then it returns whatever is in those invocations (reformatted slightly). Because this process uses babel, it's quite heavyweight.I was able to cut our total ESLint run time by about 15-16 seconds by first searching the file for any of the module names using a Regex, and returning an empty array if it didn't match any. This means files that definitely don't have any GraphQL code inside them can avoid invoking Babel entirely, reducing the time taken to process those files by a couple of orders of magnitude. In our project, that's about two-thirds of the files. Here's my code (this is just me hacking on the release .cjs file for
graphql-tag-pluck
in our node_modules):Before I make a PR, I'd just like to sense-check: is this a stupid change to make? I don't think it'd have any effect on behaviour, and it should speed up anyone depending on
graphql-tag-pluck
considerably. But I had also never heard of this library until this afternoon, so I may well be missing something that would make this non-viable.Thanks!
The text was updated successfully, but these errors were encountered: