Tree Shaking shared state and side effects #8370

sokra · 2024-06-07T08:19:34Z

Description

adds more test cases

Testing Instructions

vercel · 2024-06-07T08:19:37Z

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	Jun 10, 2024 0:44am
rust-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 10, 2024 0:44am

8 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	Jun 10, 2024 0:44am

sokra · 2024-06-07T08:20:01Z

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/shared-and-side-effects/output.md

+## Item 2: Stmt 1, `VarDeclarator(0)`
+
+```js
+const value = externalFunction();
+
+```
+
+- Declares: `value`
+- Reads: `externalFunction`
+- Write: `value`


This should be flagged as side effect as it calls an external function.

sokra · 2024-06-07T08:20:35Z

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/shared-and-side-effects/output.md

+## Item 3: Stmt 2, `VarDeclarator(0)`
+
+```js
+const value2 = externalObject.propertyWithGetter;
+
+```
+
+- Declares: `value2`
+- Reads: `externalObject`
+- Write: `value2`


This should be flagged as side effect as it accesses a property on an object which might have an getter.

sokra · 2024-06-07T08:20:43Z

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/shared-and-side-effects/output.md

+
+```
+
+- Side effects


That's correct

github-actions · 2024-06-07T08:21:39Z

🟢 Turbopack Benchmark CI successful 🟢

Thanks

sokra · 2024-06-07T08:22:50Z

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/shared-and-side-effects/output.md

+## Item 5: Stmt 4, `VarDeclarator(0)`
+
+```js
+const value3 = externalFunction();
+
+```
+
+- Declares: `value3`
+- Reads: `externalFunction`
+- Write: `value3`


This should not be flagged as side effect as the function call was annotated with PURE.

...bopack-tests/tests/execution/turbopack/tree-shaking/split-chunks-shared-state/input/index.js

github-actions · 2024-06-07T08:26:28Z

✅ This change can build next-swc

github-actions · 2024-06-07T08:30:16Z

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/complex/output.md

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/node-globals/output.md

sokra · 2024-06-10T13:27:41Z

...put/79fb1_turbopack-tests_tests_snapshot_basic-tree-shake_import-namespace_input_66459d._.js

@@ -217,14 +217,17 @@ var __TURBOPACK__imported__module__$5b$project$5d2f$crates$2f$turbopack$2d$tests
 "[project]/crates/turbopack-tests/tests/snapshot/basic-tree-shake/import-namespace/input/index.js [test] (ecmascript) <module evaluation>": (({ r: __turbopack_require__, f: __turbopack_module_context__, i: __turbopack_import__, s: __turbopack_esm__, v: __turbopack_export_value__, n: __turbopack_export_namespace__, c: __turbopack_cache__, M: __turbopack_modules__, l: __turbopack_load__, j: __turbopack_dynamic__, P: __turbopack_resolve_absolute_path__, U: __turbopack_relative_url__, R: __turbopack_resolve_module_id_path__, g: global, __dirname }) => (() => {
 "use strict";

-__turbopack_esm__({});
+__turbopack_esm__({
+    "lib": ()=>__TURBOPACK__imported__module__$5b$project$5d2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$basic$2d$tree$2d$shake$2f$import$2d$namespace$2f$input$2f$lib$2e$js__$5b$test$5d$__$28$ecmascript$29$__$3c$facade$3e$__


That does seem unneeded, but can be follow up fix

sokra · 2024-06-10T13:28:18Z

Approved

### What? This is a fix for a bundle potentially referring to two different external modules (e.g. `react` because it gets invalided from the require cache) See vercel/turborepo#7988 ### Turbopack Updates * vercel/turborepo#8376  * vercel/turborepo#8371  * vercel/turborepo#8370  * vercel/turborepo#7988  Closes PACK-2622

### Description adds more test cases ### Testing Instructions  --------- Co-authored-by: 강동윤 (Donny) <kdy1997.dev@gmail.com>

This is a fix for a bundle potentially referring to two different external modules (e.g. `react` because it gets invalided from the require cache) See vercel/turborepo#7988 * vercel/turborepo#8376  * vercel/turborepo#8371  * vercel/turborepo#8370  * vercel/turborepo#7988  Closes PACK-2622

add test cases

98f6cf8

turbo-orchestrator bot added created-by: turbopack labels Jun 7, 2024

sokra commented Jun 7, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/shared-and-side-effects/output.md

```

- Side effects

Copy link

Member Author

sokra Jun 7, 2024

Choose a reason for hiding this comment

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

That's correct

vercel bot deployed to Preview – rust-docs June 7, 2024 08:21 View deployment

add pure case

3292cf1

vercel bot deployed to Preview – examples-nonmonorepo June 7, 2024 08:22 View deployment

sokra commented Jun 7, 2024

View reviewed changes

...bopack-tests/tests/execution/turbopack/tree-shaking/split-chunks-shared-state/input/index.js Show resolved Hide resolved

vercel bot deployed to Preview – rust-docs June 7, 2024 08:24 View deployment

add test case

fd78a99

vercel bot deployed to Preview – examples-nonmonorepo June 10, 2024 08:03 View deployment

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md Show resolved Hide resolved

Treat obj of member expr as write

b0ac8ff

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md Show resolved Hide resolved

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md Outdated Show resolved Hide resolved

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md Outdated Show resolved Hide resolved

vercel bot deployed to Preview – rust-docs June 10, 2024 08:05 View deployment

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md Show resolved Hide resolved

kdy1 added 2 commits June 10, 2024 17:08

side_effects for unresolved

051bbcb

Update test refs

8c489b4

vercel bot deployed to Preview – examples-nonmonorepo June 10, 2024 08:09 View deployment

vercel bot deployed to Preview – rust-docs June 10, 2024 08:11 View deployment

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/complex/output.md Outdated Show resolved Hide resolved

Update test refs

4de570d

vercel bot deployed to Preview – examples-nonmonorepo June 10, 2024 09:28 View deployment

vercel bot deployed to Preview – rust-docs June 10, 2024 09:30 View deployment

kdy1 approved these changes Jun 10, 2024

View reviewed changes

sokra commented Jun 10, 2024

View reviewed changes

crates/turbopack-ecmascript/tests/tree-shaker/analyzer/node-globals/output.md Show resolved Hide resolved

kdy1 self-assigned this Jun 10, 2024

kdy1 added 2 commits June 10, 2024 21:16

vars.found_unresolved

08aa8b3

Update test refs

89b3bdf

vercel bot deployed to Preview – examples-nonmonorepo June 10, 2024 12:18 View deployment

vercel bot deployed to Preview – rust-docs June 10, 2024 12:20 View deployment

kdy1 added 5 commits June 10, 2024 21:33

Merge two visitors

0bc9de0

fix visitor

cc17957

revert visitor patch

f7d9936

side_effects of assign

9065c1d

Update test refs

83f895e

vercel bot deployed to Preview – examples-nonmonorepo June 10, 2024 12:42 View deployment

vercel bot deployed to Preview – rust-docs June 10, 2024 12:44 View deployment

sokra commented Jun 10, 2024

View reviewed changes

sokra marked this pull request as ready for review June 10, 2024 13:28

sokra requested a review from a team as a code owner June 10, 2024 13:28

sokra merged commit 4061e06 into main Jun 10, 2024
56 of 58 checks passed

sokra deleted the sokra/tree-shaking-shared branch June 10, 2024 13:28

ForsakenHarmony mentioned this pull request Jun 10, 2024

feat(turbopack-ecmascript): cache external modules with wrapper vercel/next.js#63337

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tree Shaking shared state and side effects #8370

Tree Shaking shared state and side effects #8370

sokra commented Jun 7, 2024

vercel bot commented Jun 7, 2024 •

edited

Loading

sokra Jun 7, 2024

sokra Jun 7, 2024

sokra Jun 7, 2024

github-actions bot commented Jun 7, 2024 •

edited

Loading

sokra Jun 7, 2024

github-actions bot commented Jun 7, 2024

github-actions bot commented Jun 7, 2024 •

edited

Loading

sokra Jun 10, 2024

sokra commented Jun 10, 2024

Tree Shaking shared state and side effects #8370

Tree Shaking shared state and side effects #8370

Conversation

sokra commented Jun 7, 2024

Description

Testing Instructions

vercel bot commented Jun 7, 2024 • edited Loading

sokra Jun 7, 2024

Choose a reason for hiding this comment

sokra Jun 7, 2024

Choose a reason for hiding this comment

sokra Jun 7, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 7, 2024 • edited Loading

🟢 Turbopack Benchmark CI successful 🟢

sokra Jun 7, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 7, 2024

github-actions bot commented Jun 7, 2024 • edited Loading

⚠️ CI failed ⚠️

sokra Jun 10, 2024

Choose a reason for hiding this comment

sokra commented Jun 10, 2024

vercel bot commented Jun 7, 2024 •

edited

Loading

github-actions bot commented Jun 7, 2024 •

edited

Loading

github-actions bot commented Jun 7, 2024 •

edited

Loading