RFC: Error code system #6874

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants
@keyanzhang
Member

keyanzhang commented May 25, 2016

This is a work in progress on implementing React's Error Code System. Comments and feedback are more than welcome!

Background

We currently have fbjs-scripts/babel-6/dev-expression as a babel pass. It rewrites all invariant calls from

invariant(condition, argument, argument);

to

if (!condition) {
  if ("production" !== process.env.NODE_ENV) {
    invariant(false, argument, argument);
  } else {
    invariant(false);
  }
}

If you take a look at invariant's implementation, this is why we have the "Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings." error message in production.

What's new

This PR implements an error code system and changes the rewriting to

if (!condition) {
  if ("production" !== process.env.NODE_ENV) {
    invariant(false, argument, 'foo', 'bar');
  } else {
    PROD_INVARIANT('XYZ', 'foo', 'bar');
  }
}

where

How it works

  1. We have a script that extracts error codes to a JSON file. Implemented in #6882
  2. Add a page to display the error codes. Implemented in #6946
  3. As we walk through files, the second argument (the error message template) of invariant is taken. Then it rewrites the invariant in production to reactProdInvariant using error IDs. #6948

Size comparison

   raw     gz Compared to master
     =      = build/react-dom-server.js
     =      = build/react-dom-server.min.js
     =      = build/react-dom.js
     =      = build/react-dom.min.js
 +2191   -276 build/react-with-addons.js
 +2363  +1045 build/react-with-addons.min.js
 +1605   -382 build/react.js
 +2261  +1007 build/react.min.js
@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao May 25, 2016

Member

Ironically I had been working on #6876 this morning to make sure that works for us (as well as the other projects using the preset). I think we'd still be able to do what you did and fork that dev transform, would just need to opt-out of the one already packaged (which is the opposite of how it exists today where you opt-in).

Member

zpao commented May 25, 2016

Ironically I had been working on #6876 this morning to make sure that works for us (as well as the other projects using the preset). I think we'd still be able to do what you did and fork that dev transform, would just need to opt-out of the one already packaged (which is the opposite of how it exists today where you opt-in).

@keyanzhang

This comment has been minimized.

Show comment
Hide comment
@keyanzhang

keyanzhang May 25, 2016

Member

@zpao Haha I moved it from fbjs to here to make my development easier and to gather more feedback. Do you think this pass should eventually stay here or move to fbjs? It's not really coupled with React at the moment.

Member

keyanzhang commented May 25, 2016

@zpao Haha I moved it from fbjs to here to make my development easier and to gather more feedback. Do you think this pass should eventually stay here or move to fbjs? It's not really coupled with React at the moment.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao May 25, 2016

Member

Stay here until (if) we decide it's a good thing to do in other projects and then we can "promote" it to fbjs (that's the path we've taken for all the transforms). It might not end up being a good idea for every project and we can move faster and experiment more with it here.

Member

zpao commented May 25, 2016

Stay here until (if) we decide it's a good thing to do in other projects and then we can "promote" it to fbjs (that's the path we've taken for all the transforms). It might not end up being a good idea for every project and we can move faster and experiment more with it here.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 25, 2016

@keyanzhang updated the pull request.

ghost commented May 25, 2016

@keyanzhang updated the pull request.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 25, 2016

@keyanzhang updated the pull request.

ghost commented May 25, 2016

@keyanzhang updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot May 25, 2016

@keyanzhang updated the pull request.

@keyanzhang updated the pull request.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits May 25, 2016

Member

Discussed in person, but let's split this out into three parts:

  1. Script to extract error codes to a JSON file. I don't think we need to store the version at all – instead, the script will just add any new error messages to the end of the file without changing existing codes.
  2. Changes to the website to display the error codes. Right now I'm thinking we could have one page called error-decoder.html or similar so you can visit error-decoder.html?code=123&args=%5B%22ComponentName%22%5D and it will show you the full error message with the args substituted in.
  3. Changes to the build pipeline to actually transform the invariant calls. I was thinking we would output something like reactInvariant(false, 123, componentName) which would then output a link like my example above.
Member

sophiebits commented May 25, 2016

Discussed in person, but let's split this out into three parts:

  1. Script to extract error codes to a JSON file. I don't think we need to store the version at all – instead, the script will just add any new error messages to the end of the file without changing existing codes.
  2. Changes to the website to display the error codes. Right now I'm thinking we could have one page called error-decoder.html or similar so you can visit error-decoder.html?code=123&args=%5B%22ComponentName%22%5D and it will show you the full error message with the args substituted in.
  3. Changes to the build pipeline to actually transform the invariant calls. I was thinking we would output something like reactInvariant(false, 123, componentName) which would then output a link like my example above.
@keyanzhang

This comment has been minimized.

Show comment
Hide comment
@keyanzhang

keyanzhang Jun 8, 2016

Member

Implemented in #6882, #6946, and #6948.

Member

keyanzhang commented Jun 8, 2016

Implemented in #6882, #6946, and #6948.

@keyanzhang keyanzhang closed this Jun 8, 2016

@keyanzhang keyanzhang deleted the keyanzhang:yo-error-map branch Jun 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment