feat:✨remove unused specifier in importInfo #963
Conversation
需要有结尾行。 |
| const content = files["index.js"]; | ||
|
|
||
| assert( | ||
| content.includes("/a.js") && |
There was a problem hiding this comment.
这部分的 assert 是不是直接判定 现在 module 是什么样是不是更加直接
done |
done |
| // 1. import { a } from 'x'; | ||
| // 2. import a from 'x'; | ||
| // 3. import * as a from 'x'; | ||
| // if specifier not use and x has sideEffect, become import 'x'; |
There was a problem hiding this comment.
| // if specifier not use and x has sideEffect, become import 'x'; | |
| // if specifier is not used and x has sideEffect, convert to import 'x'; |
WalkthroughThe update enhances the Changes
Note Pull Request Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://coderabbit.ai Files selected for processing (1)
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/mako.config.jsonis excluded by:!**/*.json
Files selected for processing (8)
- crates/mako/src/plugins/farm_tree_shake/remove_useless_stmts.rs (2 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/expect.js (1 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/a.js (1 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/b.js (1 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/c.js (1 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/entry-a.js (1 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/entry-b.js (1 hunks)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/entry-c.js (1 hunks)
Files skipped from review due to trivial changes (1)
- e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/entry-b.js
Additional comments: 7
e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/entry-a.js (1)
- 1-1: The import statement aligns with the PR's objective to optimize default imports when the imported variable is not used. Ensure that the usage of
ais indeed optimized out in the broader context of the codebase.Verification successful
The verification process indicates that the import statement "import a from './a.js';" is only found in
entry-a.js, aligning with the PR's objective to optimize default imports when the imported variable is not used elsewhere in the codebase. This suggests that the optimization goal was likely achieved for this specific case. However, it's important to note that this verification does not explicitly confirm the absence ofa's usage withinentry-a.jsitself or potential indirect usages. The broader impact and effectiveness of the optimization should still be assessed in the context of the entire codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that 'a' is not used elsewhere in the codebase, indicating successful optimization. rg "import a from './a.js';" -C3Length of output: 140
e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/entry-c.js (1)
- 1-1: The import statement aligns with the PR's objective to optimize named imports when the imported variables are not used. Ensure that the usage of
candcccis indeed optimized out in the broader context of the codebase.e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/a.js (1)
- 1-5: The file correctly demonstrates a module with side effects, aligning with the PR's objectives to test the optimization of imports when the imported variable is not used but the module has side effects.
e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/b.js (1)
- 1-6: The file correctly demonstrates a module with side effects, aligning with the PR's objectives to test the optimization of imports when the imported variable is not used but the module has side effects.
e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/src/c.js (1)
- 1-4: The file correctly demonstrates a module with side effects, aligning with the PR's objectives to test the optimization of named imports when the imported variables are not used but the module has side effects.
e2e/fixtures/tree-shaking.side-effects-true.unused-specifier/expect.js (1)
- 1-61: The assertions are correctly designed to validate the optimization of import statements, aligning with the PR's objectives. This demonstrates thorough testing to ensure the effectiveness of the tree-shaking enhancements.
crates/mako/src/plugins/farm_tree_shake/remove_useless_stmts.rs (1)
- 10-16: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-123]
The logic to remove unused specifiers in import and export statements aligns with the PR's objectives and demonstrates good Rust practices. Ensure that all scenarios for import optimization are thoroughly tested, including edge cases.
|
|
如果模块 x 有副作用，但是变量 a 没有被使用，tree-shaking 完应该变成:
import 'x';
现在 master 只有 1 加了这个优化，补充 2 和 3
Summary by CodeRabbit
New Features
Tests
Documentation