-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add ability to specify custom document transform during manifest generation #412
Changes from 8 commits
187f237
112d7af
d9ea93c
6a68e88
f49e2c2
28eaff0
3fe6d4a
a0d25fb
771350e
7590911
add0f9d
2c9754b
f2e483b
0e5b322
e317103
d7475fa
a50c3a6
4216dbf
6a57af8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
--- | ||
"@apollo/generate-persisted-query-manifest": minor | ||
--- | ||
|
||
Add ability to specify a custom document transform used during manifest generation. | ||
|
||
> [!NOTE] | ||
> You must be running Apollo Client 3.8.0 or greater to use this feature. | ||
|
||
> [!IMPORTANT] | ||
> This should be the same document transform that is passed to your `ApolloClient` instance, otherwise you risk mismatches in the query output. | ||
|
||
```ts | ||
// persisted-query-manifest.config.ts | ||
import { PersistedQueryManifestConfig } from "@apollo/generate-persisted-query-manifest"; | ||
import { DocumentTransform } from "@apollo/client/core"; | ||
|
||
const documentTransform = new DocumentTransform((document) => { | ||
// transform your document | ||
return transformedDocument; | ||
}) | ||
|
||
const config: PersistedQueryManifestConfig = { | ||
documentTransform, | ||
}; | ||
|
||
export default config; | ||
``` |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import { createHash } from "node:crypto"; | |
import { readFileSync } from "node:fs"; | ||
import path from "node:path"; | ||
import type { PersistedQueryManifestOperation } from "../index"; | ||
import packageJson from "../../package.json"; | ||
|
||
test("prints help message with --help", async () => { | ||
const { cleanup, runCommand } = await setup(); | ||
|
@@ -676,7 +677,7 @@ test("can specify custom document location with config file", async () => { | |
|
||
{ | ||
const ymlConfig = ` | ||
documents: | ||
documents: | ||
- queries/**/*.graphql | ||
`; | ||
|
||
|
@@ -690,7 +691,7 @@ module.exports = { | |
import { PersistedQueryManifestConfig } from '@apollo/generate-persisted-query-manifest'; | ||
|
||
const config: PersistedQueryManifestOperation = { | ||
documents: ['queries/**/*.graphql'] | ||
documents: ['queries/**/*.graphql'] | ||
} | ||
|
||
export default config; | ||
|
@@ -894,7 +895,7 @@ const config: PersistedQueryManifestConfig = { | |
return Buffer.from(query).toString("base64"); | ||
default: | ||
return createDefaultId(); | ||
} | ||
} | ||
} | ||
}; | ||
|
||
|
@@ -925,6 +926,71 @@ export default config; | |
await cleanup(); | ||
}); | ||
|
||
test("can specify custom document transform in config", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably add a test for Apollo Client 3.7 since document transforms aren't run. Will do this after we figure out the right approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you going to do this? I'll approve but feel free to follow through on this idea or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes thanks! I'll add this before merging. Appreciate the approval! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out this is a bit tougher than I originally thought because its difficult to control module resolution on the CLI package itself in tests. While I can install Apollo Client 3.7 which is properly used in the config itself, the cli still resolves to the latest version of the client, so the transform is still run. I'll test locally with a built package against a repo with 3.7 just to make sure I didn't miss anything, but I feel pretty confident this is ok otherwise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added an update to provide a nicer type when using Apollo Client 3.7 where |
||
const { cleanup, runCommand, writeFile, readFile, execute } = await setup(); | ||
const query = gql` | ||
query GreetingQuery { | ||
user { | ||
id | ||
name @custom | ||
} | ||
} | ||
`; | ||
|
||
const expectedQuery = gql` | ||
query GreetingQuery { | ||
user { | ||
id | ||
name | ||
} | ||
} | ||
`; | ||
|
||
await writeFile( | ||
"./package.json", | ||
JSON.stringify({ | ||
dependencies: { | ||
"@apollo/client": packageJson.devDependencies["@apollo/client"], | ||
}, | ||
}), | ||
); | ||
await execute("npm", "install"); | ||
await writeFile("./src/greeting-query.graphql", print(query)); | ||
await writeFile( | ||
"./persisted-query-manifest.config.ts", | ||
` | ||
import { PersistedQueryManifestConfig } from '@apollo/generate-persisted-query-manifest'; | ||
import { DocumentTransform } from '@apollo/client/core'; | ||
import { removeDirectivesFromDocument } from '@apollo/client/utilities'; | ||
|
||
const documentTransform = new DocumentTransform((document) => { | ||
return removeDirectivesFromDocument([{ name: 'custom' }], document); | ||
}); | ||
|
||
const config: PersistedQueryManifestConfig = { | ||
documentTransform, | ||
}; | ||
|
||
export default config; | ||
`, | ||
); | ||
|
||
const { code } = await runCommand(); | ||
const manifest = await readFile("./persisted-query-manifest.json"); | ||
|
||
expect(code).toBe(0); | ||
expect(manifest).toBeManifestWithOperations([ | ||
{ | ||
id: sha256(addTypenameToDocument(expectedQuery)), | ||
name: "GreetingQuery", | ||
body: print(addTypenameToDocument(expectedQuery)), | ||
type: "query", | ||
}, | ||
]); | ||
|
||
await cleanup(); | ||
}); | ||
|
||
test("errors on anonymous operations", async () => { | ||
const { cleanup, runCommand, writeFile } = await setup(); | ||
const query = gql` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
{ | ||
"extends": "../../../../tsconfig.test.base", | ||
"include": ["**/*"], | ||
"compilerOptions": { | ||
"resolveJsonModule": true | ||
}, | ||
"include": ["**/*", "../../package.json"], | ||
"references": [{ "path": "../../" }] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,10 @@ import { | |
InMemoryCache, | ||
Observable, | ||
} from "@apollo/client/core"; | ||
import type { | ||
ApolloClientOptions, | ||
DocumentTransform, | ||
} from "@apollo/client/core"; | ||
import { sortTopLevelDefinitions } from "@apollo/persisted-query-lists"; | ||
import { gqlPluckFromCodeStringSync } from "@graphql-tools/graphql-tag-pluck"; | ||
import globby from "globby"; | ||
|
@@ -40,6 +44,21 @@ export interface PersistedQueryManifestConfig { | |
*/ | ||
documents?: string | string[]; | ||
|
||
/** | ||
* A `DocumentTransform` instance that will be used to transform the GraphQL | ||
* document before it is saved to the manifest. | ||
* | ||
* For more information about document transforms, see the [Document | ||
* transforms](https://www.apollographql.com/docs/react/data/document-transforms) | ||
* documentation page. | ||
* | ||
* IMPORTANT: You must be running `@apollo/client` 3.8.0 or greater to use | ||
* this feature. | ||
* | ||
* @since 1.2.0 | ||
*/ | ||
documentTransform?: DocumentTransform; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the best approach or should we just have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats not a bad idea. It would future proof it a bit more and could avoid a situation where you're using Apollo Client v3.7 where the But you're right that we probably should omit a few options that don't make sense (I can also think of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking a bit more through the client options I'd actually say that almost all of these are unnecessary for manifest generation since a lot of them are for actual network calls. Let's talk through these in our chat later and figure out how we feel about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW I didn't mean to omit them because they don't make sense, but rather because they're what we explicitly pass in ourselves. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We talked on Zoom and I agree that there aren't any other compelling options other than documentTransform so we might as well just do it this way. |
||
|
||
/** | ||
* Path where the manifest file will be written. | ||
*/ | ||
|
@@ -298,10 +317,15 @@ export async function generatePersistedQueryManifest( | |
...sources.map(({ node }) => node).filter(Boolean), | ||
); | ||
const manifestOperationIds = new Map<string, string>(); | ||
|
||
const manifestOperations: PersistedQueryManifestOperation[] = []; | ||
const clientConfig: Partial<ApolloClientOptions<any>> = {}; | ||
|
||
if (config.documentTransform) { | ||
clientConfig.documentTransform = config.documentTransform; | ||
} | ||
|
||
const client = new ApolloClient({ | ||
...clientConfig, | ||
cache: new InMemoryCache({ | ||
fragments, | ||
}), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we update the peer dep while we're at it? maybe we don't really need to leave in support for a random alpha now that 3.8.0 is old news
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya good call! Wasn't sure how much I should clutter this PR up, but I think thats a good change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3fe6d4a