Add `react-error-overlay` #484

Timer · 2017-10-30T04:39:39Z

I'm going to bed, but this should get you started.

If you'd like, you can tell me what adjustments to make and I'll see what I can do. 😄

I'm not familiar with react-cosmos, so I added build and runtime error reporting to where I felt it made most sense.

http://g.recordit.co/LGb1qX9yJw.gif

Enables all react-error-overlay features: runtime errors, click-to-open source, and build time errors.

ovidiuch · 2017-10-30T17:03:38Z

@Timer what a wonderful surprise! 😍

I have a few questions:

The runtime error overlay doesn't auto close after a HMR update that fixes the problem. Is this the expected behavior? It probably is because of the X close button and it makes sense for runtime errors to stick because they might have corrupted something global, but I wanted to make sure this is how it works in CRA as well. Y/n?
CRA's runtime handler seems to render (pun intended) the root componentDidCatch handler I just merged useless. I don't mind of course because this is next level stuff, but I'm curious what's React's vision in general for this. In this case it seems componentDidCatch overlaps 100% with CRA's runtime handler. Is componentDidCatch meant more for graceful error handling in production and CRA will always overlap custom error states in development? cc @gaearon
It seems like react-error-overlay publishes ES modules, which is at the moment incompatible with Cosmos' packaging (react-dev-utils seems all CJS which is great). loader-entry is included in the user bundle and because Cosmos is meant to work with a wide range of setups we always publish CJS modules and make no assumptions about the user's loaders. Not sure how to handle this... 🤔 It's the only real obstacle in merging this. Ideas welcome!

Timer · 2017-10-30T17:22:47Z

The runtime error overlay doesn't auto close after a HMR update that fixes the problem. Is this the expected behavior? It probably is because of the X close button and it makes sense for runtime errors to stick because they might have corrupted something global, but I wanted to make sure this is how it works in CRA as well.

It is the expected but not the best behavior.
In CRA we force a page reload on next save after a runtime error because of global corrupted state, so we should make that the default behavior here (doesn't do that currently).

... componentDidCatch ...

I'm not on the React team so take what I say with a grain of salt.

IMO, componentDidCatch is meant for a production use so that your users aren't left with a broken application.
In development, using react-error-overlay is going to give a much better development experience -- you could always close the overlay and look at your componentDidCatch page for testing purposes too.

It seems like react-error-overlay publishes ES modules, which is at the moment ... It's the only real obstacle in merging this. Ideas welcome!

We can dual publish a CJS and ESM version (based on package.json toggle). Can you open an issue on CRA for this please?

gaearon · 2017-10-30T17:30:54Z

We can dual publish a CJS and ESM version (based on package.json toggle). Can you open an issue on CRA for this please?

We should.

ovidiuch · 2017-10-30T19:36:23Z

It is the expected but not the best behavior.
In CRA we force a page reload on next save after a runtime error because of global corrupted state, so we should make that the default behavior here (doesn't do that currently).

Actually I think it's best not to hard reload in this case. The nice thing about Cosmos is that it keeps state outside of the user's render tree, so regardless of what crashes in the user code we can re-render from scratch after a HMR change and still preserve previous state.

Even if it's Redux state, we create the store from scratch and flush all component instances on every render. It's a different take on hot reloading...

We can dual publish a CJS and ESM version (based on package.json toggle). Can you open an issue on CRA for this please?

Done: facebook/create-react-app#3354

Timer · 2017-10-30T20:09:11Z

Actually I think it's best not to hard reload in this case.

Totally your call. 😄

If you'd like, you can open another issue for support for something like dismiss-runtime-error that way it can be automatically closed.

ovidiuch · 2017-10-30T22:56:29Z

If you'd like, you can open another issue for support for something like dismiss-runtime-error that way it can be automatically closed.

Sounds good, but we can definitely merge without that.

If you're interested in finishing the remaining bits, there are four outstanding ESLint errors and 3-4 falling Jest tests—partly due to react-error-overlay ES syntax that Jest doesn't transform and thus understand, so best to tackle tests after facebook/create-react-app#3354 is resolved.

Thanks again for your interest in this!

Timer · 2017-10-31T02:51:27Z

I've rebased this so we don't fall behind -- did you want to tackle facebook/create-react-app#3354 or do you want me to take care of it? 😄 No rush.

ovidiuch · 2017-10-31T09:43:41Z

I've rebased this so we don't fall behind -- did you want to tackle facebook/create-react-app#3354 or do you want me to take care of it? 😄 No rush.

I'll take a stab at it 🔪

ovidiuch · 2017-10-31T17:02:15Z

@Timer is there a reason for using react-error-overlay@2.x (eg. webpack version compatibility)? Because react-error-overlay@3 is already published as a umd module and seems to work just fine out of the box.

Still it seems like the API changed a bit because I get this warning after upgrading:

index.js:1783 Warning: startReportingRuntimeErrors doesn’t accept launchEditorEndpoint argument anymore. Use listenToOpenInEditor instead with your own implementation to open errors in editor

Timer · 2017-10-31T17:28:48Z

Whoops, sorry! We just released react-error-overlay@3 yesterday which changed all of this. 😄

I'll update this PR tonight and see where it puts us. Sorry for the run-around.

ovidiuch · 2017-10-31T17:43:44Z

I was eager to make the change but this is even better!

I'll update this PR tonight and see where it puts us. Sorry for the run-around.

Excellent. Let me know if you run into any trouble. Besides the 3 falling tests I have one minor ask: Can you put the CRO logic inside a separate file to keep loader-entry clean? Like ./react-devtools-hook

Timer · 2017-11-01T01:37:39Z

ovidiuch · 2017-11-01T13:01:44Z

Thank you @Timer, this is a great improvement in Cosmos' DX!

Quick question: Is there any difference/advantage in using import * as ErrorOverlay from instead of just import ErrorOverlay from?

Timer · 2017-11-01T21:13:38Z

@skidding in react-error-overlay@2 doing import ErrorOverlay from was actually different than import * as ErrorOverlay from because we were working with real ES modules and not compiled ones.

I'm not sure what happens with webpack's UMD build, but it's more future proof this way (we may ship two versions instead of UMD).

So doing import ErrorOverlay resulted in strictly the default, when we really wanted the named exports (which requires import * as ErrorOverlay).

Timer · 2017-11-01T21:14:12Z

packages/react-cosmos-webpack/src/server/server.js

 import getCosmosConfig from 'react-cosmos-config';
 import extendWebpackConfig from './extend-webpack-config';
 import getUserWebpackConfig from './user-webpack-config';
+


Sorry about this new line, I didn't notice it.

ovidiuch · 2017-11-01T21:44:44Z

@Timer Got it. Thanks again!

Timer · 2017-11-02T01:08:38Z

My pleasure! Let me know if you ever need help upgrading in the future.

ovidiuch · 2017-11-02T23:37:56Z

@Timer thinking of programmatically closing runtime error overlay in some cases. I noticed there's a dismissRuntimeErrors function but it isn't exported. Do you see any reasons why it shouldn't be called? Otherwise I'd like to submit a PR for exporting it.

Timer · 2017-11-03T03:20:43Z

Mainly because in our use case it doesn't make sense to dismiss. Happy to accept a pull request exporting it.

Timer mentioned this pull request Oct 30, 2017

Integrate react-error-overlay from Create React App #451

Closed

KyleAMathews mentioned this pull request Oct 30, 2017

Show javascript errors in overlay gatsbyjs/gatsby#582

Closed

ovidiuch mentioned this pull request Oct 30, 2017

Publish dual CJS and ESM version of react-error-overlay facebook/create-react-app#3354

Closed

Timer force-pushed the feature/reo branch from 4298fbe to 0f648fc Compare October 31, 2017 02:50

ovidiuch mentioned this pull request Oct 31, 2017

Cosmos v3 BETA TESTING thread #475

Closed

Timer added 6 commits October 31, 2017 21:12

Add react-error-overlay

de36e53

Add launch editor support

847feb9

Show build errors

32e3b4d

Fix errors -> warnings

15a1ca5

Fix lint

f49b1c1

Update code to use react-error-overlay@3

1c46e76

Timer force-pushed the feature/reo branch from 0f648fc to 1c46e76 Compare November 1, 2017 01:23

Timer added 2 commits October 31, 2017 21:27

Make lint happy

18c7879

Make tests pass again

03fa1fc

ovidiuch merged commit c14a726 into react-cosmos:master Nov 1, 2017

Timer deleted the feature/reo branch November 1, 2017 21:13

Timer commented Nov 1, 2017

View reviewed changes

oyeanuj mentioned this pull request Nov 3, 2017

Add a smart error handling component. ctrlplusb/react-universally#392

Closed

ovidiuch mentioned this pull request Nov 5, 2017

Export dismissRuntimeErrors function facebook/create-react-app#3414

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `react-error-overlay` #484

Add `react-error-overlay` #484

Timer commented Oct 30, 2017 •

edited

Loading

ovidiuch commented Oct 30, 2017 •

edited

Loading

Timer commented Oct 30, 2017

gaearon commented Oct 30, 2017

ovidiuch commented Oct 30, 2017

Timer commented Oct 30, 2017

ovidiuch commented Oct 30, 2017

Timer commented Oct 31, 2017

ovidiuch commented Oct 31, 2017

ovidiuch commented Oct 31, 2017

Timer commented Oct 31, 2017

ovidiuch commented Oct 31, 2017

Timer commented Nov 1, 2017

ovidiuch commented Nov 1, 2017

Timer commented Nov 1, 2017

Timer Nov 1, 2017

ovidiuch commented Nov 1, 2017

Timer commented Nov 2, 2017

ovidiuch commented Nov 2, 2017

Timer commented Nov 3, 2017

Add react-error-overlay #484

Add react-error-overlay #484

Conversation

Timer commented Oct 30, 2017 • edited Loading

ovidiuch commented Oct 30, 2017 • edited Loading

Timer commented Oct 30, 2017

gaearon commented Oct 30, 2017

ovidiuch commented Oct 30, 2017

Timer commented Oct 30, 2017

ovidiuch commented Oct 30, 2017

Timer commented Oct 31, 2017

ovidiuch commented Oct 31, 2017

ovidiuch commented Oct 31, 2017

Timer commented Oct 31, 2017

ovidiuch commented Oct 31, 2017

Timer commented Nov 1, 2017

ovidiuch commented Nov 1, 2017

Timer commented Nov 1, 2017

Timer Nov 1, 2017

Choose a reason for hiding this comment

ovidiuch commented Nov 1, 2017

Timer commented Nov 2, 2017

ovidiuch commented Nov 2, 2017

Timer commented Nov 3, 2017

Add `react-error-overlay` #484

Add `react-error-overlay` #484

Timer commented Oct 30, 2017 •

edited

Loading

ovidiuch commented Oct 30, 2017 •

edited

Loading