-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Webpack: Hide runtime errors #23175
Webpack: Hide runtime errors #23175
Conversation
@donaldpipowitch how do I test this? Can you supply links to documentation & explaining the intend of the change? Is this PR still a "Work-in-progress"? I'll make it a draft PR, so it won't come up in our triage until it's ready, I hope you don't mind. I tested this in a codespace, and it looked fine, but honestly couldn't really determine if any really changed; hence my question above. |
Thanks for asking @ndelangen. |
f1a1904
to
599e7f5
Compare
@yannbf I'm wondering what you think of this change; considering you've worked on webpack error handling (with me) before. |
FYI: Since creating this MR we switched to vite. I'd also be fine with closing this MR. But maybe it would in general be nice to add more error boundary examples to Storybook to cover some edge cases better (the test runner also had "problems" with error boundaries that is tracked in an issue). |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 4981436. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Work-in-progress
Closes #
What I did
Storybook shows runtime errors by default in an error overlay. As React in dev-mode is re-throwing actually handled errors, this can get very confusing. If I understood correctly Storybook is not using webpack-dev-server, so we can't specify this config in user land in
main.js
. If I'm not mistaken this has to be passed as a query param to the dev middleware instead.How to test
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]