Skip to content

Commit 8f6d3ef

Browse files
ardatanardatan
authored
Faster validation (#4801)
1 parent 3262a05 commit 8f6d3ef

File tree

3 files changed

+49
-220
lines changed

3 files changed

+49
-220
lines changed

‎.changeset/green-seas-repair.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-tools/utils': major
3+
---
4+
5+
_BREAKING_: `checkValidationErrors` has been dropped and `validateGraphQlDocuments` now accepts `DocumentNode[]` instead and it throws the original `GraphQLError`s with the correct stack trace

‎packages/utils/src/validate-documents.ts‎

Lines changed: 21 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -2,108 +2,45 @@ import {
22
Kind,
33
validate,
44
GraphQLSchema,
5-
GraphQLError,
65
specifiedRules,
7-
FragmentDefinitionNode,
86
ValidationContext,
97
ASTVisitor,
10-
DefinitionNode,
11-
concatAST,
128
DocumentNode,
139
versionInfo,
10+
DefinitionNode,
1411
} from 'graphql';
15-
import { Source } from './loaders.js';
16-
import { AggregateError } from './AggregateError.js';
1712

1813
export type ValidationRule = (context: ValidationContext) => ASTVisitor;
1914

20-
export interface LoadDocumentError {
21-
readonly filePath?: string;
22-
readonly errors: ReadonlyArray<GraphQLError>;
23-
}
24-
25-
export async function validateGraphQlDocuments(
15+
export function validateGraphQlDocuments(
2616
schema: GraphQLSchema,
27-
documentFiles: Source[],
28-
effectiveRules: ValidationRule[] = createDefaultRules()
29-
): Promise<ReadonlyArray<LoadDocumentError>> {
30-
const allFragmentMap = new Map<string, FragmentDefinitionNode>();
31-
const documentFileObjectsToValidate: {
32-
location?: string;
33-
document: DocumentNode;
34-
}[] = [];
35-
36-
for (const documentFile of documentFiles) {
37-
if (documentFile.document) {
38-
const definitionsToValidate: DefinitionNode[] = [];
39-
for (const definitionNode of documentFile.document.definitions) {
40-
if (definitionNode.kind === Kind.FRAGMENT_DEFINITION) {
41-
allFragmentMap.set(definitionNode.name.value, definitionNode);
42-
} else {
43-
definitionsToValidate.push(definitionNode);
44-
}
17+
documents: DocumentNode[],
18+
rules: ValidationRule[] = createDefaultRules()
19+
) {
20+
const definitionMap = new Map<string, DefinitionNode>();
21+
for (const document of documents) {
22+
for (const docDefinition of document.definitions) {
23+
if ('name' in docDefinition && docDefinition.name) {
24+
definitionMap.set(docDefinition.name.value, docDefinition);
25+
} else {
26+
definitionMap.set(Date.now().toString(), docDefinition);
4527
}
46-
documentFileObjectsToValidate.push({
47-
location: documentFile.location,
48-
document: {
49-
kind: Kind.DOCUMENT,
50-
definitions: definitionsToValidate,
51-
},
52-
});
5328
}
5429
}
55-
56-
const allErrors: LoadDocumentError[] = [];
57-
58-
const allFragmentsDocument: DocumentNode = {
30+
const fullAST: DocumentNode = {
5931
kind: Kind.DOCUMENT,
60-
definitions: [...allFragmentMap.values()],
32+
definitions: Array.from(definitionMap.values()),
6133
};
62-
63-
await Promise.all(
64-
documentFileObjectsToValidate.map(async documentFile => {
65-
const documentToValidate = concatAST([allFragmentsDocument, documentFile.document]);
66-
67-
const errors = validate(schema, documentToValidate, effectiveRules);
68-
69-
if (errors.length > 0) {
70-
allErrors.push({
71-
filePath: documentFile.location,
72-
errors,
73-
});
74-
}
75-
})
76-
);
77-
78-
return allErrors;
79-
}
80-
81-
export function checkValidationErrors(loadDocumentErrors: ReadonlyArray<LoadDocumentError>): void | never {
82-
if (loadDocumentErrors.length > 0) {
83-
const errors: Error[] = [];
84-
85-
for (const loadDocumentError of loadDocumentErrors) {
86-
for (const graphQLError of loadDocumentError.errors) {
87-
const error = new Error();
88-
error.name = 'GraphQLDocumentError';
89-
error.message = `${error.name}: ${graphQLError.message}`;
90-
error.stack = error.message;
91-
if (graphQLError.locations) {
92-
for (const location of graphQLError.locations) {
93-
error.stack += `\n at ${loadDocumentError.filePath}:${location.line}:${location.column}`;
94-
}
95-
}
96-
97-
errors.push(error);
34+
const errors = validate(schema, fullAST, rules);
35+
for (const error of errors) {
36+
error.stack = error.message;
37+
if (error.locations) {
38+
for (const location of error.locations) {
39+
error.stack += `\n at ${error.source?.name}:${location.line}:${location.column}`;
9840
}
9941
}
100-
101-
throw new AggregateError(
102-
errors,
103-
`GraphQL Document Validation failed with ${errors.length} errors;
104-
${errors.map((error, index) => `Error ${index}: ${error.stack}`).join('\n\n')}`
105-
);
10642
}
43+
return errors;
10744
}
10845

10946
export function createDefaultRules() {

‎packages/utils/tests/validate-documents.spec.ts‎

Lines changed: 23 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { checkValidationErrors, validateGraphQlDocuments } from '../src/index.js';
2-
import { buildSchema, parse, GraphQLError } from 'graphql';
1+
import { validateGraphQlDocuments } from '../src/index.js';
2+
import { buildSchema, parse, GraphQLError, Source } from 'graphql';
33

44
describe('validateGraphQlDocuments', () => {
55
it('Should throw an informative error when validation errors happens, also check for fragments validation even why they are duplicated', async () => {
@@ -25,146 +25,33 @@ describe('validateGraphQlDocuments', () => {
2525
}
2626
`;
2727

28-
const result = await validateGraphQlDocuments(schema, [
29-
{
30-
location: 'fragment.graphql',
31-
document: parse(fragment),
32-
},
33-
{
34-
location: 'query.graphql',
35-
document: parse(/* GraphQL */ `
36-
query searchPage {
37-
otherStuff {
38-
foo
28+
const result = validateGraphQlDocuments(schema, [
29+
parse(new Source(fragment, 'packages/client/src/fragments/pizzeriaFragment.fragment.graphql')),
30+
parse(
31+
new Source(
32+
/* GraphQL */ `
33+
query searchPage {
34+
otherStuff {
35+
foo
36+
}
37+
...pizzeriaFragment
3938
}
40-
...pizzeriaFragment
41-
}
4239
43-
${fragment}
44-
`),
45-
},
40+
${fragment}
41+
`,
42+
'packages/client/src/pages/search/searchPage.query.graphql'
43+
)
44+
),
4645
]);
4746

4847
expect(result).toHaveLength(1);
49-
expect(result[0].filePath).toBe('query.graphql');
50-
expect(result[0].errors[0] instanceof GraphQLError).toBeTruthy();
51-
expect(result[0].errors[0].message).toBe(
48+
expect(result[0].source?.name).toBe('packages/client/src/pages/search/searchPage.query.graphql');
49+
expect(result[0] instanceof GraphQLError).toBeTruthy();
50+
expect(result[0].message).toBe(
5251
'Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".'
5352
);
54-
55-
try {
56-
checkValidationErrors(result);
57-
expect(true).toBeFalsy();
58-
} catch (aggregateError: any) {
59-
const { errors } = aggregateError;
60-
expect(Symbol.iterator in errors).toBeTruthy();
61-
const generator = errors[Symbol.iterator]();
62-
63-
const error = generator.next().value;
64-
65-
expect(error).toBeInstanceOf(Error);
66-
expect(error.name).toEqual('GraphQLDocumentError');
67-
expect(error.message).toEqual(
68-
'GraphQLDocumentError: Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".'
69-
);
70-
expect(error.stack).toEqual(
71-
[
72-
'GraphQLDocumentError: Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".',
73-
' at query.graphql:6:13',
74-
].join('\n')
75-
);
76-
}
77-
});
78-
});
79-
80-
describe('checkValidationErrors', () => {
81-
it('Should throw errors source files and locations', async () => {
82-
const loadDocumentErrors = [
83-
{
84-
filePath: 'packages/server/src/modules/github-check-run/providers/documents/create-check-run.mutation.graphql',
85-
errors: [
86-
{
87-
message: 'Cannot query field "randomField" on type "CheckRun".',
88-
locations: [
89-
{
90-
line: 7,
91-
column: 13,
92-
},
93-
],
94-
},
95-
{
96-
message: 'Cannot query field "randomField2" on type "CheckRun".',
97-
locations: [
98-
{
99-
line: 8,
100-
column: 13,
101-
},
102-
],
103-
},
104-
],
105-
},
106-
{
107-
filePath: 'packages/server/src/modules/github-check-run/providers/documents/check-run.query.graphql',
108-
errors: [
109-
{
110-
message: 'Cannot query field "randomField" on type "CheckRun".',
111-
locations: [
112-
{
113-
line: 7,
114-
column: 13,
115-
},
116-
],
117-
},
118-
],
119-
},
120-
];
121-
122-
let errors;
123-
try {
124-
checkValidationErrors(loadDocumentErrors as any);
125-
} catch (aggregateError: any) {
126-
errors = aggregateError.errors;
127-
}
128-
129-
expect(Symbol.iterator in errors).toBeTruthy();
130-
131-
let error;
132-
const generator = errors[Symbol.iterator]();
133-
134-
error = generator.next().value;
135-
136-
expect(error).toBeInstanceOf(Error);
137-
expect(error.name).toEqual('GraphQLDocumentError');
138-
expect(error.message).toEqual('GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".');
139-
expect(error.stack).toEqual(
140-
[
141-
'GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".',
142-
' at packages/server/src/modules/github-check-run/providers/documents/create-check-run.mutation.graphql:7:13',
143-
].join('\n')
144-
);
145-
146-
error = generator.next().value;
147-
148-
expect(error).toBeInstanceOf(Error);
149-
expect(error.name).toEqual('GraphQLDocumentError');
150-
expect(error.message).toEqual('GraphQLDocumentError: Cannot query field "randomField2" on type "CheckRun".');
151-
expect(error.stack).toEqual(
152-
[
153-
'GraphQLDocumentError: Cannot query field "randomField2" on type "CheckRun".',
154-
' at packages/server/src/modules/github-check-run/providers/documents/create-check-run.mutation.graphql:8:13',
155-
].join('\n')
156-
);
157-
158-
error = generator.next().value;
159-
160-
expect(error).toBeInstanceOf(Error);
161-
expect(error.name).toEqual('GraphQLDocumentError');
162-
expect(error.message).toEqual('GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".');
163-
expect(error.stack).toEqual(
164-
[
165-
'GraphQLDocumentError: Cannot query field "randomField" on type "CheckRun".',
166-
' at packages/server/src/modules/github-check-run/providers/documents/check-run.query.graphql:7:13',
167-
].join('\n')
168-
);
53+
expect(result[0].stack)
54+
.toBe(`Fragment "pizzeriaFragment" cannot be spread here as objects of type "Query" can never be of type "Pizzeria".
55+
at packages/client/src/pages/search/searchPage.query.graphql:6:15`);
16956
});
17057
});

0 commit comments

Comments
 (0)
⚡