Skip to content
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

Avoid using Babel in graphql-tag-pluck's gqlPluckFromCodeString where possible #6263

Open
drivasperez opened this issue Jun 13, 2024 · 0 comments

Comments

@drivasperez
Copy link

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):

const defaultModules = {
  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. 
  ],
};

const moduleList = [
  ...new Set(defaultModules.modules.map((m) => m.name).filter(Boolean)),
];
const GQL_MODULE_REGEX = new RegExp(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.
 */
const gqlPluckFromCodeStringSync = (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 [];
  }
  const fileExt = extractExtension(filePath);
  if (fileExt === '.vue') {
    code = pluckVueFileScriptSync(code);
  } else if (fileExt === '.svelte') {
    code = pluckSvelteFileScriptSync(code);
  }
  return parseCode({ code, filePath, options }).map(
    (t) => new graphql_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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant