Merge pull request #717 from wincent/glh/degrade-to-warn

Soften name validation warnings to avoid CI issues

Author: wincent

src/type/__tests__/validation-test.js

@@ -356,9 +356,9 @@ describe('Type System: Objects must have fields', () => {
 
   it('warns about an Object type with reserved named fields', () => {
     /* eslint-disable no-console */
-    const realConsoleError = console.error;
+    const realConsoleWarn = console.warn;
     const calls = [];
-    console.error = function () {
+    console.warn = function () {
       calls.push(Array.prototype.slice.call(arguments));
     };
     try {
@@ -371,7 +371,7 @@ describe('Type System: Objects must have fields', () => {
         'Name "__notPartOfIntrospection" must not begin with "__", which is reserved by GraphQL introspection.'
       );
     } finally {
-      console.error = realConsoleError;
+      console.warn = realConsoleWarn;
     }
     /* eslint-enable no-console */
   });

src/utilities/__tests__/assertValidName-test.js

@@ -0,0 +1,139 @@
+/**
+ *  Copyright (c) 2015, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import { formatWarning } from '../assertValidName';
+
+/**
+ * Helper for dedenting indented template literals. This helps us to
+ * keep the tests pretty.
+ */
+function dedent(string) {
+  // Get lines, discarding empty leading and trailing lines.
+  const lines = string.replace(/^[ \t]*\n|\n[ \t]*$/g, '').split('\n');
+
+  // Find smallest indent.
+  const indent = lines.reduce((currentMinimum, line) => {
+    const whitespace = line.match(/^ +/);
+    return Math.min(
+      whitespace ? whitespace[0].length : 0,
+      currentMinimum
+    );
+  }, Infinity);
+
+  // Remove indent from each line.
+  return lines.map(line => line.slice(indent)).join('\n');
+}
+
+/**
+ * Convenience method for creating an Error object with a defined stack.
+ */
+function createErrorObject(message, stack) {
+  const error = new Error(message);
+  error.stack = stack;
+  return error;
+}
+
+describe('formatWarning()', () => {
+  it('formats given a Chrome-style stack property', () => {
+    const chromeStack = dedent(`
+      Error: foo
+        at z (<anonymous>:1:21)
+        at y (<anonymous>:1:15)
+        at x (<anonymous>:1:15)
+        at <anonymous>:1:6
+    `);
+    const error = createErrorObject('foo', chromeStack);
+    expect(formatWarning(error)).to.equal(dedent(`
+      foo
+        at z (<anonymous>:1:21)
+        at y (<anonymous>:1:15)
+        at x (<anonymous>:1:15)
+        at <anonymous>:1:6
+    `));
+  });
+
+  it('formats given a Node-style stack property', () => {
+    const nodeStack = dedent(`
+      Error: foo
+        at z (repl:1:29)
+        at y (repl:1:23)
+        at x (repl:1:23)
+        at repl:1:6
+        at ContextifyScript.Script.runInThisContext (vm.js:23:33)
+        at REPLServer.defaultEval (repl.js:340:29)
+        at bound (domain.js:280:14)
+        at REPLServer.runBound [as eval] (domain.js:293:12)
+        at REPLServer.onLine (repl.js:537:10)
+        at emitOne (events.js:101:20)
+    `);
+    const error = createErrorObject('foo', nodeStack);
+    expect(formatWarning(error)).to.equal(dedent(`
+      foo
+        at z (repl:1:29)
+        at y (repl:1:23)
+        at x (repl:1:23)
+        at repl:1:6
+        at ContextifyScript.Script.runInThisContext (vm.js:23:33)
+        at REPLServer.defaultEval (repl.js:340:29)
+        at bound (domain.js:280:14)
+        at REPLServer.runBound [as eval] (domain.js:293:12)
+        at REPLServer.onLine (repl.js:537:10)
+        at emitOne (events.js:101:20)
+    `));
+  });
+
+  it('formats given a Firefox-style stack property', () => {
+    const firefoxStack = dedent(`
+      z@debugger eval code:1:20
+      y@debugger eval code:1:14
+      x@debugger eval code:1:14
+      @debugger eval code:1:5
+    `);
+    const error = createErrorObject('foo', firefoxStack);
+    expect(formatWarning(error)).to.equal(dedent(`
+      foo
+      z@debugger eval code:1:20
+      y@debugger eval code:1:14
+      x@debugger eval code:1:14
+      @debugger eval code:1:5
+    `));
+  });
+
+  it('formats given a Safari-style stack property', () => {
+    const safariStack = dedent(`
+      z
+      y
+      x
+      global code
+      evaluateWithScopeExtension@[native code]
+      _evaluateOn
+      _evaluateAndWrap
+      evaluate
+    `);
+    const error = createErrorObject('foo', safariStack);
+    expect(formatWarning(error)).to.equal(dedent(`
+      foo
+      z
+      y
+      x
+      global code
+      evaluateWithScopeExtension@[native code]
+      _evaluateOn
+      _evaluateAndWrap
+      evaluate
+    `));
+  });
+
+  it('formats in the absence of a stack property', () => {
+    const error = createErrorObject('foo');
+    expect(formatWarning(error)).to.equal('foo');
+  });
+});

src/utilities/assertValidName.js

@@ -9,6 +9,7 @@
  */
 
 const NAME_RX = /^[_a-zA-Z][_a-zA-Z0-9]*$/;
+const ERROR_PREFIX_RX = /^Error: /;
 
 // Ensures console warnings are only issued once.
 let hasWarnedAboutDunder = false;
@@ -28,12 +29,13 @@ export function assertValidName(
   if (!isIntrospection && name.slice(0, 2) === '__' && !hasWarnedAboutDunder) {
     hasWarnedAboutDunder = true;
     /* eslint-disable no-console */
-    if (console && console.error) {
+    if (console && console.warn) {
       const error = new Error(
         `Name "${name}" must not begin with "__", which is reserved by ` +
-        'GraphQL introspection.'
+        'GraphQL introspection. In a future release of graphql this will ' +
+        'become a hard error.'
       );
-      console.error(error.stack || String(error));
+      console.warn(formatWarning(error));
     }
     /* eslint-enable no-console */
   }
@@ -43,3 +45,20 @@ export function assertValidName(
     );
   }
 }
+
+/**
+ * Returns a human-readable warning based an the supplied Error object,
+ * including stack trace information if available.
+ */
+export function formatWarning(error: Error): string {
+  let formatted = '';
+  const errorString = String(error).replace(ERROR_PREFIX_RX, '');
+  const stack = error.stack;
+  if (stack) {
+    formatted = stack.replace(ERROR_PREFIX_RX, '');
+  }
+  if (formatted.indexOf(errorString) === -1) {
+    formatted = errorString + '\n' + formatted;
+  }
+  return formatted.trim();
+}