Use 'invariant' only for real invariants, add 'devAssert' for t… (#2066)

Author: IvanGoncharov

.flowconfig

@@ -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

resources/inline-invariant.js

@@ -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
-  );
-}

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';
 
@@ -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;

src/execution/execute.js

@@ -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';
@@ -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.',
   );
@@ -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.

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);
+  }
+}

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';
@@ -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)}`,
     );

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 = {|
@@ -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',
     );