Rollup script can now extract error codes and build in a single pass

[bvaughn]

Relates to this comment.

Now we no longer have to run:

yarn build --extract-errors
yarn build # This second run isn't necessary anymore

This PR contains 2 changes:

scripts/rollup/build.js

The main fix was swapping the order of error-code-extraction and replacement.

I verified this by first emptying out the contents of error-codes.json ({}) and running:

yarn build dom-fiber --extract-errors

This built the bundle and extracted error codes in a single pass. I copied the resulting build artifacts to an external location and then ran yarn build dom-fiber a second time (after error codes had already been extracted). Then I used diff to compare the bundles and verify that the following files were identical:

packages/react-dom/cjs/react-dom.development.j
packages/react-dom/cjs/react-dom.production.min.js
packages/react-dom/umd/react-dom.development.j
packages/react-dom/umd/react-dom.production.min.js
facebook-www/ReactDOMFiber-dev.js
facebook-www/ReactDOMFiber-prod.js

scripts/error-codes/replace-invariant-error-codes.js

I also changed this file to always convert invariants to ternaries:

// Before
invariant(condition, 'error message');

// After
!condition ? invariant(false, 'error message') : void 0;

Regardless of whether the error message is in our error codes map. This shouldn't actually matter but...the 2nd format can be faster and so I thought I might as well do it while I was in there. (I'll revert this if anyone thinks it's overkill.)

I tested this change with AST explorer by transforming the following JavaScript:

function testErrorInMap(thing: string) {
  invariant(
    thing,
    'Error in map',
  );
}

function testErrorNotInMap(thing: string) {
  invariant(
    thing,
    'Error not in map',
  );
}

With an error map that only contained 1 of the strings:

var errorMap = {};
errorMap['Error in map'] = '1';

And the resulting transform looked good:

var _prodInvariant = require("reactProdInvariant");

function testErrorInMap(thing: string) {
  (!string ? (__DEV__ ? invariant(false, "Error in map") : _prodInvariant("1")) : void 0);
}

function testErrorNotInMap(thing: string) {
  (!string ? (__DEV__ ? invariant(false, "Error not in map") : 'Error not in map') : void 0);
}

[gaearon]

Maybe I'm missing something, but isn't this a bug? I just noticed:

function testErrorNotInMap(thing: string) {
  (!string ? (__DEV__ ? invariant(false, "Error not in map") : 'Error not in map') : void 0);
}

which in non-DEV does not produce an invariant at all.

Instead, it should probably just be:

  (!string ? invariant(false, "Error not in map") : void 0);

since behavior differences are not expected.

[gaearon]

It's expected to occur on master builds.