feat: async modules / top level await #5450
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
7 Ignored Deployments
|
|
✅ This change can build |
4e42c42 to
3ba7bf7
Compare
|
172177c to
586e769
Compare
sokra
left a comment
There was a problem hiding this comment.
Looks good in general.
I'm not sure if this hangs when there are cycles in the module graph... need to verify, maybe with a test
It would also be great to have some tests that actually evaluate the code. Wills PR would help here
crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| #[derive(Default)] | ||
| struct TopLevelAwaitVisitor { |
There was a problem hiding this comment.
It might be possible to merge this visitor into detect_dynamic_export to avoid walking the ast multiple times
sokra
left a comment
There was a problem hiding this comment.
Ok there are a bunch of new interactions that didn't exist with webpack.
-
We expose 2 types of exports
module.exports(used for require) andmodule.namespaceObject(use for import) -
For an non-ESM (where
module.namespaceObjectdoesn't exist), we use interopEsm to create a interop namespace object. -
For ESM
module.exportsandmodule.namespaceObjectmust be equal -
We need to proxify the exports for
dynamicExport. (Bothmodule.exportsandmodule.namespaceObject) -
We need to promisify the exports for async modules.
-
and 5. are quite new.
First thing: It seems like dynamicExport is a bit wrong (even on main) as it doesn't proxify module.exports. We need to add that.
Second thing: When combining 4. and 5. we need to proxify the resolved value of the promise. But we also don't want to change the Promise instance since it could already be imported (and awaited) by some other modules.
So we want to be able to change the resolved value.
So the dynamicImport method probably wants to detect async modules and modify the module.namespaceObject[turbopackExports] value. And the async modules implementation wants to avoid using it's local scoped exports object and instead resolve to the current value of promise[turbopackExports], which might be proxified.
crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts
Outdated
Show resolved
Hide resolved
crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts
Outdated
Show resolved
Hide resolved
2099396 to
d2cfc7c
Compare
dd681f7 to
41f9e62
Compare
e49643d to
6052d5d
Compare
b3439b4 to
5556ebd
Compare
Linux Benchmark for 945ba0bClick to view benchmark
|
sokra
left a comment
There was a problem hiding this comment.
I'll merge it, but there are some outstanding follow-up tasks:
- Add documentation to all public items. Especially in the async_module.rs file. Mention what
Sccstands for or maybe even write out the full name. - Add a test case of circular dependency graphs.
* vercel/turborepo#5567 <!-- Alex Kirszenberg - Remove unnecessary ValueDebugFormat item, hide Vc field --> * vercel/turborepo#5576 <!-- Alex Kirszenberg - Extract shared HMR utils to their own modules/crates --> * vercel/turborepo#5503 <!-- OJ Kwon - feat(turbopack_core): define trait for diagnostics --> * vercel/turborepo#5487 <!-- Will Binns-Smith - turbopack-cli: modularize code to support turbopack build --> * vercel/turborepo#5488 <!-- Will Binns-Smith - turbopack-cli: implement `turbopack build` --> * vercel/turborepo#5450 <!-- Leah - feat: async modules / top level await --> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Description
For now this is just for top level await, in the future we want to be able to support external esm modules
Testing Instructions
Closes WEB-434
Closes WEB-987