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

Automatic GraphQL code generation #4048

Merged
merged 9 commits into from
Jun 18, 2024
Merged

Automatic GraphQL code generation #4048

merged 9 commits into from
Jun 18, 2024

Conversation

shauns
Copy link
Contributor

@shauns shauns commented Jun 14, 2024

WHY are these changes introduced?

Fixes a long-standing issue: the types for GraphQL results and variables, as well as the content of the queries/mutations themselves, were manually specified and not automatically tested.

WHAT is this pull request doing?

🆕 The main addition here is p graphql-codegen. This takes .graphql documents (containing a query/mutation that we use) and:

  • validates the structure, fields, etc. against the appropriate schema
  • (re-)generates TS types for the result and variable formats, per query/mutation, based on the real schema

✨ It also provides auto-complete and in-editor validation & documentation when writing queries in VS Code.

🚨 generated types are accurate vs. the schema, which means they can and will find snags the hand-crafted types didn't. improved quality is a good thing! but as more queries switch to this method, more fixes will be needed.

💁‍♂️ Some implementation notes:

  • GraphQL schemas can be refreshed with dev up internally
  • I've included support for partners and business platform destination APIs. It should be obvious enough how to expand to include additional sources
  • The GraphQL schemas are not committed with this repo; the generated types are.
  • The generated type files are subject to a small amount of post-processing to fit into the rest of our code-base. This is call configured in the nx tasks.
  • Generating types is a cached operation in nx: if nothing has changed, the command will run very quickly.
  • A github action is provided that ensures graphql-codegen has been run prior to commits that do change queries.

❓ I've converted a few queries in the scope of this PR. Should the rest be converted now? Or can we do that in a follow up?

How to test your changes?

CI does most of the testing here. Perhaps the thing to try is to migrate a query/mutation to this format: see how easy/ergonomic it is, what errors it finds, etc.

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

Copy link
Contributor

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

Copy link
Contributor

github-actions bot commented Jun 14, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.97% (-0.14% 🔻)
7357/10223
🟡 Branches
68.81% (-0.21% 🔻)
3600/5232
🟡 Functions
71.49% (-0.13% 🔻)
1953/2732
🟡 Lines
73.22% (-0.15% 🔻)
6938/9475
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / organizations.ts
100% 100% 100% 100%
🟢
... / all-orgs.ts
100% 100% 100% 100%
🟢
... / update-draft.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / poll-app-logs.ts
91.84% (+1.27% 🔼)
75% (-8.33% 🔻)
71.43% (+21.43% 🔼)
91.84% (+1.27% 🔼)
🟢
... / upload.ts
83.59% (-0.5% 🔻)
77.94% (-2.94% 🔻)
85% (-0.37% 🔻)
84.35% (-0.53% 🔻)
🔴
... / partners-client.ts
18.42% (-0.16% 🔻)
34.62%
14.29% (-0.3% 🔻)
18.35% (-0.17% 🔻)
🟢
... / conf-store.ts
82.35% (-17.65% 🔻)
53.85% (-23.08% 🔻)
80% (-20% 🔻)
82.35% (-17.65% 🔻)
🔴
... / content-tokens.ts
50% (-1.61% 🔻)
20% (-13.33% 🔻)
50%
48.28% (-1.72% 🔻)
🟢
... / ConcurrentOutput.tsx
98.36% (-1.64% 🔻)
88.89% (-6.57% 🔻)
100%
98.31% (-1.69% 🔻)
🟢
... / node-package-manager.ts
84.76% (-0.27% 🔻)
81.33% (-0.48% 🔻)
84.85% (-0.45% 🔻)
84.76% (-0.27% 🔻)
🔴
... / system.ts
5.56% (-20.76% 🔻)
0% (-20.69% 🔻)
25% (-25% 🔻)
5.88% (-21.9% 🔻)
🟢
... / partners.ts
82.61% (-17.39% 🔻)
75%
71.43% (-28.57% 🔻)
81.82% (-18.18% 🔻)

Test suite run success

1695 tests passing in 793 suites.

Report generated by 🧪jest coverage report action from 7c627c2

export default {
projects: {
partners: projectFactory('partners', 'cli_schema.graphql'),
businessPlatform: projectFactory('business-platform', 'destinations_schema.graphql'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to include also the new app-management API? that's different than business-platform right?

Copy link
Contributor

@isaacroldan isaacroldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested by migrating a query myself, it works perfectly, process is easy 👌

Copy link
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/api/business-platform.d.ts
@@ -1,4 +1,6 @@
 import { GraphQLVariables } from './graphql.js';
+import { TypedDocumentNode } from '@graphql-typed-document-node/core';
+import { Variables } from 'graphql-request';
 /**
  * Executes a GraphQL query against the Business Platform Destinations API.
  *
@@ -7,4 +9,13 @@ import { GraphQLVariables } from './graphql.js';
  * @param variables - GraphQL variables to pass to the query.
  * @returns The response of the query of generic type <T>.
  */
-export declare function businessPlatformRequest<T>(query: string, token: string, variables?: GraphQLVariables): Promise<T>;
\ No newline at end of file
+export declare function businessPlatformRequest<T>(query: string, token: string, variables?: GraphQLVariables): Promise<T>;
+/**
+ * Executes a GraphQL query against the Business Platform Destinations API. Uses typed documents.
+ *
+ * @param query - GraphQL query to execute.
+ * @param token - Business Platform token.
+ * @param variables - GraphQL variables to pass to the query.
+ * @returns The response of the query of generic type <TResult>.
+ */
+export declare function businessPlatformRequestDoc<TResult, TVariables extends Variables>(query: TypedDocumentNode<TResult, TVariables>, token: string, variables?: TVariables): Promise<TResult>;
\ No newline at end of file
packages/cli-kit/dist/public/node/api/graphql.d.ts
@@ -1,19 +1,26 @@
 import { rawRequest, RequestDocument, Variables } from 'graphql-request';
+import { TypedDocumentNode } from '@graphql-typed-document-node/core';
 export interface GraphQLVariables {
     [key: string]: any;
 }
 export type GraphQLResponse<T> = Awaited<ReturnType<typeof rawRequest<T>>>;
-export interface GraphQLRequestOptions<T> {
-    query: RequestDocument;
+interface GraphQLRequestBaseOptions<TResult> {
     api: string;
     url: string;
     token?: string;
     addedHeaders?: {
         [header: string]: string;
     };
-    variables?: Variables;
-    responseOptions?: GraphQLResponseOptions<T>;
+    responseOptions?: GraphQLResponseOptions<TResult>;
 }
+export type GraphQLRequestOptions<T> = GraphQLRequestBaseOptions<T> & {
+    query: RequestDocument;
+    variables?: Variables;
+};
+export type GraphQLRequestDocOptions<TResult, TVariables> = GraphQLRequestBaseOptions<TResult> & {
+    query: TypedDocumentNode<TResult, TVariables>;
+    variables?: TVariables;
+};
 export interface GraphQLResponseOptions<T> {
     handleErrors?: boolean;
     onResponse?: (response: GraphQLResponse<T>) => void;
@@ -24,4 +31,12 @@ export interface GraphQLResponseOptions<T> {
  * @param options - GraphQL request options.
  * @returns The response of the query of generic type <T>.
  */
-export declare function graphqlRequest<T>(options: GraphQLRequestOptions<T>): Promise<T>;
\ No newline at end of file
+export declare function graphqlRequest<T>(options: GraphQLRequestOptions<T>): Promise<T>;
+/**
+ * Executes a GraphQL query to an endpoint. Uses typed documents.
+ *
+ * @param options - GraphQL request options.
+ * @returns The response of the query of generic type <TResult>.
+ */
+export declare function graphqlRequestDoc<TResult, TVariables extends Variables>(options: GraphQLRequestDocOptions<TResult, TVariables>): Promise<TResult>;
+export {};
\ No newline at end of file
packages/cli-kit/dist/public/node/api/partners.d.ts
@@ -1,4 +1,6 @@
 import { GraphQLVariables, GraphQLResponse } from './graphql.js';
+import { TypedDocumentNode } from '@graphql-typed-document-node/core';
+import { Variables } from 'graphql-request';
 /**
  * Executes a GraphQL query against the Partners API.
  *
@@ -8,6 +10,15 @@ import { GraphQLVariables, GraphQLResponse } from './graphql.js';
  * @returns The response of the query of generic type <T>.
  */
 export declare function partnersRequest<T>(query: string, token: string, variables?: GraphQLVariables): Promise<T>;
+/**
+ * Executes a GraphQL query against the Partners API. Uses typed documents.
+ *
+ * @param query - GraphQL query to execute.
+ * @param token - Partners token.
+ * @param variables - GraphQL variables to pass to the query.
+ * @returns The response of the query of generic type <TResult>.
+ */
+export declare function partnersRequestDoc<TResult, TVariables extends Variables>(query: TypedDocumentNode<TResult, TVariables>, token: string, variables?: TVariables): Promise<TResult>;
 export interface FunctionUploadUrlGenerateResponse {
     functionUploadUrlGenerate: {
         generatedUrlDetails: {

@shauns shauns enabled auto-merge June 17, 2024 11:02
@shauns shauns added this pull request to the merge queue Jun 18, 2024
Merged via the queue into main with commit 226f119 Jun 18, 2024
36 checks passed
@shauns shauns deleted the graphql-codegen-partners branch June 18, 2024 10:03
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

Successfully merging this pull request may close these issues.

None yet

2 participants