Skip to content

Commit f987479

Browse files
committed
Updated to most recent version of spec proposal.
This is now *a breaking change*. The default validation rules are stricter, however with a configuration flag the previous lax behavior can be used which will ensure an existing service can support all existing incoming operations. For example to continue to support existing queries after updating to the new version, replace: ```js graphql({ schema, source }) ``` With: ```js graphql({ schema, source, options: { allowNullableVariablesInNonNullPositions: true }}) ``` Another more minor breaking change is that the final `typeInfo` argument to `validate` has moved positions. However very few should be reliant on this experimental arg.
1 parent ace990f commit f987479

9 files changed

+149
-70
lines changed

src/graphql.js

+18-4
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99

1010
import { validateSchema } from './type/validate';
1111
import { parse } from './language/parser';
12-
import { validate } from './validation/validate';
12+
import { validate, specifiedRules } from './validation';
1313
import { execute } from './execution/execute';
1414
import type { ObjMap } from './jsutils/ObjMap';
15-
import type { Source } from './language/source';
15+
import type { ParseOptions, Source } from './language';
1616
import type { GraphQLFieldResolver } from './type/definition';
1717
import type { GraphQLSchema } from './type/schema';
1818
import type { ExecutionResult } from './execution/execute';
19+
import type { ValidationOptions } from './validation';
1920
import type { MaybePromise } from './jsutils/MaybePromise';
2021

2122
/**
@@ -47,6 +48,9 @@ import type { MaybePromise } from './jsutils/MaybePromise';
4748
* A resolver function to use when one is not provided by the schema.
4849
* If not provided, the default field resolver is used (which looks for a
4950
* value or method on the source value with the field's name).
51+
* options:
52+
* An additional set of flags which enable legacy, compataibility, or
53+
* experimental behavior.
5054
*/
5155
export type GraphQLArgs = {|
5256
schema: GraphQLSchema,
@@ -56,6 +60,7 @@ export type GraphQLArgs = {|
5660
variableValues?: ?ObjMap<mixed>,
5761
operationName?: ?string,
5862
fieldResolver?: ?GraphQLFieldResolver<any, any>,
63+
options?: ParseOptions & ValidationOptions,
5964
|};
6065
declare function graphql(GraphQLArgs, ..._: []): Promise<ExecutionResult>;
6166
/* eslint-disable no-redeclare */
@@ -67,6 +72,7 @@ declare function graphql(
6772
variableValues?: ?ObjMap<mixed>,
6873
operationName?: ?string,
6974
fieldResolver?: ?GraphQLFieldResolver<any, any>,
75+
options?: ParseOptions & ValidationOptions,
7076
): Promise<ExecutionResult>;
7177
export function graphql(
7278
argsOrSchema,
@@ -76,6 +82,7 @@ export function graphql(
7682
variableValues,
7783
operationName,
7884
fieldResolver,
85+
options,
7986
) {
8087
/* eslint-enable no-redeclare */
8188
// Always return a Promise for a consistent API.
@@ -91,6 +98,7 @@ export function graphql(
9198
argsOrSchema.variableValues,
9299
argsOrSchema.operationName,
93100
argsOrSchema.fieldResolver,
101+
argsOrSchema.options,
94102
)
95103
: graphqlImpl(
96104
argsOrSchema,
@@ -100,6 +108,7 @@ export function graphql(
100108
variableValues,
101109
operationName,
102110
fieldResolver,
111+
options,
103112
),
104113
),
105114
);
@@ -121,6 +130,7 @@ declare function graphqlSync(
121130
variableValues?: ?ObjMap<mixed>,
122131
operationName?: ?string,
123132
fieldResolver?: ?GraphQLFieldResolver<any, any>,
133+
options?: ParseOptions & ValidationOptions,
124134
): ExecutionResult;
125135
export function graphqlSync(
126136
argsOrSchema,
@@ -130,6 +140,7 @@ export function graphqlSync(
130140
variableValues,
131141
operationName,
132142
fieldResolver,
143+
options,
133144
) {
134145
// Extract arguments from object args if provided.
135146
const result =
@@ -142,6 +153,7 @@ export function graphqlSync(
142153
argsOrSchema.variableValues,
143154
argsOrSchema.operationName,
144155
argsOrSchema.fieldResolver,
156+
argsOrSchema.options,
145157
)
146158
: graphqlImpl(
147159
argsOrSchema,
@@ -151,6 +163,7 @@ export function graphqlSync(
151163
variableValues,
152164
operationName,
153165
fieldResolver,
166+
options,
154167
);
155168

156169
// Assert that the execution was synchronous.
@@ -169,6 +182,7 @@ function graphqlImpl(
169182
variableValues,
170183
operationName,
171184
fieldResolver,
185+
options,
172186
): MaybePromise<ExecutionResult> {
173187
// Validate Schema
174188
const schemaValidationErrors = validateSchema(schema);
@@ -179,13 +193,13 @@ function graphqlImpl(
179193
// Parse
180194
let document;
181195
try {
182-
document = parse(source);
196+
document = parse(source, options);
183197
} catch (syntaxError) {
184198
return { errors: [syntaxError] };
185199
}
186200

187201
// Validate
188-
const validationErrors = validate(schema, document);
202+
const validationErrors = validate(schema, document, specifiedRules, options);
189203
if (validationErrors.length > 0) {
190204
return { errors: validationErrors };
191205
}

src/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ export { subscribe, createSourceEventStream } from './subscription';
268268
// Validate GraphQL queries.
269269
export {
270270
validate,
271+
ValidationOptions,
271272
ValidationContext,
272273
// All validation rules in the GraphQL Specification.
273274
specifiedRules,

src/validation/ValidationContext.js

+18
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,21 @@ import { TypeInfo } from '../utilities/TypeInfo';
3333
type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode;
3434
type VariableUsage = { node: VariableNode, type: ?GraphQLInputType };
3535

36+
/**
37+
* Configuration options to control validation behavior.
38+
*/
39+
export type ValidationOptions = {
40+
/**
41+
* If enabled, validation will allow nullable variables with default values
42+
* to be used in non-null positions. Earlier versions explicitly allowed such
43+
* operations due to a slightly different interpretation of default values and
44+
* null values. GraphQL services accepting operations written before this
45+
* version may continue to allow such operations by enabling this option,
46+
* however GraphQL services established after this version should not.
47+
*/
48+
allowNullableVariablesInNonNullPositions?: boolean,
49+
};
50+
3651
/**
3752
* An instance of this class is passed as the "this" context to all validators,
3853
* allowing access to commonly useful contextual information from within a
@@ -42,6 +57,7 @@ export default class ValidationContext {
4257
_schema: GraphQLSchema;
4358
_ast: DocumentNode;
4459
_typeInfo: TypeInfo;
60+
options: ValidationOptions;
4561
_errors: Array<GraphQLError>;
4662
_fragments: ObjMap<FragmentDefinitionNode>;
4763
_fragmentSpreads: Map<SelectionSetNode, $ReadOnlyArray<FragmentSpreadNode>>;
@@ -59,10 +75,12 @@ export default class ValidationContext {
5975
schema: GraphQLSchema,
6076
ast: DocumentNode,
6177
typeInfo: TypeInfo,
78+
options?: ValidationOptions,
6279
): void {
6380
this._schema = schema;
6481
this._ast = ast;
6582
this._typeInfo = typeInfo;
83+
this.options = options || {};
6684
this._errors = [];
6785
this._fragmentSpreads = new Map();
6886
this._recursivelyReferencedFragments = new Map();

src/validation/__tests__/VariablesInAllowedPosition-test.js

+75-47
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@
66
*/
77

88
import { describe, it } from 'mocha';
9-
import { expectPassesRule, expectFailsRule } from './harness';
9+
import {
10+
expectPassesRule,
11+
expectFailsRule,
12+
expectPassesRuleWithOptions,
13+
expectFailsRuleWithOptions,
14+
} from './harness';
1015
import {
1116
VariablesInAllowedPosition,
1217
badVarPosMessage,
@@ -91,20 +96,6 @@ describe('Validate: Variables are in allowed positions', () => {
9196
);
9297
});
9398

94-
it('Int => Int! with non-null default value', () => {
95-
expectPassesRule(
96-
VariablesInAllowedPosition,
97-
`
98-
query Query($intArg: Int = 1)
99-
{
100-
complicatedArgs {
101-
nonNullIntArgField(nonNullIntArg: $intArg)
102-
}
103-
}
104-
`,
105-
);
106-
});
107-
10899
it('[String] => [String]', () => {
109100
expectPassesRule(
110101
VariablesInAllowedPosition,
@@ -201,18 +192,6 @@ describe('Validate: Variables are in allowed positions', () => {
201192
);
202193
});
203194

204-
it('Boolean => Boolean! in directive with default', () => {
205-
expectPassesRule(
206-
VariablesInAllowedPosition,
207-
`
208-
query Query($boolVar: Boolean = false)
209-
{
210-
dog @include(if: $boolVar)
211-
}
212-
`,
213-
);
214-
});
215-
216195
it('Int => Int!', () => {
217196
expectFailsRule(
218197
VariablesInAllowedPosition,
@@ -233,26 +212,6 @@ describe('Validate: Variables are in allowed positions', () => {
233212
);
234213
});
235214

236-
it('Int => Int! with null default value', () => {
237-
expectFailsRule(
238-
VariablesInAllowedPosition,
239-
`
240-
query Query($intArg: Int = null) {
241-
complicatedArgs {
242-
nonNullIntArgField(nonNullIntArg: $intArg)
243-
}
244-
}
245-
`,
246-
[
247-
{
248-
message: badVarPosMessage('intArg', 'Int', 'Int!'),
249-
locations: [{ line: 2, column: 19 }, { line: 4, column: 45 }],
250-
path: undefined,
251-
},
252-
],
253-
);
254-
});
255-
256215
it('Int => Int! within fragment', () => {
257216
expectFailsRule(
258217
VariablesInAllowedPosition,
@@ -401,4 +360,73 @@ describe('Validate: Variables are in allowed positions', () => {
401360
],
402361
);
403362
});
363+
364+
describe('allowNullableVariablesInNonNullPositions', () => {
365+
it('Int => Int! with non-null default value without option', () => {
366+
expectFailsRule(
367+
VariablesInAllowedPosition,
368+
`
369+
query Query($intVar: Int = 1) {
370+
complicatedArgs {
371+
nonNullIntArgField(nonNullIntArg: $intVar)
372+
}
373+
}
374+
`,
375+
[
376+
{
377+
message: badVarPosMessage('intVar', 'Int', 'Int!'),
378+
locations: [{ line: 2, column: 21 }, { line: 4, column: 47 }],
379+
path: undefined,
380+
},
381+
],
382+
);
383+
});
384+
385+
it('Int => Int! with null default value fails with option', () => {
386+
expectFailsRuleWithOptions(
387+
{ allowNullableVariablesInNonNullPositions: true },
388+
VariablesInAllowedPosition,
389+
`
390+
query Query($intVar: Int = null) {
391+
complicatedArgs {
392+
nonNullIntArgField(nonNullIntArg: $intVar)
393+
}
394+
}
395+
`,
396+
[
397+
{
398+
message: badVarPosMessage('intVar', 'Int', 'Int!'),
399+
locations: [{ line: 2, column: 23 }, { line: 4, column: 49 }],
400+
path: undefined,
401+
},
402+
],
403+
);
404+
});
405+
406+
it('Int => Int! with non-null default value with option', () => {
407+
expectPassesRuleWithOptions(
408+
{ allowNullableVariablesInNonNullPositions: true },
409+
VariablesInAllowedPosition,
410+
`
411+
query Query($intVar: Int = 1) {
412+
complicatedArgs {
413+
nonNullIntArgField(nonNullIntArg: $intVar)
414+
}
415+
}
416+
`,
417+
);
418+
});
419+
420+
it('Boolean => Boolean! in directive with default value with option', () => {
421+
expectPassesRuleWithOptions(
422+
{ allowNullableVariablesInNonNullPositions: true },
423+
VariablesInAllowedPosition,
424+
`
425+
query Query($boolVar: Boolean = false) {
426+
dog @include(if: $boolVar)
427+
}
428+
`,
429+
);
430+
});
431+
});
404432
});

src/validation/__tests__/harness.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -422,13 +422,15 @@ export const testSchema = new GraphQLSchema({
422422
],
423423
});
424424

425-
function expectValid(schema, rules, queryString) {
426-
const errors = validate(schema, parse(queryString), rules);
425+
function expectValid(schema, rules, queryString, options) {
426+
const ast = parse(queryString);
427+
const errors = validate(schema, ast, rules, options);
427428
expect(errors).to.deep.equal([], 'Should validate');
428429
}
429430

430-
function expectInvalid(schema, rules, queryString, expectedErrors) {
431-
const errors = validate(schema, parse(queryString), rules);
431+
function expectInvalid(schema, rules, queryString, expectedErrors, options) {
432+
const ast = parse(queryString);
433+
const errors = validate(schema, ast, rules, options);
432434
expect(errors).to.have.length.of.at.least(1, 'Should not validate');
433435
expect(errors.map(formatError)).to.deep.equal(expectedErrors);
434436
return errors;
@@ -442,6 +444,14 @@ export function expectFailsRule(rule, queryString, errors) {
442444
return expectInvalid(testSchema, [rule], queryString, errors);
443445
}
444446

447+
export function expectPassesRuleWithOptions(options, rule, queryString) {
448+
return expectValid(testSchema, [rule], queryString, options);
449+
}
450+
451+
export function expectFailsRuleWithOptions(options, rule, queryString, errors) {
452+
return expectInvalid(testSchema, [rule], queryString, errors, options);
453+
}
454+
445455
export function expectPassesRuleWithSchema(schema, rule, queryString, errors) {
446456
return expectValid(schema, [rule], queryString, errors);
447457
}

src/validation/__tests__/validation-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ describe('Validate: Supports full validation', () => {
7373
}
7474
`);
7575

76-
const errors = validate(testSchema, ast, specifiedRules, typeInfo);
76+
const errors = validate(testSchema, ast, specifiedRules, {}, typeInfo);
7777

7878
const errorMessages = errors.map(err => err.message);
7979

src/validation/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export { validate } from './validate';
1212
// https://github.com/tc39/proposal-export-default-from
1313
import ValidationContext from './ValidationContext';
1414
export { ValidationContext };
15+
export type { ValidationOptions } from './ValidationContext';
1516

1617
export { specifiedRules } from './specifiedRules';
1718

0 commit comments

Comments
 (0)