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

Import Next's CJS AsyncLocalStorage modules #3634

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

jridgewell
Copy link
Contributor

The node evaluation always renders with type: "commonjs" and require() calls, so we'll always import the CJS files. But here we're importing the ESM files. That means we have 2 distinct instances of requestAsyncStorage in our node instance, and they cannot properly communicate with the other.

Fixes WEB-543

The node evaluation always renders with `type: "commonjs"` and `require()` calls, so we'll always import the CJS files. But here we're importing the ESM files. That means we have 2 distinct instances of `requestAsyncStorage` in our node instance, and they cannot properly communicate with the other.
@jridgewell jridgewell requested a review from a team as a code owner February 4, 2023 05:11
@vercel
Copy link

vercel bot commented Feb 4, 2023

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

Name Status Preview Comments Updated
examples-tailwind-web 🔄 Building (Inspect) Feb 6, 2023 at 10:28PM (UTC)
turbo-site 🔄 Building (Inspect) Visit Preview Feb 6, 2023 at 10:28PM (UTC)
8 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)
examples-vite-web ⬜️ Ignored (Inspect) Feb 6, 2023 at 10:28PM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

Benchmark for 812f0b6

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.22ms ± 0.09ms 10.18ms ± 0.10ms -0.35%
bench_hmr_to_commit/Turbopack RCC/1000 modules 10.32ms ± 0.10ms 10.32ms ± 0.08ms -0.03%
bench_hmr_to_commit/Turbopack RSC/1000 modules 503.17ms ± 1.38ms 503.73ms ± 1.45ms +0.11%
bench_hmr_to_commit/Turbopack SSR/1000 modules 10.32ms ± 0.10ms 10.30ms ± 0.08ms -0.14%
bench_hmr_to_eval/Turbopack CSR/1000 modules 9100.56µs ± 86.33µs 9185.21µs ± 72.79µs +0.93%
bench_hmr_to_eval/Turbopack RCC/1000 modules 9322.54µs ± 75.15µs 9268.01µs ± 85.49µs -0.58%
bench_hmr_to_eval/Turbopack SSR/1000 modules 9172.90µs ± 54.68µs 9210.71µs ± 77.38µs +0.41%
bench_hydration/Turbopack RCC/1000 modules 4314.27ms ± 7.97ms 4303.15ms ± 9.98ms -0.26%
bench_hydration/Turbopack RSC/1000 modules 3911.30ms ± 12.81ms 3897.35ms ± 25.27ms -0.36%
bench_hydration/Turbopack SSR/1000 modules 3694.27ms ± 13.23ms 3719.49ms ± 10.10ms +0.68%
bench_startup/Turbopack CSR/1000 modules 2817.30ms ± 8.72ms 2807.04ms ± 12.29ms -0.36%
bench_startup/Turbopack RCC/1000 modules 2619.16ms ± 5.32ms 2600.92ms ± 7.04ms -0.70%
bench_startup/Turbopack RSC/1000 modules 2530.47ms ± 12.22ms 2542.46ms ± 8.43ms +0.47%
bench_startup/Turbopack SSR/1000 modules 2087.13ms ± 3.21ms 2075.56ms ± 2.58ms -0.55%

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

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.

Could we add a test case for this to not regress on that?

@jridgewell jridgewell requested a review from a team as a code owner February 6, 2023 22:28
@jridgewell
Copy link
Contributor Author

Added test case

@jridgewell jridgewell added the pr: automerge Kodiak will merge these automatically after checks pass label Feb 6, 2023
@kodiakhq kodiakhq bot merged commit c4d9170 into main Feb 6, 2023
@kodiakhq kodiakhq bot deleted the jrl-async-local-storage-fix-esm-cjs branch February 6, 2023 22:47
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

Benchmark for 8f4927f

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9089.00µs ± 95.13µs 9115.75µs ± 49.82µs +0.29%
bench_hmr_to_commit/Turbopack RCC/1000 modules 9207.16µs ± 74.78µs 9313.47µs ± 74.21µs +1.15%
bench_hmr_to_commit/Turbopack RSC/1000 modules 487.10ms ± 1.95ms 486.35ms ± 1.58ms -0.15%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9277.64µs ± 110.34µs 9288.49µs ± 72.84µs +0.12%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8043.60µs ± 58.56µs 8202.45µs ± 90.06µs +1.97%
bench_hmr_to_eval/Turbopack RCC/1000 modules 8373.00µs ± 86.51µs 8307.20µs ± 76.49µs -0.79%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8215.39µs ± 57.53µs 8168.85µs ± 79.43µs -0.57%
bench_hydration/Turbopack RCC/1000 modules 4166.95ms ± 7.75ms 4167.71ms ± 11.45ms +0.02%
bench_hydration/Turbopack RSC/1000 modules 3744.83ms ± 16.83ms 3732.43ms ± 15.74ms -0.33%
bench_hydration/Turbopack SSR/1000 modules 3631.14ms ± 21.78ms 3630.68ms ± 21.37ms -0.01%
bench_startup/Turbopack CSR/1000 modules 2745.46ms ± 9.31ms 2742.00ms ± 6.49ms -0.13%
bench_startup/Turbopack RCC/1000 modules 2528.05ms ± 7.81ms 2522.21ms ± 6.88ms -0.23%
bench_startup/Turbopack RSC/1000 modules 2419.71ms ± 7.33ms 2418.39ms ± 5.07ms -0.05%
bench_startup/Turbopack SSR/1000 modules 2083.56ms ± 3.32ms 2080.33ms ± 2.40ms -0.16%

jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
The node evaluation always renders with `type: "commonjs"` and `require()` calls, so we'll always import the CJS files. But here we're importing the ESM files. That means we have 2 distinct instances of `requestAsyncStorage` in our node instance, and they cannot properly communicate with the other.

Fixes WEB-543
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
The node evaluation always renders with `type: "commonjs"` and `require()` calls, so we'll always import the CJS files. But here we're importing the ESM files. That means we have 2 distinct instances of `requestAsyncStorage` in our node instance, and they cannot properly communicate with the other.

Fixes WEB-543
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants