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

Fix HMR bugs #7514

Merged
merged 5 commits into from
Jan 2, 2022
Merged

Fix HMR bugs #7514

merged 5 commits into from
Jan 2, 2022

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jan 1, 2022

Possibly also fixes #6685, but I couldn't find a reproduction in that issue that didn't already work...

@height
Copy link

height bot commented Jan 1, 2022

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@parcel-benchmark
Copy link

parcel-benchmark commented Jan 1, 2022

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 2.56s +146.00ms ⚠️
Cached 414.00ms +35.00ms ⚠️

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 738.00ms +41.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 746.00ms +48.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 746.00ms +48.00ms ⚠️
dist/legacy/index.0a5c751e.js 1.59kb +0.00b 1.08s +75.00ms ⚠️
dist/legacy/index.8aaa89c9.js 1.20kb +0.00b 1.08s +75.00ms ⚠️
dist/modern/index.6be20f01.js 1.13kb +0.00b 1.08s +75.00ms ⚠️
dist/legacy/index.html 826.00b +0.00b 1.25s +70.00ms ⚠️
dist/modern/index.html 749.00b +0.00b 1.25s +70.00ms ⚠️
dist/legacy/index.c1bc86aa.css 94.00b +0.00b 1.42s +86.00ms ⚠️
dist/modern/index.57a95cbe.css 94.00b +0.00b 1.41s +83.00ms ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 767.00ms +84.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 769.00ms +83.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 768.00ms +81.00ms ⚠️
dist/legacy/index.0a5c751e.js 1.59kb +0.00b 1.12s +95.00ms ⚠️
dist/legacy/index.8aaa89c9.js 1.20kb +0.00b 1.12s +96.00ms ⚠️
dist/modern/index.6be20f01.js 1.13kb +0.00b 1.12s +97.00ms ⚠️
dist/legacy/index.html 826.00b +0.00b 1.31s +115.00ms ⚠️
dist/modern/index.html 749.00b +0.00b 1.31s +115.00ms ⚠️
dist/legacy/index.c1bc86aa.css 94.00b +0.00b 1.50s +156.00ms ⚠️
dist/modern/index.57a95cbe.css 94.00b +0.00b 1.50s +157.00ms ⚠️

React HackerNews ✅

Timings

Description Time Difference
Cold 12.02s -170.00ms
Cached 507.00ms -37.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 1.29m -595.00ms
Cached 1.73s +211.00ms ⚠️

Cold Bundles

Bundle Size Difference Time Difference
dist/editorView.124f5fc0.js 594.92kb +0.00b 57.96s +13.71s ⚠️
dist/popup.64bc9a82.js 209.67kb +0.00b 57.97s +13.73s ⚠️
dist/Toolbar.1af0e801.js 107.23kb +0.00b 57.96s +13.71s ⚠️
dist/Modal.cd71eaf3.js 45.33kb +0.00b 57.97s +13.73s ⚠️
dist/ui.5d3f7adc.js 14.94kb +0.00b 57.97s +13.72s ⚠️
dist/smartMediaEditor.48c8cf63.js 13.25kb +0.00b 57.96s +13.71s ⚠️
dist/dropzone.39132d0c.js 12.16kb +0.00b 57.95s +13.71s ⚠️
dist/EmojiPickerComponent.0482d6c0.js 3.73kb +0.00b 57.97s +13.72s ⚠️
dist/dropzone.55bef257.js 3.29kb +0.00b 57.97s +13.73s ⚠️
dist/clipboard.df70240c.js 2.93kb +0.00b 57.95s +13.71s ⚠️
dist/ResourcedEmojiComponent.667554b4.js 2.12kb +0.00b 57.97s +13.72s ⚠️
dist/browser.4e039ed7.js 1.69kb +0.00b 57.96s +13.71s ⚠️
dist/media-card-analytics-error-boundary.c718a9a7.js 1.12kb +0.00b 57.95s +13.71s ⚠️
dist/media-picker-analytics-error-boundary.8b2547e5.js 966.00b +0.00b 57.95s +13.71s ⚠️
dist/simpleHasher.fc0d6100.js 643.00b +0.00b 57.97s +13.72s ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/editorView.af442747.js 594.92kb +0.00b 43.49s -16.10s 🚀
dist/popup.64bc9a82.js 209.67kb +0.00b 43.49s -16.10s 🚀
dist/Toolbar.1af0e801.js 107.23kb +0.00b 43.49s -16.10s 🚀
dist/Modal.cd71eaf3.js 45.33kb +0.00b 43.49s -16.10s 🚀
dist/ui.5d3f7adc.js 14.94kb +0.00b 43.50s -16.08s 🚀
dist/smartMediaEditor.48c8cf63.js 13.25kb +0.00b 43.49s -16.10s 🚀
dist/dropzone.39132d0c.js 12.16kb +0.00b 43.49s -16.10s 🚀
dist/EmojiPickerComponent.0482d6c0.js 3.73kb +0.00b 43.50s -16.08s 🚀
dist/dropzone.55bef257.js 3.29kb +0.00b 43.49s -16.10s 🚀
dist/clipboard.df70240c.js 2.93kb +0.00b 43.49s -16.10s 🚀
dist/ResourcedEmojiComponent.667554b4.js 2.12kb +0.00b 43.50s -16.08s 🚀
dist/browser.4e039ed7.js 1.69kb +0.00b 43.49s -16.10s 🚀
dist/media-card-analytics-error-boundary.c718a9a7.js 1.12kb +0.00b 43.49s -16.10s 🚀
dist/media-picker-analytics-error-boundary.8b2547e5.js 966.00b +0.00b 43.49s -16.10s 🚀
dist/simpleHasher.fc0d6100.js 643.00b +0.00b 43.50s -16.08s 🚀

Three.js ✅

Timings

Description Time Difference
Cold 8.56s +201.00ms
Cached 451.00ms +10.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/Three.js 579.68kb +0.00b 5.88s -399.00ms 🚀

Click here to view a detailed benchmark overview.

return accepted;
}

function hmrAcceptCheckOne(
Copy link
Member

Choose a reason for hiding this comment

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

So I guess the idea was that hmrAcceptCheck does the parent traversal and hmrAcceptCheckOne just checks if the single asset accepts, without the ancestry? But hmrAcceptCheckOne still calls hmrAcceptCheck, is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it should be ok because it only happens for the initial bundle, not when finding parents (because depsByBundle is null then).

@Systemcluster
Copy link

Is this PR included in parcel 2.0.0-nightly.961 / @parcel/runtime-browser-hmr 2.0.0-nightly.963?

@mischnic
Copy link
Member

mischnic commented Jan 3, 2022

Yes

@Nikruto
Copy link

Nikruto commented Jan 3, 2022

import React from "react";
import { render } from "react-dom";
import "./index.css";

render(<div>Heyyo</div>, document.getElementById("root"));

using 2.0.0-nightly.961 and hmr still not working for css

@devongovett
Copy link
Member Author

@Nikruto your example works fine for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment