Skip to content

Commit

Permalink
fix(visitor-plugin-common): avoid reading from null values (#9709)
Browse files Browse the repository at this point in the history
* fix(visitor-plugin-common): avoid reading from null values

* Add e2e test
  • Loading branch information
frandiox authored Oct 23, 2023
1 parent 5d37ab6 commit 43b525d
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/perfect-forks-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-codegen/visitor-plugin-common': patch
---

Avoid reading from null values when selection sets only contain fragments.
11 changes: 11 additions & 0 deletions dev-test/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,17 @@ const config: CodegenConfig = {
preset: 'client',
presetConfig: { fragmentMasking: true },
},
'./dev-test/test-null-value/result.d.ts': {
schema: './dev-test/test-null-value/schema.graphql',
documents: ['./dev-test/test-null-value/query.ts'],
plugins: ['typescript', 'typescript-operations'],
config: {
// The combination of these two flags caused the following issue:
// https://github.com/dotansimha/graphql-code-generator/pull/9709
skipTypename: true,
mergeFragmentTypes: true,
},
},
},
};

Expand Down
16 changes: 16 additions & 0 deletions dev-test/test-null-value/query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export const MY_QUERY = /* GraphQL */ `
fragment CartLine on CartLine {
id
quantity
}
query Test {
cart {
lines {
nodes {
...CartLine
}
}
}
}
`;
50 changes: 50 additions & 0 deletions dev-test/test-null-value/result.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
export type Maybe<T> = T | null;
export type InputMaybe<T> = Maybe<T>;
export type Exact<T extends { [key: string]: unknown }> = { [K in keyof T]: T[K] };
export type MakeOptional<T, K extends keyof T> = Omit<T, K> & { [SubKey in K]?: Maybe<T[SubKey]> };
export type MakeMaybe<T, K extends keyof T> = Omit<T, K> & { [SubKey in K]: Maybe<T[SubKey]> };
export type MakeEmpty<T extends { [key: string]: unknown }, K extends keyof T> = { [_ in K]?: never };
export type Incremental<T> = T | { [P in keyof T]?: P extends ' $fragmentName' | '__typename' ? T[P] : never };
/** All built-in and custom scalars, mapped to their actual values */
export type Scalars = {
ID: { input: string; output: string };
String: { input: string; output: string };
Boolean: { input: boolean; output: boolean };
Int: { input: number; output: number };
Float: { input: number; output: number };
};

export type BaseCartLine = {
id: Scalars['String']['output'];
quantity: Scalars['Int']['output'];
};

export type BaseCartLineConnection = {
id: Scalars['String']['output'];
nodes: Array<BaseCartLine>;
};

export type Cart = {
id: Scalars['String']['output'];
lines: BaseCartLineConnection;
};

export type CartLine = BaseCartLine & {
id: Scalars['String']['output'];
quantity: Scalars['Int']['output'];
};

export type ComponentizableCartLine = BaseCartLine & {
id: Scalars['String']['output'];
quantity: Scalars['Int']['output'];
};

export type QueryRoot = {
cart?: Maybe<Cart>;
};

export type CartLineFragment = { id: string; quantity: number };

export type TestQueryVariables = Exact<{ [key: string]: never }>;

export type TestQuery = { cart?: { lines: { nodes: Array<{ id: string; quantity: number }> } } | null };
32 changes: 32 additions & 0 deletions dev-test/test-null-value/schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
schema {
query: QueryRoot
}

interface BaseCartLine {
id: String!
quantity: Int!
}

type BaseCartLineConnection {
id: String!
nodes: [BaseCartLine!]!
}

type Cart {
id: String!
lines: BaseCartLineConnection!
}

type CartLine implements BaseCartLine {
id: String!
quantity: Int!
}

type ComponentizableCartLine implements BaseCartLine {
id: String!
quantity: Int!
}

type QueryRoot {
cart: Cart
}
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,6 @@ export class SelectionSetToObject<Config extends ParsedDocumentsConfig = ParsedD
return (
Object.keys(grouped)
.map(typeName => {
const hasUnions = grouped[typeName].filter(s => typeof s !== 'string' && s.union).length > 0;
const relevant = grouped[typeName].filter(Boolean);

if (relevant.length === 0) {
Expand All @@ -729,6 +728,8 @@ export class SelectionSetToObject<Config extends ParsedDocumentsConfig = ParsedD
})
.join(' & ');

const hasUnions = grouped[typeName].filter(s => typeof s !== 'string' && s.union).length > 0;

return relevant.length > 1 && !hasUnions ? `( ${res} )` : res;
})
.filter(Boolean)
Expand Down

0 comments on commit 43b525d

Please sign in to comment.