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: Fix tree shaking even more #8864

Merged
merged 10 commits into from
Jul 30, 2024
Merged

fix: Fix tree shaking even more #8864

merged 10 commits into from
Jul 30, 2024

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Jul 29, 2024

Description

Fix server action and rename the injected variable.

The previous code fails if the name of the top-level variable is v. This was not a problem for almost all modules, but some packages like @firebase/webchannel-wrapper is published after mangling, so it can be a problem

Testing Instructions

See vercel/next.js#66689

kdy1 added 4 commits July 29, 2024 17:42
This reverts commit 3304ada5149e3b0ebb9f51bfaa52d7a45b1fb0db.
Copy link

vercel bot commented Jul 29, 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 Jul 30, 2024 8:04am
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 30, 2024 8:04am
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 8:04am

Copy link
Contributor

github-actions bot commented Jul 29, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Jul 29, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

kdy1 added 3 commits July 29, 2024 20:06
This reverts commit 7c0898e.
This reverts commit fd34007.
@kdy1 kdy1 changed the title fix: Fix tree shaking fix: Fix tree shaking even mroe Jul 30, 2024
@kdy1 kdy1 changed the title fix: Fix tree shaking even mroe fix: Fix tree shaking even more Jul 30, 2024
@kdy1 kdy1 marked this pull request as ready for review July 30, 2024 05:40
@kdy1 kdy1 requested a review from a team as a code owner July 30, 2024 05:40
@@ -175,7 +175,7 @@ impl Module for EcmascriptModulePartAsset {
impl Asset for EcmascriptModulePartAsset {
#[turbo_tasks::function]
fn content(&self) -> Vc<AssetContent> {
todo!()
self.full_module.content()
Copy link
Member Author

Choose a reason for hiding this comment

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

The value is still not used.

@kdy1 kdy1 requested a review from sokra July 30, 2024 08:02
@kdy1 kdy1 enabled auto-merge (squash) July 30, 2024 10:51
@kdy1 kdy1 merged commit deee36b into main Jul 30, 2024
58 of 60 checks passed
@kdy1 kdy1 deleted the kdy1/tree-shake branch July 30, 2024 10:51
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 30, 2024
### Description

Fix server action and rename the injected variable.

The previous code fails if the name of the top-level variable is `v`.
This was not a problem for almost all modules, but some packages like
[`@firebase/webchannel-wrapper`](https://unpkg.com/@firebase/webchannel-wrapper@0.2.41/dist/index.esm.js)
is published after mangling, so it can be a problem



### Testing Instructions

See #66689
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### Description

Fix server action and rename the injected variable.

The previous code fails if the name of the top-level variable is `v`.
This was not a problem for almost all modules, but some packages like
[`@firebase/webchannel-wrapper`](https://unpkg.com/@firebase/webchannel-wrapper@0.2.41/dist/index.esm.js)
is published after mangling, so it can be a problem



### Testing Instructions

See #66689
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.

2 participants