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

feat: async modules / top level await #5450

Merged
merged 16 commits into from
Jul 21, 2023
Merged

feat: async modules / top level await #5450

merged 16 commits into from
Jul 21, 2023

Conversation

ForsakenHarmony
Copy link
Contributor

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

@vercel
Copy link

vercel bot commented Jul 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-designsystem-docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 2:39pm
examples-kitchensink-blog 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 2:39pm
examples-native-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 2:39pm
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 2:39pm
7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm
examples-cra-web ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2023 2:39pm

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

✅ This change can build next-swc

@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-434-async-modules branch 2 times, most recently from 4e42c42 to 3ba7bf7 Compare July 3, 2023 21:24
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-434-async-modules branch 2 times, most recently from 172177c to 586e769 Compare July 4, 2023 17:31
Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

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

let mut visitor = TopLevelAwaitVisitor::default();

m.visit_with(&mut visitor);

visitor.has_top_level_await
visitor.top_level_await_span
}

#[derive(Default)]
struct TopLevelAwaitVisitor {
Copy link
Member

Choose a reason for hiding this comment

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

It might be possible to merge this visitor into detect_dynamic_export to avoid walking the ast multiple times

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

Ok there are a bunch of new interactions that didn't exist with webpack.

  1. We expose 2 types of exports module.exports (used for require) and module.namespaceObject (use for import)

  2. For an non-ESM (where module.namespaceObject doesn't exist), we use interopEsm to create a interop namespace object.

  3. For ESM module.exports and module.namespaceObject must be equal

  4. We need to proxify the exports for dynamicExport. (Both module.exports and module.namespaceObject)

  5. We need to promisify the exports for async modules.

  6. 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.

@ForsakenHarmony ForsakenHarmony force-pushed the hrmny/web-434-async-modules branch 5 times, most recently from dd681f7 to 41f9e62 Compare July 10, 2023 15:53
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 10, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@vercel vercel deleted a comment from github-actions bot Jul 20, 2023
@github-actions
Copy link
Contributor

Linux Benchmark for 945ba0b

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 7588.84µs ± 24.87µs 7488.74µs ± 42.07µs -1.32%
bench_hmr_to_eval/Turbopack CSR/1000 modules 6842.72µs ± 15.45µs 6927.77µs ± 75.00µs +1.24%
bench_startup/Turbopack CSR/1000 modules 871.09ms ± 5.06ms 863.18ms ± 4.54ms -0.91%

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

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 Scc stands for or maybe even write out the full name.
  • Add a test case of circular dependency graphs.

@sokra sokra merged commit fc4cfaa into main Jul 21, 2023
35 of 37 checks passed
@sokra sokra deleted the hrmny/web-434-async-modules branch July 21, 2023 06:53
sokra added a commit to vercel/next.js that referenced this pull request Jul 21, 2023
* 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>
ForsakenHarmony added a commit to vercel/next.js that referenced this pull request Jul 25, 2024
### 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
ForsakenHarmony added a commit to vercel/next.js that referenced this pull request Jul 29, 2024
### 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
ForsakenHarmony added a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants