Upgrade babel and associated plugins to v7

[AaronCCWong]

This PR completes Issue #1266.

I have not added a matrix to get Travis to build on a different version of node before running tests in node v4 since I see that the PR #1338 drops support for node v4.

Do let me know if you want me to add it anyway. I have the Travis builds set up to run tests on node v4 successfully on a different branch.

[IvanGoncharov]

@AaronCCWong Awersome 👍
This PR looks great.
То be 100% sure that this PR doesn't introduce any unexpected changes can you please run a diff against npm run build output inside dist folder before and after your change.

[AaronCCWong]

@IvanGoncharov Good point. I am running a diff right now and unfortunately there are a horrible amount of differences. I am also realizing that __tests__ are no longer ignored by babel. I will have a commit that fixes that shortly.

You can check out the diff here: https://github.com/AaronCCWong/graphql-js/pull/4/files

The __tests__ folders are successfully ignored in that diff but there are still 300+ file changes...

[IvanGoncharov]

You can check out the diff here: AaronCCWong/pull/4/files

Not sure what is happening with this diff but it's invalid.
For example 'LICENSE' exists in both branches however it is shown as added:
https://github.com/AaronCCWong/graphql-js/blob/master-build/dist/LICENSE
https://github.com/AaronCCWong/graphql-js/blob/upgrade-babel-dist/dist/LICENSE

I suggest you do the following:

npm run build
mv dist oldDist
npm run build
diff oldDist dist -ru

[AaronCCWong]

Thanks! That is helpful. The diff is 26k+ lines long. It looks like most of the changes are due to single quotes in the old dist getting changed into double quotes and single line objects being separated into multiple lines.

What worries me is I am noticing in some places that certain comments in the old dist are not in the new dist.

[IvanGoncharov]

It looks like most of the changes are due to single quotes in the old dist getting changed into double quotes and single line objects being separated into multiple lines.

@AaronCCWong You can run prettier on both oldDist and newDist it will remove all discrepancies in code styling.

What worries me is I am noticing in some places that certain comments in the old dist are not in the new dist.

Can you please give some examples?

[AaronCCWong]

@IvanGoncharov This is the diff of GraphQLError.js after running prettier on both the old and new versions.

--- oldDist/error/GraphQLError.js	2018-05-17 12:07:12.000000000 -0400
+++ dist/error/GraphQLError.js	2018-05-17 12:42:03.000000000 -0400
@@ -10,12 +10,6 @@
 var _location = require('../language/location');
 
 /**
- * A GraphQLError describes an Error found during the parse, validate, or
- * execute phases of performing a GraphQL operation. In addition to a message
- * and stack trace, it also includes information about the locations in a
- * GraphQL document and/or execution result that correspond to the Error.
- */
-/**
  * Copyright (c) 2015-present, Facebook, Inc.
  *
  * This source code is licensed under the MIT license found in the
@@ -23,7 +17,6 @@
  *
  *  strict
  */
-
 function GraphQLError( // eslint-disable-line no-redeclare
   message,
   nodes,
@@ -40,29 +33,33 @@
       : undefined
     : nodes
       ? [nodes]
-      : undefined;
+      : undefined; // Compute locations in the source for the given nodes/positions.
 
-  // Compute locations in the source for the given nodes/positions.
   var _source = source;
+
   if (!_source && _nodes) {
     var node = _nodes[0];
     _source = node && node.loc && node.loc.source;
   }
 
   var _positions = positions;
+
   if (!_positions && _nodes) {
     _positions = _nodes.reduce(function(list, node) {
       if (node.loc) {
         list.push(node.loc.start);
       }
+
       return list;
     }, []);
   }
+
   if (_positions && _positions.length === 0) {
     _positions = undefined;
   }
 
-  var _locations = void 0;
+  var _locations;
+
   if (positions && source) {
     _locations = positions.map(function(pos) {
       return (0, _location.getLocation)(source, pos);
@@ -72,6 +69,7 @@
       if (node.loc) {
         list.push((0, _location.getLocation)(node.loc.source, node.loc.start));
       }
+
       return list;
     }, []);
   }
@@ -126,9 +124,8 @@
       // service adheres to the spec.
       enumerable: Boolean(_extensions),
     },
-  });
+  }); // Include (non-enumerable) stack trace.
 
-  // Include (non-enumerable) stack trace.
   if (originalError && originalError.stack) {
     Object.defineProperty(this, 'stack', {
       value: originalError.stack,
@@ -147,8 +144,12 @@
 }
 
 GraphQLError.prototype = Object.create(Error.prototype, {
-  constructor: { value: GraphQLError },
-  name: { value: 'GraphQLError' },
+  constructor: {
+    value: GraphQLError,
+  },
+  name: {
+    value: 'GraphQLError',
+  },
   toString: {
     value: function toString() {
       return (0, _printError.printError)(this);

Prettier isn't very consistent with the way it splits up objects. Also notice the comment that is missing in the new file at the top.

[IvanGoncharov]

@AaronCCWong Looks good 👍

Also notice the comment that is missing in the new file at the top.

I don't think we should worry about it especially since previously it was placed before license and it looked strange.

I think it's ready but we should merge #1338 first.

[AaronCCWong]

@IvanGoncharov I just want to make sure that you are aware that this is just one of the many files that have changes in them. The diff of the dist shows 20k+ line changes even after running prettier. Is there any way for me to make sure that this hasn't caused any regressions beyond just making sure the tests pass? I do not want to be that guy that broke everything. 😅

[IvanGoncharov]

Is there any way for me to make sure that this hasn't caused any regressions beyond just making sure the tests pass?

@AaronCCWong I don't think you should check every line in diff :)
I suggested you to use diff mostly to discover missing/unnecessary files, e.g. content of the __test__ folder.

As for general JS code, the exact same babel config applied over all files so if something goes wrong you should see this in first few files in a diff.
If you want to be extra careful it makes sense to look at changes in src/type/definition.js since it uses es6 classes and other advanced features.

Also, I just remembered that we have one custom babel plugin:
https://github.com/graphql/graphql-js/blob/master/resources/inline-invariant.js
Can you please check that invariants are correctly transformed with babel@7?

I do not want to be that guy that broke everything. 😅

Since the next version should be a first stable release 14.0.0 I expect that we will have some RCs before publishing it.

[AaronCCWong]

@IvanGoncharov The invariants do look like they are being inlined correctly.

I am also looking over src/type/definition.js. There are some differences. For example, we have

babel v6:

function GraphQLInputObjectType(config) {
    _classCallCheck(this, GraphQLInputObjectType);

    this.name = config.name;
    this.description = config.description;
    this.astNode = config.astNode;
    this._typeConfig = config;

and babel v7:

function GraphQLInputObjectType(config) {
      _defineProperty(
        _defineProperty(
          _defineProperty(
            _defineProperty(
              _defineProperty(
                _defineProperty(
                  _defineProperty(this, 'name', void 0),
                  'description',
                  void 0,
                ),
                'astNode',
                void 0,
              ),
              '_typeConfig',
              void 0,
            ),
            '_fields',
            void 0,
          ),
          'toJSON',
          void 0,
        ),
        'inspect',
        void 0,
      );

      this.name = config.name;
      this.description = config.description;
      this.astNode = config.astNode;
      this._typeConfig = config;

but I believe this is just two different ways of defining properties on GraphQLInputObjectType.

[IvanGoncharov]

but I believe this is just two different ways of defining properties on GraphQLInputObjectType.

In babel we trust 😄
I run out of ideas on what else you can test so I think everything is ready 👍

[IvanGoncharov]

Hi @AaronCCWong

We started to work on 14.0.0 and merged few babel related commits.
Can you please rebase this PR?
I think merging it should be the next step in 14.0.0 preparation.

[IvanGoncharov]

Also you can remove .babelrc since v7 should support .baberc.js natively.

[AaronCCWong]

@IvanGoncharov Done! Looks like Travis and coveralls are all passing too.

[IvanGoncharov]

@AaronCCWong Thanks 👍
@mjmahone Can you please review this PR?

[mjmahone]

Excellent! Thanks for updating this @AaronCCWong !