Skip to content

Conversation

@mischnic
Copy link
Contributor

@mischnic mischnic commented Nov 3, 2025

We had tree shaking enabled for NFT.
That is incorrect however, as even unused imports (if you don't use any of the imported bindings, and the file is marked as sideEffects: false) are still needed at runtime, since Node.js will execute everything anyway.

  • I made the treeShakingMode: None explicit now (though that was already the Default::default anyway)
  • The real problem was that we still dropped side-effect free ModulePart::Evaluation references even when tree shaking was disabled.

Closes #85172
Fixes PACK-5756

Copy link
Contributor Author

mischnic commented Nov 3, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mischnic mischnic changed the title Add test Turbopack: Nov 3, 2025
@mischnic mischnic changed the title Turbopack: Turbopack: disable tree shaking for tracing Nov 3, 2025
@mischnic mischnic marked this pull request as ready for review November 3, 2025 09:41
@mischnic mischnic requested a review from a team November 3, 2025 09:43
@ijjk
Copy link
Member

ijjk commented Nov 3, 2025

Tests Passed

@mischnic mischnic force-pushed the mischnic/trace-external-side-effects branch from c6d2160 to fac0225 Compare November 3, 2025 10:38
@ijjk ijjk added created-by: Turbopack team PRs by the Turbopack team. tests Turbopack Related to Turbopack with Next.js. labels Nov 3, 2025
@mischnic mischnic merged commit 3883a8d into canary Nov 4, 2025
280 of 290 checks passed
@mischnic mischnic deleted the mischnic/trace-external-side-effects branch November 4, 2025 09:24
huozhi pushed a commit that referenced this pull request Nov 12, 2025
We had tree shaking enabled for NFT.
That is incorrect however, as even unused imports (if you don't use any of the imported bindings, and the file is marked as `sideEffects: false`) are still needed at runtime, since Node.js will execute everything anyway.

- I made the `treeShakingMode: None` explicit now (though that was already the `Default::default` anyway)
- The real problem was that we still dropped side-effect free `ModulePart::Evaluation` references even when tree shaking was disabled.

Closes #85172
Fixes PACK-5756
huozhi pushed a commit that referenced this pull request Nov 12, 2025
We had tree shaking enabled for NFT.
That is incorrect however, as even unused imports (if you don't use any of the imported bindings, and the file is marked as `sideEffects: false`) are still needed at runtime, since Node.js will execute everything anyway.

- I made the `treeShakingMode: None` explicit now (though that was already the `Default::default` anyway)
- The real problem was that we still dropped side-effect free `ModulePart::Evaluation` references even when tree shaking was disabled.

Closes #85172
Fixes PACK-5756
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

created-by: Turbopack team PRs by the Turbopack team. locked tests Turbopack Related to Turbopack with Next.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Turbopack tree-shaking generates invalid output for some pages router SSR chunks

4 participants