-
Notifications
You must be signed in to change notification settings - Fork 194
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
feat(overlay): allow full customization of the error overlay integration #44
Conversation
@pmmmwh I think this proposal looks pretty solid. When I started using this project, I was surprised to see a custom built error screen and not reuse Can you maybe explain a bit why we need to have our own custom overlay within this project? |
@blainekasten Some of those includes: the |
d76dda3
to
c42cd5b
Compare
It would be good to get feedback from @Timer, as i know that he had some thoughts on what a next gen of |
Would be good to just be able to turn it off. I personally prefer just seeing the errors in the console. |
With this API you can just pass |
I like it! @pmmmwh As I said in facebook/create-react-app#8582 (comment) once this is wrapped it will probably make for a better CRA integration (even though I like the overlay in this plugin more than the CRA one). |
Any news on this? |
I am trying to get a bit more feedback, so no concrete eta yet, sorry. Hopefully this can be merged next week. |
@@ -50,6 +36,7 @@ class ReactRefreshPlugin { | |||
|
|||
// Inject refresh utilities to Webpack's global scope | |||
const providePlugin = new webpack.ProvidePlugin({ | |||
[errorOverlay]: this.options.overlay && require.resolve(this.options.overlay.module), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I noticed this. A fix is on its way.
Background
It has been a while since the last release, and a lot of opinion have been brought up related to the error overlay integration. This PR aims to address most of them, whether it is to not use it, or use some tool-dependent ones.
Proposed API
Read README.md for a better explanation since there some caveats related to this approach.
Fixes #28
Partially addresses #7
Supersedes #33 #41
Related facebook/create-react-app#8582
CC @gaearon @blainekasten @ianschmitz @Timer
Would love your thoughts as framework authors since you are the main target for the verbose API. I can create a PR in
create-react-app
after this lands so thatreact-error-overlay
can be made compatible.