-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert react-error-overlay to React #2100
Comments
So I went ahead and turned this into a separate package (and flow-type'd it) a while ago simply because it was virtually impossible to change things without breaking everything. See here. The current copy in this repo is basically the rollup'd version of that package (which isn't published). Switching it to React is viable now since we're using an iframe, but wasn't before since it wasn't in it's own frame. In hindsight, doing it in React probably would've been optimal but that was pre-iframe days. |
Yea I was being silly for not thinking of this earlier. Do you mind bringing your package into our monorepo? |
Sure, we can do that. 😄 |
I think we should do this asap because it’s going to be tough to fix bugs as soon as this is out. |
I’m cool with keeping it separate too if you would prefer that. Just need access to publishing. |
The package(s) have been brought into this repo. 👍 |
Let's do an equivalent of #2142 as part of that. I'll close that PR for now. |
Is anyone working on this? If not, I would like to write some React code 😄 |
I don't think anyone started it yet. Please do! |
The latest code lives in |
Ok. Thanks! |
Note that it has a build step (see its package.json). |
Noted! |
Fixed in 645dc42. |
It's currently pretty complicated. We should turn this into a separate package with a build step, and probably use React as a dependency instead of vanilla JS code.
We shouldn't have the "duplicate React" issue because it's technically inside an iframe and would get its own copy of React. So if I understand it right, they wouldn't share any state, and if React in the app breaks, the one in the overlay should not be affected.
This should also work if the main app stops depending on React.
The text was updated successfully, but these errors were encountered: