Suggestion: Don't display error iframe · Issue #28 · pmmmwh/react-refresh-webpack-plugin

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

Sometimes, the Promise reject data which is expected, but this plugin report error message by create an iframe#react-refresh-overlay element to html.body. That's boring, because I can see error message in Console .

This behavior will interrupted my mind and I have to close iframe by hand.

Sometimes, the Promise reject data which is expected, but this plugin report error message by create an iframe#react-refresh-overlay element to html.body. That's boring, because I can see error message in Console .

This behavior will interrupted my mind and I have to close iframe by hand.

I thought the overlay only shows up when there are unhandled promise rejections? That's an Error by definition, isn't it? Or are you saying that it shows up even when the rejection is handled?

I just tested Promise.reject in RN 0.61, it's reported as a warning display at the bottom of screen. You recognize it as error.

Whatever, I also want to have an option to disable showing error.

My thoughts on this:

Unhandled promise rejection should be warned as it is being warned in the console anyways. I can tune it down to be an error, but I wouldn't want to allow someone to switch of unhandled rejections independently in the error overlay

Realistically, I can allow an option to make the plugin not inject the error overlay runtime code, which will effectively disable the overlay. I can see this useful in a few ways, for example custom HMR implementations or custom error logic. (also would be useful for CRA since they have both of these)

CC @gaearon your thoughts?

For cases where you need to ignore a thrown exception, you should have something in your user code that is catching it and doing something with it (you could also use an Error Boundary ):

try {
  const res = await api.post('data');
  return res.data;
} catch (error) {
  // if the promise is rejected
  console.log(error); // log out the error, but catching here prevents this from being an uncaught exception
  // do something to display error message
Keep in mind that in React, if you hit an uncaught exception it will crash the entire app. From the React Docs:
This change has an important implication. As of React 16, errors that were not caught by any error boundary will result in unmounting of the whole React component tree.
This feature you are asking for is not just about development experience. It means that even if you ignored the error in development mode, it will still crash the React component tree.
Keep in mind that in React, if you hit an uncaught exception it will crash the entire app.
My project run well when I am using react-hot-loader, and never ever crash when I uncaught rejection. I don't think Promise.reject can crash React component tree, it's async and out of control for react
          Error overlay should display unhandled rejections. It is easy to intentionally handle it by adding catch() if you don't want to treat it as an error.
It is also true that we should offer a way to customize the error overlay to "redirect" it to another module. We'll need need this for opinionated integrations like Next. Seems like that would cover the use case for a "noop" overlay too anyway.
Using the builtin error overlay in webpack creates duplicate overlays with this plugin active.
Are you talking about the compile-time or runtime overlay?
          yes, please let us disable it. the visualization is not that great. we already use error-overlay-webpack-plugin which give better errors.
this is far better for us:
          webpack-dev-server provide builtin overlay already.

https://webpack.js.org/configuration/dev-server/#devserveroverlay

I think it is enough for most case, and this plugin should have an options to disable the layout to avoid duplicate
We just hit a situation where the overlay makes our app unusable and prevents testing and development. I think maybe it shouldn't sorry about it at all, but even if you're unsure about that please merge the pr defaulting it to off while you decide.
Hi, sorry that I have been very busy lately. I'll take a look at it tomorrow and provide my thoughts on that.
  JesseClifft, vdh, nchevsky, robyn-57blocks, flute, yuis-ice, sptessier, minpuff, globad, k4mr4n, and 40 more reacted with thumbs up emoji
  rheaditi, AnvarAka1, AkhrorLorem, robyn-57blocks, flute, vladislavbagnyuk, connect-dips, Aprillion, mnpenner, aleksfront, and 13 more reacted with heart emoji
  globad, RiaanVenterIR, borie88, stefvhuynh, protoEvangelion, rcreasi, SherCong, and martinbee reacted with rocket emoji
    All reactions