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

Add React error overlay and HMR source maps #8034

Merged
merged 7 commits into from May 8, 2022
Merged

Conversation

devongovett
Copy link
Member

Closes #7604. Closes #643. Closes #5316.

image

This adds an overlay that appears when there is a runtime error in a React app, using the react-error-overlay package from Create React App. It is implemented as part of @parcel/runtime-react-refresh since it is React-specific, and that seemed like the easiest place to put it. It will appear when a runtime error occurs, and is automatically dismissed when an HMR update comes in.

This required support for source maps in HMR, which has been a longstanding issue due to our use of new Function, which has poor support for source maps. Now, we use eval instead, which has better support in Chrome and Firefox. However, Safari still doesn't support sourceMappingURL or sourceURL in eval error stack traces, so we detect this case and fall back to loading the asset from a new dev server endpoint that serves asset contents by id.

As a side benefit, HMR should now fall back to loading URLs for changed assets via HTTP if eval is not allowed, e.g. with a strict CSP. This may help web extension v3 support, though I have not tested this. (cc. @101arrowz)

The error overlay also supports clicking the code frame to open it directly in your editor, which I also added to the standard parcel build error overlay and the error 500 page. This is implemented through a second new dev server endpoint which auto-detects which editor you're using, and opens it to the line and column of the error message.

Finally, the error 500 page has been improved to automatically reload when an HMR update comes in so you don't need to manually do that.

@devongovett devongovett requested a review from mischnic May 1, 2022 04:13
codeframe: string,
/** A list of code frames with highlighted code and file locations separately. */
frames: Array<FormattedCodeFrame>,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unfortunate, but couldn't think of a better way...

Copy link
Member

@101arrowz 101arrowz May 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make codeframe a getter that processes frames in the default manner so there's less duplication. Beyond disgusting regex I don't think there's any other way to support rewriting paths than saving the frames array.

if (sourcemap) {
let sourcemapStringified = await sourcemap.stringify({
format: 'inline',
sourceRoot: '/__parcel_source_root/',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat related, but this source root seems to be displayed by react-error-overlay rather than either the original file path, or nothing. Not sure if there is a way to improve that?

@101arrowz
Copy link
Member

Yes, this should help manifest V3 hot reload. As long as the HMR server is on some kind of localhost domain, dynamic script loading should work fine via a similar CSP patching technique to MV2. I'll look into it more after this is merged.

@parcel-benchmark
Copy link

parcel-benchmark commented May 1, 2022

Benchmark Results

Kitchen Sink 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

React HackerNews ✅

Timings

Description Time Difference
Cold 8.79s +58.00ms
Cached 445.00ms +14.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/logo.c5bb83f1.png 246.00b +0.00b 3.84s -581.00ms 🚀

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 1.50m -666.00ms
Cached 2.41s -92.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.0605f25b.js 3.32mb +144.00b ⚠️ 29.70s -349.00ms
dist/16.069344b7.js 905.00b +0.00b 33.73s -3.84s 🚀
dist/simpleHasher.46d6f2e5.js 742.00b +0.00b 37.29s -31.24s 🚀
dist/index.html 240.00b +0.00b 29.71s -3.31s 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/esm.6190c9ac.js 61.54kb +0.00b 1.13m +30.73s ⚠️
dist/workerHasher.e50d242f.js 1.72kb +0.00b 1.13m +30.73s ⚠️
dist/16.1969624f.js 1.08kb +0.00b 36.72s +3.07s ⚠️
dist/16.069344b7.js 905.00b +0.00b 36.72s +3.07s ⚠️
dist/simpleHasher.46d6f2e5.js 742.00b +0.00b 1.13m +30.73s ⚠️
dist/index.html 240.00b +0.00b 1.13m +38.42s ⚠️

Three.js ✅

Timings

Description Time Difference
Cold 6.04s -26.00ms
Cached 297.00ms +25.00ms ⚠️

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

It relies on regeneratorRuntime which isn't always defined, and SWC will compile the runtime anyway
@devongovett devongovett merged commit e6a6684 into v2 May 8, 2022
@devongovett devongovett deleted the react-error-overlay branch May 8, 2022 23:01
@101arrowz
Copy link
Member

The logic here for the dev server is a bit problematic for web extensions since it depends on parcel serve rather than just parcel watch in order to serve the new chunks. To fix this we'd need to refactor a lot of serving logic. Additionally, some of the hardcoded /__parcel_source_root won't work because it will try to access the web extension context, we need to change it into absolute paths including hostname if serveOptions.host != null.

I'll try to do this at some point.

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 this pull request may close these issues.

React Error Overlay Bug: Source maps don't work with hot reload
3 participants