这是indexloc提供的服务,不要输入任何密码
Skip to content

Fix deduplication when importing module statically and dynamically #3297

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

Merged
merged 6 commits into from
Jan 13, 2023

Conversation

jridgewell
Copy link
Contributor

The processed_assets IndexSet was performing deduplication on the AssetVc, which is the same between static (import ... from "foo") and dynamic (import("foo")) imports of the same specifier. But, the reference to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with #3193's ability to load chunks on the server side, this fixes fixes WEB-381.

The `processed_assets` `IndexSet` was performing deduplication on the `AssetVc`, which is the same between static (`import ... from "foo"`) and dynamic (`import("foo")`) imports of the same specifier. But, the _reference_ to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with #3193's ability to load chunks on the server side, this fixes fixes WEB-381.
@jridgewell jridgewell requested a review from alexkirsz January 12, 2023 21:40
@jridgewell jridgewell requested a review from a team as a code owner January 12, 2023 21:40
@vercel
Copy link

vercel bot commented Jan 12, 2023

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

Name Status Preview Comments Updated
turbo-site 🔄 Building (Inspect) Jan 13, 2023 at 4:39AM (UTC)
9 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)
turbo-vite-web ⬜️ Ignored (Inspect) Jan 13, 2023 at 4:39AM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Jan 12, 2023

🟢 CI successful 🟢

Thanks

@github-actions
Copy link
Contributor

Benchmark for 3ac5c11

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8526.13µs ± 40.43µs 8523.70µs ± 57.65µs -0.03%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8920.30µs ± 78.96µs 8984.00µs ± 74.34µs +0.71%
bench_hmr_to_commit/Turbopack RSC/1000 modules 475.90ms ± 1.99ms 481.17ms ± 4.25ms +1.11%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8564.35µs ± 59.22µs 8523.36µs ± 34.58µs -0.48%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7511.44µs ± 55.14µs 7376.26µs ± 47.91µs -1.80%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7632.84µs ± 84.04µs 7687.13µs ± 53.92µs +0.71%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7642.59µs ± 61.20µs 7662.74µs ± 56.61µs +0.26%
bench_hydration/Turbopack RCC/1000 modules 3374.44ms ± 8.41ms 3342.29ms ± 11.93ms -0.95%
bench_hydration/Turbopack RSC/1000 modules 2865.70ms ± 10.70ms 2850.31ms ± 13.02ms -0.54%
bench_hydration/Turbopack SSR/1000 modules 2675.51ms ± 10.78ms 2686.37ms ± 8.45ms +0.41%
bench_startup/Turbopack CSR/1000 modules 1649.14ms ± 5.29ms 1647.38ms ± 4.22ms -0.11%
bench_startup/Turbopack RCC/1000 modules 2560.02ms ± 10.71ms 2542.08ms ± 10.43ms -0.70%
bench_startup/Turbopack RSC/1000 modules 2404.60ms ± 6.96ms 2394.31ms ± 8.05ms -0.43%
bench_startup/Turbopack SSR/1000 modules 2057.77ms ± 4.23ms 2066.52ms ± 4.63ms +0.43%

@github-actions
Copy link
Contributor

Benchmark for d703658

Test Base PR % Significant %
bench_hydration/Turbopack RCC/1000 modules 3378.81ms ± 6.79ms 3333.62ms ± 8.19ms -1.34% -0.45%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8721.29µs ± 77.06µs 8824.73µs ± 59.00µs +1.19%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8845.64µs ± 76.72µs 8824.28µs ± 56.44µs -0.24%
bench_hmr_to_commit/Turbopack RSC/1000 modules 468.94ms ± 1.30ms 475.13ms ± 2.73ms +1.32%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8740.28µs ± 67.93µs 8781.65µs ± 95.00µs +0.47%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7540.73µs ± 59.13µs 7659.37µs ± 44.48µs +1.57%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7773.41µs ± 62.17µs 7739.12µs ± 24.12µs -0.44%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7668.87µs ± 59.02µs 7582.41µs ± 81.81µs -1.13%
bench_hydration/Turbopack RCC/1000 modules 3378.81ms ± 6.79ms 3333.62ms ± 8.19ms -1.34% -0.45%
bench_hydration/Turbopack RSC/1000 modules 2861.58ms ± 9.62ms 2863.51ms ± 9.14ms +0.07%
bench_hydration/Turbopack SSR/1000 modules 2669.49ms ± 14.06ms 2663.35ms ± 7.93ms -0.23%
bench_startup/Turbopack CSR/1000 modules 1644.00ms ± 5.11ms 1642.99ms ± 9.44ms -0.06%
bench_startup/Turbopack RCC/1000 modules 2506.72ms ± 8.07ms 2530.46ms ± 9.15ms +0.95%
bench_startup/Turbopack RSC/1000 modules 2402.49ms ± 6.51ms 2404.28ms ± 5.15ms +0.07%
bench_startup/Turbopack SSR/1000 modules 2048.28ms ± 5.49ms 2040.89ms ± 5.42ms -0.36%

@jridgewell jridgewell added the pr: automerge Kodiak will merge these automatically after checks pass label Jan 12, 2023
@github-actions
Copy link
Contributor

Benchmark for 05958a2

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8867.32µs ± 71.23µs 8954.85µs ± 67.86µs +0.99%
bench_hmr_to_commit/Turbopack RCC/1000 modules 9175.96µs ± 29.25µs 9159.88µs ± 60.93µs -0.18%
bench_hmr_to_commit/Turbopack RSC/1000 modules 498.13ms ± 2.38ms 499.62ms ± 2.71ms +0.30%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8991.66µs ± 61.60µs 8979.22µs ± 85.12µs -0.14%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7887.39µs ± 54.30µs 7852.62µs ± 61.28µs -0.44%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7964.69µs ± 57.15µs 7987.71µs ± 70.90µs +0.29%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7939.95µs ± 51.65µs 7933.22µs ± 81.19µs -0.08%
bench_hydration/Turbopack RCC/1000 modules 3553.13ms ± 13.21ms 3542.92ms ± 6.65ms -0.29%
bench_hydration/Turbopack RSC/1000 modules 3004.51ms ± 5.25ms 3012.99ms ± 6.68ms +0.28%
bench_hydration/Turbopack SSR/1000 modules 2780.21ms ± 13.84ms 2764.31ms ± 6.27ms -0.57%
bench_startup/Turbopack CSR/1000 modules 1729.77ms ± 5.35ms 1722.22ms ± 4.34ms -0.44%
bench_startup/Turbopack RCC/1000 modules 2657.01ms ± 12.60ms 2642.45ms ± 7.95ms -0.55%
bench_startup/Turbopack RSC/1000 modules 2534.13ms ± 7.59ms 2545.84ms ± 10.00ms +0.46%
bench_startup/Turbopack SSR/1000 modules 2144.79ms ± 4.66ms 2156.50ms ± 7.00ms +0.55%

@github-actions
Copy link
Contributor

Benchmark for 435d428

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8707.14µs ± 88.77µs 8731.50µs ± 58.61µs +0.28%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8944.49µs ± 26.06µs 9004.12µs ± 89.27µs +0.67%
bench_hmr_to_commit/Turbopack RSC/1000 modules 488.89ms ± 3.14ms 486.25ms ± 3.10ms -0.54%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8745.95µs ± 74.91µs 8776.40µs ± 65.24µs +0.35%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7628.99µs ± 34.03µs 7728.19µs ± 47.14µs +1.30%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7756.70µs ± 80.24µs 7849.69µs ± 53.67µs +1.20%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7781.78µs ± 68.58µs 7700.12µs ± 87.23µs -1.05%
bench_hydration/Turbopack RCC/1000 modules 3434.05ms ± 14.19ms 3450.14ms ± 12.55ms +0.47%
bench_hydration/Turbopack RSC/1000 modules 2926.64ms ± 11.00ms 2919.11ms ± 9.52ms -0.26%
bench_hydration/Turbopack SSR/1000 modules 2711.17ms ± 12.62ms 2710.77ms ± 6.88ms -0.01%
bench_startup/Turbopack CSR/1000 modules 1675.71ms ± 3.95ms 1675.13ms ± 5.63ms -0.03%
bench_startup/Turbopack RCC/1000 modules 2571.62ms ± 12.56ms 2564.45ms ± 9.89ms -0.28%
bench_startup/Turbopack RSC/1000 modules 2450.52ms ± 10.83ms 2455.86ms ± 8.45ms +0.22%
bench_startup/Turbopack SSR/1000 modules 2097.90ms ± 5.93ms 2087.81ms ± 5.55ms -0.48%

@github-actions
Copy link
Contributor

Benchmark for 4d29b37

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8591.47µs ± 47.48µs 8635.91µs ± 72.34µs +0.52%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8873.28µs ± 84.54µs 8891.61µs ± 59.83µs +0.21%
bench_hmr_to_commit/Turbopack RSC/1000 modules 489.86ms ± 5.27ms 491.84ms ± 1.69ms +0.40%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8719.53µs ± 65.26µs 8725.88µs ± 74.81µs +0.07%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7546.06µs ± 61.68µs 7602.31µs ± 52.78µs +0.75%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7833.16µs ± 55.70µs 7753.88µs ± 69.85µs -1.01%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7672.26µs ± 50.48µs 7699.01µs ± 83.24µs +0.35%
bench_hydration/Turbopack RCC/1000 modules 3450.45ms ± 14.96ms 3431.25ms ± 11.05ms -0.56%
bench_hydration/Turbopack RSC/1000 modules 2943.79ms ± 13.61ms 2932.57ms ± 8.30ms -0.38%
bench_hydration/Turbopack SSR/1000 modules 2716.25ms ± 8.54ms 2733.85ms ± 10.65ms +0.65%
bench_startup/Turbopack CSR/1000 modules 1690.55ms ± 5.25ms 1693.76ms ± 4.71ms +0.19%
bench_startup/Turbopack RCC/1000 modules 2581.15ms ± 11.88ms 2593.29ms ± 9.73ms +0.47%
bench_startup/Turbopack RSC/1000 modules 2457.88ms ± 7.62ms 2472.65ms ± 9.43ms +0.60%
bench_startup/Turbopack SSR/1000 modules 2098.25ms ± 4.86ms 2101.33ms ± 6.49ms +0.15%

@@ -407,7 +409,7 @@ async fn chunk_content_internal<I: FromChunkableAsset>(
for asset in assets
.await?
.iter()
.filter(|asset| processed_assets.insert(**asset))
.filter(|asset| processed_assets.insert((chunking_type, **asset)))
Copy link
Member

Choose a reason for hiding this comment

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

This causes Placed and PlacedOrParallel to generate two chunk items.

Can we add a processed_assets.insert((Placed, **asset)) into the placed code path in PlacedOrParallel and a processed_assets.insert((Parallel, **asset)) into the parallel code path.

@kodiakhq kodiakhq bot merged commit d616751 into main Jan 13, 2023
@kodiakhq kodiakhq bot deleted the jrl-fix-sync-async-import branch January 13, 2023 05:48
jridgewell added a commit to vercel/next.js that referenced this pull request Mar 10, 2023
…ercel/turborepo#3297)

The `processed_assets` `IndexSet` was performing deduplication on the `AssetVc`, which is the same between static (`import ... from "foo"`) and dynamic (`import("foo")`) imports of the same specifier. But, the _reference_ to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with vercel/turborepo#3193's ability to load chunks on the server side, this fixes fixes WEB-381.
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
…ercel/turborepo#3297)

The `processed_assets` `IndexSet` was performing deduplication on the `AssetVc`, which is the same between static (`import ... from "foo"`) and dynamic (`import("foo")`) imports of the same specifier. But, the _reference_ to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with vercel/turborepo#3193's ability to load chunks on the server side, this fixes fixes WEB-381.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…ercel/turborepo#3297)

The `processed_assets` `IndexSet` was performing deduplication on the `AssetVc`, which is the same between static (`import ... from "foo"`) and dynamic (`import("foo")`) imports of the same specifier. But, the _reference_ to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with #3193's ability to load chunks on the server side, this fixes fixes WEB-381.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…ercel/turborepo#3297)

The `processed_assets` `IndexSet` was performing deduplication on the `AssetVc`, which is the same between static (`import ... from "foo"`) and dynamic (`import("foo")`) imports of the same specifier. But, the _reference_ to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with #3193's ability to load chunks on the server side, this fixes fixes WEB-381.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…ercel/turborepo#3297)

The `processed_assets` `IndexSet` was performing deduplication on the `AssetVc`, which is the same between static (`import ... from "foo"`) and dynamic (`import("foo")`) imports of the same specifier. But, the _reference_ to those assets is different, and generates different chunking semantics. In order for both to succeed, we need to process both assets.

Coupled with #3193's ability to load chunks on the server side, this fixes fixes WEB-381.
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.

2 participants