Skip to content

Commit a4b085b

Browse files
Remove 'devAssert' checks that duplicate TS types (#3642)
`graphql` provides TS types since `14.5.0` (released 3 years ago) and we fully switched to TS in `15.0.0` so I think it's time to drop runtime typechecks. Motivations: This type checks were added long time ago since we shifted towards TS we just maintained them without adding new ones. In general, this check increase bundle size add runtime cost and we can't realistically check all arguments to all functions. Instead we should focus on adding more asserts on stuff that can't be checked by TS.
1 parent e414279 commit a4b085b

21 files changed

+45
-664
lines changed

src/execution/__tests__/executor-test.ts

-48
Original file line numberDiff line numberDiff line change
@@ -22,54 +22,6 @@ import { GraphQLSchema } from '../../type/schema';
2222
import { execute, executeSync } from '../execute';
2323

2424
describe('Execute: Handles basic execution tasks', () => {
25-
it('throws if no document is provided', () => {
26-
const schema = new GraphQLSchema({
27-
query: new GraphQLObjectType({
28-
name: 'Type',
29-
fields: {
30-
a: { type: GraphQLString },
31-
},
32-
}),
33-
});
34-
35-
// @ts-expect-error
36-
expect(() => executeSync({ schema })).to.throw('Must provide document.');
37-
});
38-
39-
it('throws if no schema is provided', () => {
40-
const document = parse('{ field }');
41-
42-
// @ts-expect-error
43-
expect(() => executeSync({ document })).to.throw(
44-
'Expected undefined to be a GraphQL schema.',
45-
);
46-
});
47-
48-
it('throws on invalid variables', () => {
49-
const schema = new GraphQLSchema({
50-
query: new GraphQLObjectType({
51-
name: 'Type',
52-
fields: {
53-
fieldA: {
54-
type: GraphQLString,
55-
args: { argA: { type: GraphQLInt } },
56-
},
57-
},
58-
}),
59-
});
60-
const document = parse(`
61-
query ($a: Int) {
62-
fieldA(argA: $a)
63-
}
64-
`);
65-
const variableValues = '{ "a": 1 }';
66-
67-
// @ts-expect-error
68-
expect(() => executeSync({ schema, document, variableValues })).to.throw(
69-
'Variables must be provided as an Object where each property is a variable value. Perhaps look to see if an unparsed JSON string was provided.',
70-
);
71-
});
72-
7325
it('executes arbitrary code', async () => {
7426
const data = {
7527
a: () => 'Apple',

src/execution/__tests__/subscribe-test.ts

-31
Original file line numberDiff line numberDiff line change
@@ -390,37 +390,6 @@ describe('Subscription Initialization Phase', () => {
390390
});
391391
});
392392

393-
it('throws an error if some of required arguments are missing', async () => {
394-
const document = parse('subscription { foo }');
395-
const schema = new GraphQLSchema({
396-
query: DummyQueryType,
397-
subscription: new GraphQLObjectType({
398-
name: 'Subscription',
399-
fields: {
400-
foo: { type: GraphQLString },
401-
},
402-
}),
403-
});
404-
405-
// @ts-expect-error (schema must not be null)
406-
expect(() => subscribe({ schema: null, document })).to.throw(
407-
'Expected null to be a GraphQL schema.',
408-
);
409-
410-
// @ts-expect-error
411-
expect(() => subscribe({ document })).to.throw(
412-
'Expected undefined to be a GraphQL schema.',
413-
);
414-
415-
// @ts-expect-error (document must not be null)
416-
expect(() => subscribe({ schema, document: null })).to.throw(
417-
'Must provide document.',
418-
);
419-
420-
// @ts-expect-error
421-
expect(() => subscribe({ schema })).to.throw('Must provide document.');
422-
});
423-
424393
it('resolves to an error if schema does not support subscriptions', async () => {
425394
const schema = new GraphQLSchema({ query: DummyQueryType });
426395
const document = parse('subscription { unknownField }');

src/execution/execute.ts

+2-15
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { devAssert } from '../jsutils/devAssert';
21
import { inspect } from '../jsutils/inspect';
32
import { invariant } from '../jsutils/invariant';
43
import { isAsyncIterable } from '../jsutils/isAsyncIterable';
@@ -239,21 +238,9 @@ function buildResponse(
239238
* TODO: consider no longer exporting this function
240239
* @internal
241240
*/
242-
export function assertValidExecutionArguments(
243-
schema: GraphQLSchema,
244-
document: DocumentNode,
245-
rawVariableValues: Maybe<{ readonly [variable: string]: unknown }>,
246-
): void {
247-
devAssert(document != null, 'Must provide document.');
248-
241+
export function assertValidExecutionArguments(schema: GraphQLSchema): void {
249242
// If the schema used for execution is invalid, throw an error.
250243
assertValidSchema(schema);
251-
252-
// Variables, if provided, must be an object.
253-
devAssert(
254-
rawVariableValues == null || isObjectLike(rawVariableValues),
255-
'Variables must be provided as an Object where each property is a variable value. Perhaps look to see if an unparsed JSON string was provided.',
256-
);
257244
}
258245

259246
/**
@@ -281,7 +268,7 @@ export function buildExecutionContext(
281268
} = args;
282269

283270
// If arguments are missing or incorrect, throw an error.
284-
assertValidExecutionArguments(schema, document, rawVariableValues);
271+
assertValidExecutionArguments(schema);
285272

286273
let operation: OperationDefinitionNode | undefined;
287274
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);

src/language/__tests__/source-test.ts

-14
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,6 @@ import { describe, it } from 'mocha';
44
import { Source } from '../source';
55

66
describe('Source', () => {
7-
it('asserts that a body was provided', () => {
8-
// @ts-expect-error
9-
expect(() => new Source()).to.throw(
10-
'Body must be a string. Received: undefined.',
11-
);
12-
});
13-
14-
it('asserts that a valid body was provided', () => {
15-
// @ts-expect-error
16-
expect(() => new Source({})).to.throw(
17-
'Body must be a string. Received: {}.',
18-
);
19-
});
20-
217
it('can be Object.toStringified', () => {
228
const source = new Source('');
239

src/language/source.ts

-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { devAssert } from '../jsutils/devAssert';
2-
import { inspect } from '../jsutils/inspect';
32
import { instanceOf } from '../jsutils/instanceOf';
43

54
interface Location {
@@ -24,11 +23,6 @@ export class Source {
2423
name: string = 'GraphQL request',
2524
locationOffset: Location = { line: 1, column: 1 },
2625
) {
27-
devAssert(
28-
typeof body === 'string',
29-
`Body must be a string. Received: ${inspect(body)}.`,
30-
);
31-
3226
this.body = body;
3327
this.name = name;
3428
this.locationOffset = locationOffset;

src/type/__tests__/assertName-test.ts

-5
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ describe('assertName', () => {
88
expect(assertName('_ValidName123')).to.equal('_ValidName123');
99
});
1010

11-
it('throws for non-strings', () => {
12-
// @ts-expect-error
13-
expect(() => assertName({})).to.throw('Expected name to be a string.');
14-
});
15-
1611
it('throws on empty strings', () => {
1712
expect(() => assertName('')).to.throw(
1813
'Expected name to be a non-empty string.',

0 commit comments

Comments
 (0)