Compile invariant directly to throw expressions #15071
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
acdlite
changed the title
|
React: size: -2.6%, gzip: -2.7% ReactDOM: size: -0.3%, gzip: -0.6% Details of bundled changes.Comparing: df7b87d...8e7cb77 react
react-dom
react-art
react-native-renderer
react-test-renderer
react-reconciler
create-subscription
jest-react
Generated by 🚫 dangerJS |
|
AFAIK, we still need to compile to |
|
Not sure, I was hoping one of you had more context on that. |
|
Looks like Logview will still get the messages but our |
gaearon
reviewed
Mar 11, 2019
|
|
||
| function ReactError(message) { | ||
| const error = new Error(message); | ||
| error.name = 'Invariant Violation'; |
There was a problem hiding this comment.
While we're at it, can we just say "React Error"? Is that a breaking change? 😄
There was a problem hiding this comment.
I'd be comfortable changing in a minor
There was a problem hiding this comment.
I think it's a breaking change. It breaks continuity in logs.
scripts/error-codes/__tests__/__snapshots__/minify-error-messages.js.snap
Outdated
Show resolved
Hide resolved
|
I looked into it more, and in www, the only things the
I'll look into fbsource, too. |
|
AFAICT the version in fbsource does nothing special at all. |
acdlite
force-pushed
the
reacterror
branch
2 times, most recently
from
49f5282
to
96d30de
Compare
This transforms calls to the invariant module:
```js
invariant(condition, 'A %s message that contains %s', adj, noun);
```
Into throw statements:
```js
if (!condition) {
if (__DEV__) {
throw ReactError(`A ${adj} message that contains ${noun}`);
} else {
throw ReactErrorProd(ERR_CODE, [adj, noun]);
}
}
```
The only thing ReactError does is return an error whose name is set
to "Invariant Violation" to match the existing behavior.
ReactErrorProd is a special version used in production that throws
a minified error code, with a link to see to expanded form. This
replaces the reactProdInvariant module.
As a next step, I would like to replace our use of the invariant module
for user facing errors by transforming normal Error constructors to
ReactError and ReactErrorProd. (We can continue using invariant for
internal React errors that are meant to be unreachable, which was the
original purpose of invariant.)
I wasn't sure about this part so I asked Sebastian, and his rationale was that using arguments will make ReactErrorProd slightly slower, but using an array will likely make all the functions that throw slightly slower to compile, so it's hard to say which way is better. But since ReactErrorProd is in an error path, and fewer bytes is generally better, no array is good.
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first of two PRs to improve our error extraction process.
Ok I went down a bit of a rabbit hole.
It started with this todo:
react/packages/react-reconciler/src/ReactFiberUnwindWork.js
Lines 390 to 398 in d0289c7
For some context, production builds of React do not include error messages. We strip them out at compile time and replace them with error codes and a link to a URL that displays the full message. Our error messages can be as verbose as we want without increasing code size. Here's an example: https://reactjs.org/docs/error-decoder.html?invariant=109&args%5B%5D=Foo.
The error code script works by finding calls to our
invariantmodule, looking up the message in a map, and replacing it with the corresponding code. Because we useinvariantthroughout our codebase to assert expected behavior, including for user facing errors, this mostly works really well.But error in the excerpted code above doesn't use
invariant. It's part of React's error handling path; we can't useinvariantthere because invariant will immediately throw. Instead, we create an error object and pass it along to the next step of the error handling algorithm. That means this error is not being replaced with an error code.While thinking about the best way to solve this, I collected some other flaws in the way we do error extraction:
invariantmodule is a Facebook-ism that's not commonly used by other developers. It's a marginal burden for first time contributors to learn how to use it. For example,invariantthrows an error when its first argument evaluates tofalse, but most people are accustomed to checking for the positive case:if (cond) throw Error(msg)as opposed toinvariant(!cond, msg). It's also not obvious that you're supposed to useinvariantfor errors if you've never contributed to React before.invariantwas originally meant to assert some property of the program that is always supposed to be true. These types of errors should only surface to the user if there's a bug in the program itself. Over time, both Facebook and React have started using it for user facing errors, too.invariantin our codebase. We have our sizebot that reports increases in bundle size, but if a PR already increases bundle size for other reasons, a strayErrorcould increase it slightly further without being noticed by the reviewer.invariantcan only be used to immediately throw an error. It's not suited for cases where you need to create an error and pass it along.Proposed solution
Compile
invariantdirectly to throw expressions.This PR turns
invariantinto a compile-time only module. It turns code like this:into this:
The only thing
ReactErrordoes is return an error whose name is set to "Invariant Violation" to match the existing behavior.ReactProdErroris a special version used in production that throws a minified error code, with a link to see to expanded form. This replaces thereactProdInvariantmodule.The runtime behavior is the same as it is today.
Compile normal
Errorconstructors toReactErrorandReactErrorProdI'll open this part as a separate PR.
Use a lint rule to enforce that error messages are written in the correct format
This will go in the next PR, too.