-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Simplify top-level await deadlock prevention #6121
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
Conversation
There are different scenarios where this can lead to cycles. This pessimistic approach should solve all of them.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#improve-tla-deadlock-detectionNotice: Ensure you have installed the latest nightly Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust. or load it into the REPL: |
Performance report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6121 +/- ##
==========================================
- Coverage 98.74% 98.73% -0.01%
==========================================
Files 271 271
Lines 10644 10630 -14
Branches 2850 2846 -4
==========================================
- Hits 10510 10496 -14
Misses 89 89
Partials 45 45 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Looks good to me! That’s a really nice and simple approach. Although other non-TLA dynamic imports will also be considered as TLA, it completely eliminates this kind of issue. |
|
I mean the alternative would be to either force comments upon people who want stuff to "just work", or to lift analysis on a completely different level. My hope is that top-level await is not that common anyway. And in the worst case, it will just be like just turning off the dynamic import optimization, i.e. it will be like most other bundlers. |
|
This PR has been released as part of [email protected]. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
This very much simplifies the logic to prevent merges with top-level-await modules that create deadlocks. The proposed logic is to just assume that a file that contains a top-level await will await all dynamic imports within the module. This will prevent that any dependencies of dynamically imported modules are merged into the same chunk as the current module.
cc @TrickyPi, what do you think?