Skip to content

Commit 75378cc

Browse files
Code-HexCode-Hex
authored
Merge pull request #958 from trevor-anderson/fix-used-before-defined-output-issue-528
2 parents d90fb9d + ce1e1aa commit 75378cc

File tree

5 files changed

+114
-65
lines changed

5 files changed

+114
-65
lines changed

‎src/graphql.ts

Lines changed: 62 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,23 @@ import type {
1414
import { Graph } from 'graphlib';
1515
import {
1616
isSpecifiedScalarType,
17+
Kind,
1718
visit,
1819
} from 'graphql';
1920

20-
export const isListType = (typ?: TypeNode): typ is ListTypeNode => typ?.kind === 'ListType';
21-
export const isNonNullType = (typ?: TypeNode): typ is NonNullTypeNode => typ?.kind === 'NonNullType';
22-
export const isNamedType = (typ?: TypeNode): typ is NamedTypeNode => typ?.kind === 'NamedType';
21+
/**
22+
* Recursively unwraps a GraphQL type until it reaches the NamedType.
23+
*
24+
* Since a GraphQL type is defined as either a NamedTypeNode, ListTypeNode, or NonNullTypeNode,
25+
* this implementation safely recurses until the underlying NamedTypeNode is reached.
26+
*/
27+
function getNamedType(typeNode: TypeNode): NamedTypeNode {
28+
return typeNode.kind === Kind.NAMED_TYPE ? typeNode : getNamedType(typeNode.type);
29+
}
30+
31+
export const isListType = (typ?: TypeNode): typ is ListTypeNode => typ?.kind === Kind.LIST_TYPE;
32+
export const isNonNullType = (typ?: TypeNode): typ is NonNullTypeNode => typ?.kind === Kind.NON_NULL_TYPE;
33+
export const isNamedType = (typ?: TypeNode): typ is NamedTypeNode => typ?.kind === Kind.NAMED_TYPE;
2334

2435
export const isInput = (kind: string) => kind.includes('Input');
2536

@@ -48,44 +59,50 @@ export function InterfaceTypeDefinitionBuilder(useInterfaceTypes: boolean | unde
4859
export function topologicalSortAST(schema: GraphQLSchema, ast: DocumentNode): DocumentNode {
4960
const dependencyGraph = new Graph();
5061
const targetKinds = [
51-
'ObjectTypeDefinition',
52-
'InputObjectTypeDefinition',
53-
'EnumTypeDefinition',
54-
'UnionTypeDefinition',
55-
'ScalarTypeDefinition',
62+
Kind.OBJECT_TYPE_DEFINITION,
63+
Kind.INPUT_OBJECT_TYPE_DEFINITION,
64+
Kind.INTERFACE_TYPE_DEFINITION,
65+
Kind.SCALAR_TYPE_DEFINITION,
66+
Kind.ENUM_TYPE_DEFINITION,
67+
Kind.UNION_TYPE_DEFINITION,
5668
];
5769

5870
visit<DocumentNode>(ast, {
5971
enter: (node) => {
6072
switch (node.kind) {
61-
case 'ObjectTypeDefinition':
62-
case 'InputObjectTypeDefinition': {
73+
case Kind.OBJECT_TYPE_DEFINITION:
74+
case Kind.INPUT_OBJECT_TYPE_DEFINITION:
75+
case Kind.INTERFACE_TYPE_DEFINITION: {
6376
const typeName = node.name.value;
6477
dependencyGraph.setNode(typeName);
6578

6679
if (node.fields) {
6780
node.fields.forEach((field) => {
68-
if (field.type.kind === 'NamedType') {
69-
const dependency = field.type.name.value;
70-
const typ = schema.getType(dependency);
71-
if (typ?.astNode?.kind === undefined || !targetKinds.includes(typ.astNode.kind))
72-
return;
73-
74-
if (!dependencyGraph.hasNode(dependency))
75-
dependencyGraph.setNode(dependency);
81+
// Unwrap the type
82+
const namedTypeNode = getNamedType(field.type);
83+
const dependency = namedTypeNode.name.value;
84+
const namedType = schema.getType(dependency);
85+
if (
86+
namedType?.astNode?.kind === undefined
87+
|| !targetKinds.includes(namedType.astNode.kind)
88+
) {
89+
return;
90+
}
7691

77-
dependencyGraph.setEdge(typeName, dependency);
92+
if (!dependencyGraph.hasNode(dependency)) {
93+
dependencyGraph.setNode(dependency);
7894
}
95+
dependencyGraph.setEdge(typeName, dependency);
7996
});
8097
}
8198
break;
8299
}
83-
case 'ScalarTypeDefinition':
84-
case 'EnumTypeDefinition': {
100+
case Kind.SCALAR_TYPE_DEFINITION:
101+
case Kind.ENUM_TYPE_DEFINITION: {
85102
dependencyGraph.setNode(node.name.value);
86103
break;
87104
}
88-
case 'UnionTypeDefinition': {
105+
case Kind.UNION_TYPE_DEFINITION: {
89106
const dependency = node.name.value;
90107
if (!dependencyGraph.hasNode(dependency))
91108
dependencyGraph.setNode(dependency);
@@ -111,16 +128,16 @@ export function topologicalSortAST(schema: GraphQLSchema, ast: DocumentNode): Do
111128
// Create a map of definitions for quick access, using the definition's name as the key.
112129
const definitionsMap: Map<string, DefinitionNode> = new Map();
113130

114-
// SCHEMA_DEFINITION does not have name.
131+
// SCHEMA_DEFINITION does not have a name.
115132
// https://spec.graphql.org/October2021/#sec-Schema
116-
const astDefinitions = ast.definitions.filter(def => def.kind !== 'SchemaDefinition');
133+
const astDefinitions = ast.definitions.filter(def => def.kind !== Kind.SCHEMA_DEFINITION);
117134

118135
astDefinitions.forEach((definition) => {
119136
if (hasNameField(definition) && definition.name)
120137
definitionsMap.set(definition.name.value, definition);
121138
});
122139

123-
// Two arrays to store sorted and not sorted definitions.
140+
// Two arrays to store sorted and unsorted definitions.
124141
const sortedDefinitions: DefinitionNode[] = [];
125142
const notSortedDefinitions: DefinitionNode[] = [];
126143

@@ -141,7 +158,7 @@ export function topologicalSortAST(schema: GraphQLSchema, ast: DocumentNode): Do
141158

142159
if (newDefinitions.length !== astDefinitions.length) {
143160
throw new Error(
144-
`unexpected definition length after sorted: want ${astDefinitions.length} but got ${newDefinitions.length}`,
161+
`Unexpected definition length after sorting: want ${astDefinitions.length} but got ${newDefinitions.length}`,
145162
);
146163
}
147164

@@ -155,33 +172,35 @@ function hasNameField(node: ASTNode): node is DefinitionNode & { name?: NameNode
155172
return 'name' in node;
156173
}
157174

158-
// Re-implemented w/o CycleException version
159-
// https://github.com/dagrejs/graphlib/blob/8d27cb89029081c72eb89dde652602805bdd0a34/lib/alg/topsort.js
175+
/**
176+
* [Re-implemented topsort function][topsort-ref] with cycle handling. This version iterates over
177+
* all nodes in the graph to ensure every node is visited, even if the graph contains cycles.
178+
*
179+
* [topsort-ref]: https://github.com/dagrejs/graphlib/blob/8d27cb89029081c72eb89dde652602805bdd0a34/lib/alg/topsort.js
180+
*/
160181
export function topsort(g: Graph): string[] {
161-
const visited: Record<string, boolean> = {};
162-
const stack: Record<string, boolean> = {};
163-
const results: any[] = [];
182+
const visited = new Set<string>();
183+
const results: Array<string> = [];
164184

165185
function visit(node: string) {
166-
if (!(node in visited)) {
167-
stack[node] = true;
168-
visited[node] = true;
169-
const predecessors = g.predecessors(node);
170-
if (Array.isArray(predecessors))
171-
predecessors.forEach(node => visit(node));
172-
173-
delete stack[node];
174-
results.push(node);
175-
}
186+
if (visited.has(node))
187+
return;
188+
visited.add(node);
189+
// Recursively visit all predecessors of the node.
190+
g.predecessors(node)?.forEach(visit);
191+
results.push(node);
176192
}
177193

178-
g.sinks().forEach(node => visit(node));
194+
// Visit every node in the graph (instead of only sinks)
195+
g.nodes().forEach(visit);
179196

180197
return results.reverse();
181198
}
182199

183200
export function isGeneratedByIntrospection(schema: GraphQLSchema): boolean {
184-
return Object.entries(schema.getTypeMap()).filter(([name, type]) => !name.startsWith('__') && !isSpecifiedScalarType(type)).every(([, type]) => type.astNode === undefined)
201+
return Object.entries(schema.getTypeMap())
202+
.filter(([name, type]) => !name.startsWith('__') && !isSpecifiedScalarType(type))
203+
.every(([, type]) => type.astNode === undefined)
185204
}
186205

187206
// https://spec.graphql.org/October2021/#EscapedCharacter

‎tests/graphql.spec.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,34 @@ describe('topologicalSortAST', () => {
197197
expect(sortedSchema).toBe(expectedSortedSchema);
198198
});
199199

200+
it('should place interface definitions before types that depend on them', () => {
201+
const schema = /* GraphQL */ `
202+
type A {
203+
id: ID!
204+
node: Node
205+
}
206+
207+
interface Node {
208+
id: ID!
209+
}
210+
`;
211+
212+
const sortedSchema = getSortedSchema(schema);
213+
214+
const expectedSortedSchema = dedent/* GraphQL */`
215+
interface Node {
216+
id: ID!
217+
}
218+
219+
type A {
220+
id: ID!
221+
node: Node
222+
}
223+
`;
224+
225+
expect(sortedSchema).toBe(expectedSortedSchema);
226+
});
227+
200228
it('should correctly handle schema with circular dependencies', () => {
201229
const schema = /* GraphQL */ `
202230
input A {
@@ -235,13 +263,13 @@ describe('topologicalSortAST', () => {
235263
const sortedSchema = getSortedSchema(schema);
236264

237265
const expectedSortedSchema = dedent/* GraphQL */`
238-
input A {
239-
a: A
240-
}
241-
242266
input B {
243267
b: B
244268
}
269+
270+
input A {
271+
a: A
272+
}
245273
`;
246274

247275
expect(sortedSchema).toBe(expectedSortedSchema);

‎tests/myzod.spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,12 +1506,6 @@ describe('myzod', () => {
15061506
);
15071507
expect(removedInitialEmitValue(result.content)).toMatchInlineSnapshot(`
15081508
"
1509-
export const UserCreateInputSchema: myzod.Type<UserCreateInput> = myzod.object({
1510-
name: myzod.string(),
1511-
date: myzod.date(),
1512-
email: myzod.string().email()
1513-
});
1514-
15151509
export const UserSchema: myzod.Type<User> = myzod.object({
15161510
__typename: myzod.literal('User').optional(),
15171511
id: myzod.string(),
@@ -1521,6 +1515,12 @@ describe('myzod', () => {
15211515
isMember: myzod.boolean().optional().nullable(),
15221516
createdAt: myzod.date()
15231517
});
1518+
1519+
export const UserCreateInputSchema: myzod.Type<UserCreateInput> = myzod.object({
1520+
name: myzod.string(),
1521+
date: myzod.date(),
1522+
email: myzod.string().email()
1523+
});
15241524
"
15251525
`)
15261526
for (const wantNotContain of ['Query', 'Mutation', 'Subscription'])

‎tests/yup.spec.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,6 +1486,7 @@ describe('yup', () => {
14861486
},
14871487
{},
14881488
);
1489+
14891490
expect(result.content).toMatchInlineSnapshot(`
14901491
"
14911492
@@ -1495,12 +1496,6 @@ describe('yup', () => {
14951496
}).defined()
14961497
}
14971498
1498-
export const UserCreateInputSchema: yup.ObjectSchema<UserCreateInput> = yup.object({
1499-
name: yup.string().defined().nonNullable(),
1500-
date: yup.date().defined().nonNullable(),
1501-
email: yup.string().email().defined().nonNullable()
1502-
});
1503-
15041499
export const UserSchema: yup.ObjectSchema<User> = yup.object({
15051500
__typename: yup.string<'User'>().optional(),
15061501
id: yup.string().defined().nonNullable(),
@@ -1510,6 +1505,12 @@ describe('yup', () => {
15101505
isMember: yup.boolean().defined().nullable().optional(),
15111506
createdAt: yup.date().defined().nonNullable()
15121507
});
1508+
1509+
export const UserCreateInputSchema: yup.ObjectSchema<UserCreateInput> = yup.object({
1510+
name: yup.string().defined().nonNullable(),
1511+
date: yup.date().defined().nonNullable(),
1512+
email: yup.string().email().defined().nonNullable()
1513+
});
15131514
"
15141515
`)
15151516

‎tests/zod.spec.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,14 +1732,9 @@ describe('zod', () => {
17321732
},
17331733
{},
17341734
);
1735+
17351736
expect(removedInitialEmitValue(result.content)).toMatchInlineSnapshot(`
17361737
"
1737-
export const UserCreateInputSchema: z.ZodObject<Properties<UserCreateInput>> = z.object({
1738-
name: z.string(),
1739-
date: z.date(),
1740-
email: z.string().email()
1741-
});
1742-
17431738
export const UserSchema: z.ZodObject<Properties<User>> = z.object({
17441739
__typename: z.literal('User').optional(),
17451740
id: z.string(),
@@ -1749,6 +1744,12 @@ describe('zod', () => {
17491744
isMember: z.boolean().nullish(),
17501745
createdAt: z.date()
17511746
});
1747+
1748+
export const UserCreateInputSchema: z.ZodObject<Properties<UserCreateInput>> = z.object({
1749+
name: z.string(),
1750+
date: z.date(),
1751+
email: z.string().email()
1752+
});
17521753
"
17531754
`)
17541755

0 commit comments

Comments
 (0)