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

fix(turbopack): Fix edge cases of tree shaking #7986

Merged
merged 326 commits into from
May 28, 2024
Merged

fix(turbopack): Fix edge cases of tree shaking #7986

merged 326 commits into from
May 28, 2024

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Apr 16, 2024

Description

Closes PACK-2966

Testing Instructions

I'll add some tests

Copy link

vercel bot commented Apr 16, 2024

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

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 28, 2024 2:18am
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 28, 2024 2:18am
turbo-turbo-tracing-next-plugin ❌ Failed (Inspect) May 28, 2024 2:18am
8 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 2:18am

Copy link
Contributor

github-actions bot commented Apr 16, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented Apr 16, 2024

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Apr 16, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

@kdy1
Copy link
Member Author

kdy1 commented May 27, 2024

The test case with added issues is a wrong test case, but I'm not sure what should I do.

package-star/index.js

export { notCompiled } from "./not-compiled.js";
export { notExisting } from "./not-existing.js";
export { notExecuted } from "./not-executed.js";
export * from "./not-executed.js";
export * from "./a.js";
export * from "./b.js";
export const local = "local";

We import a and b from package-star in test case, but we cannot deduce that we can ignore

export { notCompiled } from "./not-compiled.js";
export { notExisting } from "./not-existing.js";
export { notExecuted } from "./not-executed.js";

because we have several export * from 'package-star's. We need to pass down a and b to export * from 'package-star's but I expect it to be a large work by itself.

So, I want to get this PR merged without it and work on it as a separate PR.

This is for two purposes:

  • I can start working on scope hoisting in parallel
  • I want to make WIP small.

Problematic case

package-reexport-tla-side-effect/index.js:

export * from "package-star";
export * from "./side-effect.js";
import "./side-effect2.js";
export const outer = "outer";

import { effect } from "./check-side-effect.js";

effect("index.js");

We import a and b from package-reexport-tla-side-effect, we can know that we don't need all exports, but we don't have a good way to pass those requirements to package-star.

@kdy1 kdy1 marked this pull request as ready for review May 27, 2024 06:03
@kdy1 kdy1 requested a review from sokra May 27, 2024 06:03
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 haven't fully reviewed it, but if it passes all tests it sounds fine to merge.
We can continue to iterate on it in future PRs

@kdy1 kdy1 enabled auto-merge (squash) May 28, 2024 02:16
@kdy1 kdy1 merged commit 4896611 into main May 28, 2024
52 of 53 checks passed
@kdy1 kdy1 deleted the kdy1/tree-shaking branch May 28, 2024 02:23
wbinnssmith added a commit to vercel/next.js that referenced this pull request May 28, 2024
This includes an update to lightningcss to 1.0.0-alpha.57

vercel/turbo#7986
vercel/turbo#8218
vercel/turbo#8222
wbinnssmith added a commit to vercel/next.js that referenced this pull request May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants