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

Incorrect chunking when code has top-level await #4708

Open
bluwy opened this issue Nov 8, 2022 · 8 comments
Open

Incorrect chunking when code has top-level await #4708

bluwy opened this issue Nov 8, 2022 · 8 comments

Comments

@bluwy
Copy link
Contributor

bluwy commented Nov 8, 2022

Rollup Version

3.2.5 (also happens in v2)

Operating System (or Browser)

macos

Node Version (if applicable)

No response

Link To Reproduction

https://rollupjs.org/repl/?version=3.2.5&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMCU3QiUyMHJlbmRlciUyMCU3RCUyMGZyb20lMjAnLiUyRmZyYW1ld29yay5qcyclNUNuJTVDbmNvbnN0JTIwbW9kJTIwJTNEJTIwYXdhaXQlMjBpbXBvcnQoJy4lMkZtb2R1bGVfMS5qcycpJTVDbmNvbnN0JTIwc29tZXRoaW5nJTIwJTNEJTIwYXdhaXQlMjByZW5kZXIoJ3RoaW5nJyklNUNuJTVDbmV4cG9ydCUyMCU3Qm1vZCUyQyUyMHNvbWV0aGluZyU3RCUyMiUyQyUyMmlzRW50cnklMjIlM0F0cnVlJTdEJTJDJTdCJTIybmFtZSUyMiUzQSUyMm1vZHVsZV8xLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMCU3QiUyMHJlbmRlciUyMCU3RCUyMGZyb20lMjAnLiUyRmZyYW1ld29yay5qcyclNUNuJTVDbmV4cG9ydCUyMGNvbnN0JTIwaHRtbCUyMCUzRCUyMHJlbmRlcignJTNDaDElM0V0ZXN0JTNDJTJGaDElM0UnKSUyMiUyQyUyMmlzRW50cnklMjIlM0FmYWxzZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJmcmFtZXdvcmsuanMlMjIlMkMlMjJjb2RlJTIyJTNBJTIyZXhwb3J0JTIwZnVuY3Rpb24lMjByZW5kZXIoc3RyKSUyMCU3QiUyMHJldHVybiUyMHN0ciUyMCU3RCUyMiU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

Expected Behaviour

The framework.js file should have it's own build chunk in the output, and module_1-ae086170.js imports from that chunk instead, because of the top-level await in main-3d8a8f11.js

Actual Behaviour

module_1-ae086170.js imports a function from framework.js through main-3d8a8f11.js (inlined). This has problems because main-3d8a8f11.js contains top-level awaits that in turn waits for module_1-ae086170.js (loop).

Running the output in nodejs silently fails (node 22 starts logging a warning). Here's a stackblitz of the REPL's output copied. Run node test.js in the terminal to see nothing printed in the console.

@lukastaegert
Copy link
Member

Thanks for spotting. I knew that we were not handling TLA execution order correctly, but I did not consider you could create deadlocks this way. The best we can do is to create more chunks, I will need to think about this a little.

@productdevbook
Copy link

I'm having the same problem and it's making us tired :/

@simplecommerce
Copy link

@bluwy I know its been a while but did you figure out a way to bypass this issue?

@bluwy
Copy link
Contributor Author

bluwy commented Sep 21, 2023

I haven't tested, but following the repro, setting manualChunks to create a specific chunk for framework.js should fix it. However, it's not always easy to spot which files to chunk in your app, so you might have to analyze and try to see which file is causing the lock.

I also started to attempt a fix recently though, so maybe this will be fixed by itself (if I can figure out the algorithm).

@simplecommerce
Copy link

I haven't tested, but following the repro, setting manualChunks to create a specific chunk for framework.js should fix it. However, it's not always easy to spot which files to chunk in your app, so you might have to analyze and try to see which file is causing the lock.

I also started to attempt a fix recently though, so maybe this will be fixed by itself (if I can figure out the algorithm).

Yes, thanks for that.
I was able to fix it on my end, its too bad there isn't a simple way to just crash an error indicating what is causing the lock/loop.

@bluwy
Copy link
Contributor Author

bluwy commented Sep 21, 2023

It seems like it's part of the ESM spec that it fails silently. Here's a stackblitz example of the deadlock happening in browsers too. If you run it through something like Bun it seems to work, maybe because it's not following the spec exactly.

@bluwy
Copy link
Contributor Author

bluwy commented Sep 22, 2023

I tried looking into it but it's a little complicated than expected. I can fix the repro's specific case by skipping deleting the shared chunks' dependent entries (used for optimization) at here. But it's not the true fix with a more complex TLA loop like in withastro/astro#8598, where shared code are spreaded more.

The true fix likely lies in the very first step at

const initialChunks = getChunksFromDependentEntries(
getModulesWithDependentEntries(dependentEntriesByModule, modulesInManualChunks)
);
where the initialChunks should have the TLA module in its own chunk, plus further optimizations need to keep in mind to not merge their chunk to this TLA chunk.

@bluwy
Copy link
Contributor Author

bluwy commented Oct 21, 2024

It looks like in Node 22, there's now a warning logged instead of silently working:

Warning: Detected unsettled top-level await at file:///Users/bjorn/Work/test/main-3d8a8f11.js:3
const mod = await import('./module_1-ae086170.js');

I found this from nodejs/node#55468

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

No branches or pull requests

4 participants