-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
Prevent circular deps from causing a stack overflow in HMR runtime #2660
Conversation
@DeMoorJasper @devongovett This is a pretty small fix. What do you guys think? |
Some kind of test would be good although at first sight the code looks good. |
Confirmed this fixes an issue my team is having with maximum call stack errors on hmr reload. Tested on multiple browsers. Any word on how soon this might merge? |
A test would def help in getting this merged, merging and updates are slow atm due to our attention being divided between updating Parcel 1 and building parcel 2 Sent with GitHawk |
I have added a test and pushed to your branch, @rhurstdialpad I hope you don't mind. |
@mischnic No problem at all. Thanks for adding the test. Stoked that has landed. 🍻 |
Any timeframes on next release? |
About to go back to webpack but I found this commit and now parcel w/ typescript works like a charm. Thanks @rhurstdialpad !!! |
@aMarCruz Not a problem, happy to help. I love Parcel. |
@aMarCruz How did you go about applying this patch? There doesn't seem to be a new version released. Thanks @RobertWHurst for the fix!!! |
@frother , git clone https://github.com/parcel-bundler/parcel.git
cd parcel/packages/core/parcel-bundler
yarn && yarn build
npm pack Move the generated "parcel-bundler-1.11.0.tgz" file to some folder in your app root and install it from there (for example, in a folder // point the package.json dependency to the local package: devDependencies: {
"parcel-bundler": "./.local-pkgs/parcel-bundler-1.11.0.tgz",
} then... yarn Done. I hope I have not forgotten something. BTW: "merging and updates are slow atm due to our attention being divided between updating Parcel 1 and building parcel 2" ...this words remind me of Windows, when the urgency of launching new features was more important than solving the bugs. |
@aMarCruz I actually build it for myself, but custom fonts were not working. You getting same kinna bug? |
@Jas99 , I don't use custom fonts. |
@aMarCruz Many thanks. |
↪️ Pull Request
This PR prevents a stack overflow when updating files that have circular dependencies. This issue occurs when HMR tries to reload dependencies and gets stuck following the loop. The fix is achieved by adding a set that contains each of the asset ID updated in the same HMR update event. This set is used to keep track of what has been updated and skips loading the same asset more than once.
Fixes #1192