Skip to content
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

improve our overlay #3689

Open
1 of 2 tasks
alexander-akait opened this issue Aug 19, 2021 · 20 comments · Fixed by #4576
Open
1 of 2 tasks

improve our overlay #3689

alexander-akait opened this issue Aug 19, 2021 · 20 comments · Fixed by #4576

Comments

@alexander-akait
Copy link
Member

alexander-akait commented Aug 19, 2021

  • This is a bug
  • This is a modification request

Code

improve our default overlay:

  1. Allow to catch runtime errors
  2. Improve errors/warnings reporting (ideally we should have )
  3. Improve styles
  4. More?

CRA has react-error-overlay, I think we can union with them and create the one great overlay

/cc @raix

Please paste the results of webpack-cli info and webpack-cli version here, and mention other relevant information

Expected Behavior

Better DX

Actual Behavior

Our overlay is very simple and in some places have bad DX

@Eli-Black-Work
Copy link

Not sure if it's possible, but it would be awesome if compilation errors were shown after refreshing the page! 🙂 Currently if the page is refreshed when there's a compilation error, we just get a whitescreen and no overlay is shown.

@SuneelFreimuth
Copy link

I'd be happy to try implementing these improvements.

@alexander-akait
Copy link
Member Author

Not sure if it's possible, but it would be awesome if compilation errors were shown after refreshing the page! slightly_smiling_face Currently if the page is refreshed when there's a compilation error, we just get a whitescreen and no overlay is shown.

Can you provide reproducible steps, I think it is bug

@ylemkimon
Copy link
Contributor

react-error-overlay seems to be easily integrable, adding it to the client works out of the box. Two notes:

  • it doesn't support build warnings, i.e., build errors are not dismissable
  • it also displays runtime errors, which is not the existing overlay did. If the dev server decides to support this, source code fetch logic should be also updated

@alexander-akait
Copy link
Member Author

@ylemkimon yep, feel free to start it, for runtime errors we need a new options for overlay, i.e. overlay.runtimeErrors

@sibelius
Copy link

how can I customize overlay behavior?

I've tried this

devServer: {
   overlay: {
      warnings: false,
      errors: true
    }
}
```

but it did not worked

@alexander-akait
Copy link
Member Author

Should work, what is the problem?

@ylemkimon
Copy link
Contributor

@sibelius If you're using dev server v4, you should put overlay in client (https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md):

devServer: {
   client: {
      overlay: {
         warnings: false,
         errors: true
       }
   }
}

@malcolm-kee
Copy link
Contributor

I think all the 3 listed can be implemented independently.

I would like to share my current thought on number 3 (improve styles).

Based on what I've seen in react-error-overlay, it uses React to render the overlay, I think we should avoid bringing in React just for overlay.

Instead, we can stick with current approach (manually constructing the HTML using plain JS), and just port-in the style.

@alexander-akait
Copy link
Member Author

@malcolm-kee What we need to implement more, and I will update our check list or maybe we need a small dicussion about other features from CRA (or other good overlays)

@malcolm-kee
Copy link
Contributor

malcolm-kee commented May 7, 2023

CRA error stack was pretty nice as it showed code snippet, but I not sure if that's possible given that we don't have control over devtool options.

Other than that I'm thinking of making runtime overlay less intrusive, like showing at bottom right, since it usually doesn't break application code like compilation error does.

@alexander-akait
Copy link
Member Author

@malcolm-kee

Other than that I'm thinking of making runtime overlay less intrusive, like showing at bottom right, since it usually doesn't break application code like compilation error does.

Sounds good, even more, I think we can add an option for this, even for compilation errors/warnings

CRA error stack was pretty nice as it showed code snippet, but I not sure if that's possible given that we don't have control over devtool options.

Can you show me, because we can use generate source maps (if they were enabled)

@malcolm-kee
Copy link
Contributor

Can you show me, because we can use generate source maps (if they were enabled)

Found the following gif from here.

cra-runtime-error

@alexander-akait
Copy link
Member Author

hm, I see, I think we can improve it on webpack side, I will look deeply on this before preparing the new release and will say how we can achive it

@malcolm-kee
Copy link
Contributor

Another inspiration from React Native:

React Native Error

We can probably support shortcut key so it's easy to show/hide the error.

@malcolm-kee
Copy link
Contributor

Adding some design: https://www.figma.com/file/xyeFjFzImMya1MdDDn7vii/Webpack-Error-Overlay?type=design&node-id=0%3A1&t=SgABTKrN4qJU6k15-1

Feedbacks are welcomed.

Compilation error

compilation-error

Runtime error notification

This notification button will be positioned fixed at bottom right

runtime-notification

Runtime error (when clicked)

runtime-error

@levrik
Copy link

levrik commented Jan 19, 2024

@malcolm-kee These designs look awesome. Sad that it's unlikely they will ever get implemented.

@alexander-akait
Copy link
Member Author

@levrik I am fine with such improvements and the new design, if you want to send PR - welcome

@snitin315
Copy link
Member

I can try implementing this design post major version release.

@levrik
Copy link

levrik commented Jan 19, 2024

@alexander-akait I would love to but doubt I'll find the time for it.
@snitin315 This would be awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants