Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 'invariant' only for real invariants, add 'devAssert' for the rest #2066

Merged
merged 1 commit into from Jul 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .flowconfig
Expand Up @@ -26,7 +26,7 @@ unsafe-getters-setters=error
inexact-spread=error
implicit-inexact-object=error
unnecessary-optional-chain=error
unnecessary-invariant=off
unnecessary-invariant=error
signature-verification-failure=error
uninitialized-instance-property=error
non-array-spread=error
Expand Down
31 changes: 18 additions & 13 deletions resources/inline-invariant.js
Expand Up @@ -14,32 +14,37 @@
* !<cond> ? invariant(0, ...) : undefined;
*/
module.exports = function inlineInvariant(context) {
const replaceTemplate = context.template(`
const invariantTemplate = context.template(`
(%%cond%%) || invariant(0, %%args%%)
`);
const assertTemplate = context.template(`
(%%cond%%) || devAssert(0, %%args%%)
`);

return {
visitor: {
CallExpression(path) {
const node = path.node;
const parent = path.parent;

if (!isAppropriateInvariantCall(node, parent)) {
if (
parent.type !== 'ExpressionStatement' ||
node.callee.type !== 'Identifier' ||
node.arguments.length === 0
) {
return;
}

const [cond, args] = node.arguments;
path.replaceWith(replaceTemplate({ cond, args }));
const calleeName = node.callee.name;
if (calleeName === 'invariant') {
const [cond, args] = node.arguments;
path.addComment('leading', ' istanbul ignore next ');
path.replaceWith(invariantTemplate({ cond, args }));
} else if (calleeName === 'devAssert') {
const [cond, args] = node.arguments;
path.replaceWith(assertTemplate({ cond, args }));
}
},
},
};
};

function isAppropriateInvariantCall(node, parent) {
return (
parent.type === 'ExpressionStatement' &&
node.callee.type === 'Identifier' &&
node.callee.name === 'invariant' &&
node.arguments.length > 0
);
}
4 changes: 2 additions & 2 deletions src/error/formatError.js
@@ -1,6 +1,6 @@
// @flow strict

import invariant from '../jsutils/invariant';
import devAssert from '../jsutils/devAssert';

import { type SourceLocation } from '../language/location';

Expand All @@ -11,7 +11,7 @@ import { type GraphQLError } from './GraphQLError';
* Response Format, Errors section of the GraphQL Specification.
*/
export function formatError(error: GraphQLError): GraphQLFormattedError {
invariant(error, 'Received null or undefined error.');
devAssert(error, 'Received null or undefined error.');
const message = error.message || 'An unknown error occurred.';
const locations = error.locations;
const path = error.path;
Expand Down
15 changes: 8 additions & 7 deletions src/execution/execute.js
Expand Up @@ -4,7 +4,7 @@ import { forEach, isCollection } from 'iterall';

import inspect from '../jsutils/inspect';
import memoize3 from '../jsutils/memoize3';
import invariant from '../jsutils/invariant';
import devAssert from '../jsutils/devAssert';
import isInvalid from '../jsutils/isInvalid';
import isNullish from '../jsutils/isNullish';
import isPromise from '../jsutils/isPromise';
Expand Down Expand Up @@ -249,13 +249,13 @@ export function assertValidExecutionArguments(
document: DocumentNode,
rawVariableValues: ?{ +[variable: string]: mixed, ... },
): void {
invariant(document, 'Must provide document');
devAssert(document, 'Must provide document');

// If the schema used for execution is invalid, throw an error.
assertValidSchema(schema);

// Variables, if provided, must be an object.
invariant(
devAssert(
rawVariableValues == null || isObjectLike(rawVariableValues),
'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.',
);
Expand Down Expand Up @@ -892,10 +892,11 @@ function completeListValue(
path: Path,
result: mixed,
): PromiseOrValue<$ReadOnlyArray<mixed>> {
invariant(
isCollection(result),
`Expected Iterable, but did not find one for field ${info.parentType.name}.${info.fieldName}.`,
);
if (!isCollection(result)) {
throw new GraphQLError(
`Expected Iterable, but did not find one for field ${info.parentType.name}.${info.fieldName}.`,
);
}

// This is specified as a simple map, however we're optimizing the path
// where the list contains no Promises by avoiding creating another Promise.
Expand Down
10 changes: 10 additions & 0 deletions src/jsutils/devAssert.js
@@ -0,0 +1,10 @@
// @flow strict

/* istanbul ignore file */
export default function devAssert(condition: mixed, message: string): void {
const booleanCondition = Boolean(condition);
/* istanbul ignore else */
if (!booleanCondition) {
throw new Error(message);
}
}
4 changes: 2 additions & 2 deletions src/language/parser.js
@@ -1,7 +1,7 @@
// @flow strict

import inspect from '../jsutils/inspect';
import invariant from '../jsutils/invariant';
import devAssert from '../jsutils/devAssert';
import defineToJSON from '../jsutils/defineToJSON';

import { syntaxError } from '../error/syntaxError';
Expand Down Expand Up @@ -173,7 +173,7 @@ class Parser {

constructor(source: string | Source, options?: ParseOptions) {
const sourceObj = typeof source === 'string' ? new Source(source) : source;
invariant(
devAssert(
sourceObj instanceof Source,
`Must provide Source. Received: ${inspect(sourceObj)}`,
);
Expand Down
6 changes: 3 additions & 3 deletions src/language/source.js
@@ -1,6 +1,6 @@
// @flow strict

import invariant from '../jsutils/invariant';
import devAssert from '../jsutils/devAssert';
import defineToStringTag from '../jsutils/defineToStringTag';

type Location = {|
Expand All @@ -25,11 +25,11 @@ export class Source {
this.body = body;
this.name = name || 'GraphQL request';
this.locationOffset = locationOffset || { line: 1, column: 1 };
invariant(
devAssert(
this.locationOffset.line > 0,
'line in locationOffset is 1-indexed and must be positive',
);
invariant(
devAssert(
this.locationOffset.column > 0,
'column in locationOffset is 1-indexed and must be positive',
);
Expand Down