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.

Sign up for GitHub

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

Jump to bottom

Import Next's CJS AsyncLocalStorage modules #3634

Merged
merged 2 commits into from
Feb 6, 2023
Merged

Import Next's CJS AsyncLocalStorage modules #3634

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

@jridgewell
Import CJS modules
cd8d62b 
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 •
edited
Loading

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 •
edited
Loading

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

sokra
sokra approved these changes Feb 6, 2023
View reviewed changes
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%

This was referenced Feb 6, 2023
Closed
Merged
timneutkens pushed a commit to vercel/next.js that referenced this pull request Feb 10, 2023
@Brooooooklyn @kodiakhq
Update turbopack to 230209.02 (#45776)Co-authored-by: kodiakhq[bot] <…
946ccb0 
…49736102+kodiakhq[bot]@users.noreply.github.com>

# New Features
- vercel/turborepo#3540
- vercel/turborepo#3549
- vercel/turborepo#3465
- vercel/turborepo#3550
- vercel/turborepo#3495
- vercel/turborepo#3624
- vercel/turborepo#3600
- vercel/turborepo#3676
- vercel/turborepo#3689

# Fixes

- vercel/turborepo#3437
- vercel/turborepo#3542
- vercel/turborepo#3531
- vercel/turborepo#3552
- vercel/turborepo#3551
- vercel/turborepo#3597
- vercel/turborepo#3644
- vercel/turborepo#3623
- vercel/turborepo#3634
- vercel/turborepo#3574
- vercel/turborepo#3673
- vercel/turborepo#3675
- vercel/turborepo#3723
- vercel/turborepo#3677
- vercel/turborepo#3717
- vercel/turborepo#3701

# Performance Improvements

- vercel/turborepo#3361 
- vercel/turborepo#3619

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
@jridgewell
Import Next's CJS AsyncLocalStorage modules (vercel/turborepo#3634)
95dd877 
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
@jridgewell
Import Next's CJS AsyncLocalStorage modules (vercel/turborepo#3634)
9694fc4 
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
@DuCanhGH DuCanhGH mentioned this pull request Mar 15, 2023
Merged
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
@jridgewell
Import Next's CJS AsyncLocalStorage modules (vercel/turborepo#3634)
b6da52c 
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
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
@jridgewell
Import Next's CJS AsyncLocalStorage modules (vercel/turborepo#3634)
f46470a 
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
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
@jridgewell
Import Next's CJS AsyncLocalStorage modules (vercel/turborepo#3634)
205ce2f 
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
Reviewers

@sokra sokra sokra approved these changes

@NicholasLYang NicholasLYang Awaiting requested review from NicholasLYang NicholasLYang is a code owner automatically assigned from vercel/turbo-oss

@arlyon arlyon Awaiting requested review from arlyon arlyon is a code owner automatically assigned from vercel/turbo-oss

Assignees
No one assigned
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jridgewell @sokra