Tree Shaking shared state and side effects #8370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Ignored Deployments
|
sokra
commented
Jun 7, 2024
Comment on lines
15
to
24
| ## Item 2: Stmt 1, `VarDeclarator(0)` | ||
|
|
||
| ```js | ||
| const value = externalFunction(); | ||
|
|
||
| ``` | ||
|
|
||
| - Declares: `value` | ||
| - Reads: `externalFunction` | ||
| - Write: `value` |
There was a problem hiding this comment.
This should be flagged as side effect as it calls an external function.
sokra
commented
Jun 7, 2024
Comment on lines
26
to
35
| ## Item 3: Stmt 2, `VarDeclarator(0)` | ||
|
|
||
| ```js | ||
| const value2 = externalObject.propertyWithGetter; | ||
|
|
||
| ``` | ||
|
|
||
| - Declares: `value2` | ||
| - Reads: `externalObject` | ||
| - Write: `value2` |
There was a problem hiding this comment.
This should be flagged as side effect as it accesses a property on an object which might have an getter.
sokra
commented
Jun 7, 2024
|
|
||
| ``` | ||
|
|
||
| - Side effects |
🟢 Turbopack Benchmark CI successful 🟢Thanks |
sokra
commented
Jun 7, 2024
Comment on lines
47
to
56
| ## Item 5: Stmt 4, `VarDeclarator(0)` | ||
|
|
||
| ```js | ||
| const value3 = externalFunction(); | ||
|
|
||
| ``` | ||
|
|
||
| - Declares: `value3` | ||
| - Reads: `externalFunction` | ||
| - Write: `value3` |
There was a problem hiding this comment.
This should not be flagged as side effect as the function call was annotated with PURE.
sokra
commented
Jun 7, 2024
...bopack-tests/tests/execution/turbopack/tree-shaking/split-chunks-shared-state/input/index.js
Show resolved
Hide resolved
|
✅ This change can build |
|
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md
Show resolved
Hide resolved
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md
Show resolved
Hide resolved
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md
Outdated
Show resolved
Hide resolved
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md
Outdated
Show resolved
Hide resolved
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/write-order/output.md
Show resolved
Hide resolved
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/complex/output.md
Outdated
Show resolved
Hide resolved
kdy1
approved these changes
Jun 10, 2024
sokra
commented
Jun 10, 2024
crates/turbopack-ecmascript/tests/tree-shaker/analyzer/node-globals/output.md
Show resolved
Hide resolved
sokra
commented
Jun 10, 2024
| @@ -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$__ | |||
There was a problem hiding this comment.
That does seem unneeded, but can be follow up fix
|
Approved |
ForsakenHarmony
added a commit
to vercel/next.js
that referenced
this pull request
Jun 10, 2024
### 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 <!-- Tim Neutkens - Rename ChunkLoading::None to ChunkLoading::Edge --> * vercel/turborepo#8371 <!-- Donny/강동윤 - test: Add an execution test for `paren_remover` --> * vercel/turborepo#8370 <!-- Tobias Koppers - Tree Shaking shared state and side effects --> * vercel/turborepo#7988 <!-- hrmny - feat(turbopack-ecmascript): cache external modules with wrapper --> Closes PACK-2622
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 25, 2024
### Description adds more test cases ### Testing Instructions <!-- Give a quick description of steps to test your changes. --> --------- Co-authored-by: 강동윤 (Donny) <[email protected]>
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 29, 2024
### Description adds more test cases ### Testing Instructions <!-- Give a quick description of steps to test your changes. --> --------- Co-authored-by: 강동윤 (Donny) <[email protected]>
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Jul 29, 2024
### Description adds more test cases ### Testing Instructions <!-- Give a quick description of steps to test your changes. --> --------- Co-authored-by: 강동윤 (Donny) <[email protected]>
ForsakenHarmony
pushed a commit
to vercel/next.js
that referenced
this pull request
Aug 1, 2024
### Description adds more test cases ### Testing Instructions <!-- Give a quick description of steps to test your changes. --> --------- Co-authored-by: 강동윤 (Donny) <[email protected]>
ForsakenHarmony
added a commit
to vercel/next.js
that referenced
this pull request
Aug 16, 2024
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 <!-- Tim Neutkens - Rename ChunkLoading::None to ChunkLoading::Edge --> * vercel/turborepo#8371 <!-- Donny/강동윤 - test: Add an execution test for `paren_remover` --> * vercel/turborepo#8370 <!-- Tobias Koppers - Tree Shaking shared state and side effects --> * vercel/turborepo#7988 <!-- hrmny - feat(turbopack-ecmascript): cache external modules with wrapper --> Closes PACK-2622
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
adds more test cases
Testing Instructions