Add support for a config file to the `generate-persisted-query-manifest` CLI #295

jerelmiller · 2023-06-02T17:28:04Z

The CLI can now take a config file that will be used for the operation extraction. The CLI uses cosmiconfig and will, by default, search for a config file in these locations:

.persisted-query-manifest.config.json
persisted-query-manifest.config.json
.persisted-query-manifest.config.yml
persisted-query-manifest.config.yml
.persisted-query-manifest.config.yaml
persisted-query-manifest.config.yaml
.persisted-query-manifest.config.js
persisted-query-manifest.config.js
.persisted-query-manifest.config.ts
persisted-query-manifest.config.ts
.persisted-query-manifest.config.cjs
persisted-query-manifest.config.cjs
A persisted-query-manifest key in package.json

You can also provide the location of the config file with the --config option to the CLI:

npx generate-persisted-query-manifest --config path/to/persisted-query-manifest.config.ts

The config file can be configured with the following options:

documents (string | string[]) - The glob patterns where the CLI tool should search for GraphQL operations.
- Defaults to src/**/*.{graphql,js,jsx,ts,tsx}.
documentIgnorePatterns (string | string[]) - Glob patterns where the tool should exclude searching for GraphQL operations.
- Defaults to ["**/*.d.ts", "**/*.spec.{js,jsx,ts,tsx}", "**/*.story.{js,jsx,ts,tsx}", "**/*.test.{js,jsx,ts,tsx}"]
output (string) - Path where the manifest should be written.
- Defaults to persisted-query-manifest.json

When the config file is a TypeScript file, you can import the PersistedQueryManifestConfig type to get autocompletion and type checking on the config object.

// persisted-query-manifest.config.ts
import { PersistedQueryManifestConfig } from '@apollo/generate-persisted-query-manifest';

const config: PersistedQueryManifestConfig = {
  documents: 'path/to/documents/**/*.{ts,tsx}'
};

export default config;

…onfig file

changeset-bot · 2023-06-02T17:28:07Z

🦋 Changeset detected

Latest commit: 1e8a07d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@apollo/generate-persisted-query-manifest	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci · 2023-06-02T17:28:30Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

jerelmiller · 2023-06-02T17:29:35Z

packages/generate-persisted-query-manifest/src/index.ts


- globInclude?: string[];
- globExclude?: string[];
+export interface PersistedQueryManifestConfig {


I tweaked the names of these options. Let me know if you like these or prefer something different.

glasser

(heading out for a bit, feel free to make progress/merge/etc)

glasser · 2023-06-02T17:30:36Z

packages/generate-persisted-query-manifest/src/index.ts

- files.add(f);
- }
+ ],
+ output: "persisted-query-manifest.json",


I would lean towards stdout as the default (I'm not sure there are great defaults, most likely people want things under src, and it's a lot easier for folks to just change stdout to a file without even using a config file than to figure out what the program's shorthand for writing to stdout is?)

The reason I went this way is because our initial list of instructions included this command:

rover pq publish <GRAPH_REF> --manifest ./operations/pqs/pqm.json

This assumes a written file to read from. I'm fine changing it to stdout as the default if the plan is to document this command using stdout by default.

I'm going to merge as-is for now and we can revisit this topic 🙂

I still feel like printing to stdout is a better default.

If I have a CLI program that prints to stdout, I 100% of the time immediately know that if I'd rather it go to a file I can just use >.
If I have a CLI program that prints to a file, I have to figure out which convention it uses for stdout (-? STDOUT? something special?) in order to change it.

Plus it'll help remind us to take care to print any errors to stderr :)

I respectfully disagree and I came to this decision for a couple reasons.

Our work-in-progress documentation discusses in detail creating persisted query manifest files, which also shows examples of the rover command that uses a file to generate that list in GraphOS. I felt there would be a huge disconnect if we defaulted to stdout when our documentation encourages files.

Files are trackable in git, and I expect many teams will want to see diffs in code reviews to understand what changes are made to query bodies. Writing to stdout is much more invisible and hard to understand what might be changing.

That being said, I see no reason why we couldn't add a CLI option that allows you to print the result to stdout/stderr in case people like that workflow better. I just wouldn't make this the default behavior.

Thoughts?

Happy to agree to disagree (and leave it your way) here. But it does seem like something that would be helpful to be able to override on a per-execution basis rather than just in the config file, and with stdout as an easily achievable goal (which is I think what you're saying here).

(For example, if you have some sort of super custom transform that requires you to post-process the manifest file every time, it does seem like writing to stdout and using a pipe would be attractive.)

Oh yes, sorry I should have mentioned that when I said "CLI option", I meant you'd do something like:

npx generate-persisted-query-manifest --stdout

(option name TBD)

This would override the output in your config file. I don't see this being an option you change in the config file itself.

Do you have recommendations on the option name? Not sure if there is a common convention in commands for this kind of thing or not.

packages/generate-persisted-query-manifest/src/index.ts

glasser · 2023-06-02T17:31:51Z

packages/generate-persisted-query-manifest/cli.js

+const { Command } = require("commander");
+const { cosmiconfig } = require("cosmiconfig");
+const { generatePersistedQueryManifest, defaults } = require("./dist/index.js");
+const { TypeScriptLoader } = require("cosmiconfig-typescript-loader");


I can't say I get the point of letting you load TS files if there's no obvious way to teach the TS file what the type it's supposed to return is? Or are you able to import PersistedQueryManifestConfig within the file?

Yep! You're able to import the type in the config file, which gives you autocompletion and inline docs for each option.

It looks like this (also in the PR description):

// persisted-query-manifest.config.ts import { PersistedQueryManifestConfig } from '@apollo/generate-persisted-query-manifest'; const config: PersistedQueryManifestConfig = { documents: 'path/to/documents/**/*.{ts,tsx}' }; export default config;

…rations

glasser · 2023-06-21T23:58:17Z

packages/generate-persisted-query-manifest/cli.js

+const supportedExtensions = ["json", "yml", "yaml", "js", "ts", "cjs"];
+
+const searchPlaces = supportedExtensions.flatMap((extension) => [
+ `.${moduleName}.config.${extension}`,


I'm not really sure that having the dot-prefix and the not-dot-prefix versions are both that valuable. It's twice as much to document and twice as many places to look for a thing? I thought this was just us using cosmiconfig defaults but if we're defining our own conventions then maybe we should just make a choice, especially since you can just pass -c if you want something different than our choice.

Prefixing with the dot would make it a hidden file and some may prefer that. I don't think its a bad thing to support both, especially because cosmiconfig does this out of the box. I'm sure we could find a way to make documentation succinct. I'm just not driven by "too much to document" as being the reason to limit the options here.

The reason I have searchPlaces to begin with and I don't just use cosmiconfig('persisted-query-manifest') is because the manifest file is written to persisted-query-manifest.json, which is included in list of files that cosmiconfig will look by default. To differentiate, I figured I'd enforce .config in the filename.

OK with an agree to disagree here too. I do think that letting people specify their own config filename means we can have a more opinionated default, but if your opinion is to have a vaguer opinion then I guess it's opinionated to be less opinionated :)

we can have a more opinionated default

I'm not necessarily against this. In fact, I'd say the search places I have here now are actually a bit more opinionated than the cosmiconfig defaults (I enforce .config.<ext>. cosmiconfig will search for rc files as well, etc).

In fact I could probably simplify how we look for this by saying:

Create a config file named persisted-query-manifest.config.<extension> where <extension> is one of json, yml, yaml, js, ts, or cjs. You may also prefix the file with a dot (.) if you prefer hidden files.

What would you like to see as a more opinionated default?

I do think that's a better way to document it than a long list. I guess my opinion would be to drop the final sentence and people who want hidden files are welcome to pass --config. But it's bikeshed-painting at this point.

True, but the reason I left it is because client teams are typically used to config files that support . in the name (jest, prettier, eslint, etc).

Agreed though, just bikeshedding at this point 😆

glasser · 2023-06-21T23:59:23Z

packages/generate-persisted-query-manifest/src/index.ts

+ /**
+ * Paths to your GraphQL documents: queries, mutations, subscriptions, and fragments.
+ */
+ documents?: string | string[];


A bit surprising that this doesn't have "patterns" in its name when the others do.

GraphQL Codegen, one of the most popular tools of its kind, uses documents for this exact thing. While its not necessarily a community convention, I figured using a name that another popular tool uses would make this feel a bit more familiar.

Should we just endeavor to be fully codegen-compatible (since many folks might use both tools) and let you specify your ignores here too with ! like codegen does? Looks like they use globby under the hood. (Or maybe we just should use @graphql-tools/code-file-loader which is one layer of wrapping up from the graphql-tag-pluck package we're using now?)

I considered that as well and would be happy to reduce the number of options here if we like this better. I opted to maintain them separate, but only because you had created them separate initially 😆.

I guess my main reaction was that it's weird to have "patterns" in one name and not the other; a nice way to resolve that is just "do the same exact thing as graphql-code-generator", and it looks like that might be relatively straightforward by switching to globby.

Yep for sure. Naming is hard and I couldn't think of anything better since documentsIgnore sounded weird and was on the fence of whether ignoredDocuments would make sense to people.

I'm all for simplifying to match graphql code generator 100% though so I'll make this change.

glasser · 2023-06-21T23:59:50Z

packages/generate-persisted-query-manifest/src/index.ts

- globExclude?: string[];
+export interface PersistedQueryManifestConfig {
+ /**
+ * Paths to your GraphQL documents: queries, mutations, subscriptions, and fragments.


Would be helpful to define how the patterns work (even just by referring to "it's anything processed by the glob npm package")

For sure! I can provide an update here.

glasser · 2023-06-22T00:02:47Z

packages/generate-persisted-query-manifest/src/index.ts

- files.add(f);
- }
+ ],
+ output: "persisted-query-manifest.json",


I still feel like printing to stdout is a better default.

If I have a CLI program that prints to stdout, I 100% of the time immediately know that if I'd rather it go to a file I can just use >.
If I have a CLI program that prints to a file, I have to figure out which convention it uses for stdout (-? STDOUT? something special?) in order to change it.

Plus it'll help remind us to take care to print any errors to stderr :)

jerelmiller added 10 commits June 2, 2023 10:40

Install cosmiconfig and cosmiconfig-typescript-loader

859f604

Use cosmiconfig to load the config file

783def7

Add ability to specify the paths used to search for documents via a c…

21f2a5b

…onfig file

Write the result to a file and allow the path to be configured

b5846ca

Add ability to specify config path in CLI

6da66eb

Add --version option to output the version

963af7d

Move handling of prgoram into action function

685ea9c

Let .action handler handle errors and set the error code

65073e2

Update package-lock

08012ca

Write the file in the CLI

3e4fa9c

jerelmiller requested a review from glasser June 2, 2023 17:28

jerelmiller commented Jun 2, 2023

View reviewed changes

glasser reviewed Jun 2, 2023

View reviewed changes

jerelmiller added 2 commits June 2, 2023 11:32

Add changeset

d1b3395

Include .gql extension as a default search location for GraphQL ope…

1e8a07d

…rations

jerelmiller merged commit c41dd06 into glasser/persisted-query-lists Jun 2, 2023

jerelmiller deleted the jerel/config-file branch June 2, 2023 19:20

jerelmiller changed the title ~~Add support for a config file to the generate-persisted-query-manifeest CLI~~ Add support for a config file to the generate-persisted-query-manifest CLI Jun 2, 2023

github-actions bot mentioned this pull request Jun 2, 2023

Version Packages (alpha) #296

Merged

glasser reviewed Jun 22, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for a config file to the `generate-persisted-query-manifest` CLI #295

Add support for a config file to the `generate-persisted-query-manifest` CLI #295

jerelmiller commented Jun 2, 2023 •

edited

Loading

changeset-bot bot commented Jun 2, 2023 •

edited

Loading

codesandbox-ci bot commented Jun 2, 2023 •

edited

Loading

jerelmiller Jun 2, 2023

glasser left a comment

glasser Jun 2, 2023

jerelmiller Jun 2, 2023

jerelmiller Jun 2, 2023

glasser Jun 22, 2023

jerelmiller Jun 22, 2023

glasser Jun 22, 2023

jerelmiller Jun 22, 2023

glasser Jun 2, 2023

jerelmiller Jun 2, 2023

glasser Jun 21, 2023

jerelmiller Jun 22, 2023

glasser Jun 22, 2023

jerelmiller Jun 22, 2023

glasser Jun 22, 2023

jerelmiller Jun 22, 2023

glasser Jun 21, 2023

jerelmiller Jun 22, 2023

glasser Jun 22, 2023

jerelmiller Jun 22, 2023

glasser Jun 22, 2023

jerelmiller Jun 22, 2023

glasser Jun 21, 2023

jerelmiller Jun 22, 2023

glasser Jun 22, 2023

Add support for a config file to the generate-persisted-query-manifest CLI #295

Add support for a config file to the generate-persisted-query-manifest CLI #295

Conversation

jerelmiller commented Jun 2, 2023 • edited Loading

changeset-bot bot commented Jun 2, 2023 • edited Loading

🦋 Changeset detected

codesandbox-ci bot commented Jun 2, 2023 • edited Loading

Choose a reason for hiding this comment

glasser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add support for a config file to the `generate-persisted-query-manifest` CLI #295

Add support for a config file to the `generate-persisted-query-manifest` CLI #295

jerelmiller commented Jun 2, 2023 •

edited

Loading

changeset-bot bot commented Jun 2, 2023 •

edited

Loading

codesandbox-ci bot commented Jun 2, 2023 •

edited

Loading