Skip to content

Commit 246b998

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 1afc4dc commit 246b998

File tree

9 files changed

+149
-70
lines changed

9 files changed

+149
-70
lines changed

src/graphql.js

Lines changed: 18 additions & 4 deletions
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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 18 additions & 0 deletions
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

Lines changed: 75 additions & 47 deletions
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,
@@ -232,26 +211,6 @@ describe('Validate: Variables are in allowed positions', () => {
232211
);
233212
});
234213

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

src/validation/__tests__/harness.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -421,13 +421,15 @@ export const testSchema = new GraphQLSchema({
421421
],
422422
});
423423

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

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

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

src/validation/__tests__/validation-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ describe('Validate: Supports full validation', () => {
7272
}
7373
`);
7474

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

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

src/validation/index.js

Lines changed: 1 addition & 0 deletions
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)