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

revert: import namespace optimize #1606

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Conversation

stormslowly
Copy link
Member

@stormslowly stormslowly commented Sep 22, 2024

Summary by CodeRabbit

  • 新功能

    • 新增接口 TransformOutput,提供更结构化的数据处理。
  • 删除

    • 移除多个与树摇相关的文件和模块,简化了代码结构。
  • 重构

    • 精简了 remove_useless_stmts.rs 文件,移除未使用的导入和函数。

Copy link
Contributor

coderabbitai bot commented Sep 22, 2024

Walkthrough

本次更改涉及多个文件的修改和删除,主要集中在 mako crate 的树摇功能上。具体而言,移除了不再需要的模块和文件,简化了配置和逻辑,同时引入了一个新的接口以增强类型定义。此外,CI 工作流中的环境变量设置也进行了调整,以简化测试过程。

Changes

文件路径 更改摘要
.github/workflows/ci.yml 移除了 Test E2E 步骤中的 RUST_BACKTRACE 环境变量。
crates/mako/src/plugins/tree_shaking.rs 移除了 collect_explicit_prop 模块的导入。
crates/mako/src/plugins/tree_shaking/collect_explicit_prop.rs 删除了 IdExplicitPropAccessCollector 结构体及其相关实现。
crates/mako/src/plugins/tree_shaking/remove_useless_stmts.rs 移除了未使用的导入和 optimize_import_namespace 函数。
crates/mako/src/plugins/tree_shaking/shake.rs 修改了 optimize_modules 函数,直接调用 shake_module,移除了 HELPERS.set 块。
e2e/fixtures/tree-shaking.import_namespace/expect.js 删除了包含断言的文件,用于验证 JavaScript 模块的内容。
e2e/fixtures/tree-shaking.import_namespace/mako.config.json 删除了与优化相关的配置文件。
e2e/fixtures/tree-shaking.import_namespace/src/index.tsx 删除了包含控制台日志的文件,演示了导入函数的用法。
e2e/fixtures/tree-shaking.import_namespace/src/mod.js 删除了 shouldKeep1shouldKeep2shouldNotKeep 三个函数。
packages/mako/binding.d.ts 新增了 TransformOutput 接口,包含 code 和可选的 map 属性。

Possibly related PRs

Poem

在树下兔子跳,
代码简化真妙,
模块不再烦恼,
新接口来报到。
兔子乐开怀,
祝贺更改真精彩! 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Tip

Early access features: enabled

We are currently testing new code review model(s) that may lead to higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (1)
packages/mako/binding.d.ts (1)

6-9: 新接口 TransformOutput 看起来不错

这个新的接口定义清晰,结构合理。它似乎与代码转换输出有关,可能用于处理源映射。

建议:

  1. 考虑为这个接口添加一些注释,解释它的用途和属性的含义,特别是 map 属性的可选性。
  2. 确保在使用这个接口的地方更新相关文档。
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 457f3ed and 8a65b9f.

Files selected for processing (10)
  • .github/workflows/ci.yml (0 hunks)
  • crates/mako/src/plugins/tree_shaking.rs (0 hunks)
  • crates/mako/src/plugins/tree_shaking/collect_explicit_prop.rs (0 hunks)
  • crates/mako/src/plugins/tree_shaking/remove_useless_stmts.rs (2 hunks)
  • crates/mako/src/plugins/tree_shaking/shake.rs (1 hunks)
  • e2e/fixtures/tree-shaking.import_namespace/expect.js (0 hunks)
  • e2e/fixtures/tree-shaking.import_namespace/mako.config.json (0 hunks)
  • e2e/fixtures/tree-shaking.import_namespace/src/index.tsx (0 hunks)
  • e2e/fixtures/tree-shaking.import_namespace/src/mod.js (0 hunks)
  • packages/mako/binding.d.ts (1 hunks)
Files not reviewed due to no reviewable changes (7)
  • .github/workflows/ci.yml
  • crates/mako/src/plugins/tree_shaking.rs
  • crates/mako/src/plugins/tree_shaking/collect_explicit_prop.rs
  • e2e/fixtures/tree-shaking.import_namespace/expect.js
  • e2e/fixtures/tree-shaking.import_namespace/mako.config.json
  • e2e/fixtures/tree-shaking.import_namespace/src/index.tsx
  • e2e/fixtures/tree-shaking.import_namespace/src/mod.js
Additional comments not posted (6)
crates/mako/src/plugins/tree_shaking/remove_useless_stmts.rs (6)

Line range hint 1-11: 导入语句的清理很好

移除未使用的导入有助于提高代码的清晰度和可维护性。剩余的导入看起来都是必要的,这是一个很好的改进。


Line range hint 13-108: remove_useless_stmts 函数的修改是合理的

函数的主要逻辑保持不变,移除了 optimize_import_namespace 函数的调用,这与整体的重构方向一致。函数仍然能够有效地处理无用语句和说明符的移除。


Line range hint 110-159: 没有变化,代码保持原样

这部分代码没有进行任何修改,保持原样是合适的。


Line range hint 161-233: 没有变化,代码保持原样

这部分代码没有进行任何修改,保持原样是合适的。


Line range hint 235-308: 建议检查并更新测试套件

虽然现有的测试没有变化,但由于移除了 optimize_import_namespace 函数,可能需要相应地调整测试套件。建议检查是否有与被移除功能相关的测试需要删除或修改,以及是否需要为当前实现添加新的测试用例。


Line range hint 1-308: 总体修改看起来合理,但需要验证功能完整性

移除 optimize_import_namespace 函数及其相关导入是这次更改的主要内容。这表明处理导入命名空间的方法发生了变化。虽然修改看起来是合理的,但建议验证这个改动是否会对树摇(tree-shaking)插件的整体功能产生负面影响。

请确保:

  1. 所有之前由 optimize_import_namespace 处理的场景现在都能被正确处理。
  2. 这个改动不会导致未使用的导入被错误地保留在代码中。
  3. 树摇功能在各种复杂的导入场景下仍然能够正常工作。

为了验证这些变更的影响,建议运行以下脚本:

这个脚本将帮助我们确认测试覆盖率是否充分,是否有遗漏的相关代码,以及集成测试是否需要更新。

packages/mako/binding.d.ts Show resolved Hide resolved
crates/mako/src/plugins/tree_shaking/shake.rs Show resolved Hide resolved
Copy link

codecov bot commented Sep 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.93%. Comparing base (457f3ed) to head (8a65b9f).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1606      +/-   ##
==========================================
- Coverage   62.11%   61.93%   -0.19%     
==========================================
  Files         129      128       -1     
  Lines       15561    15405     -156     
==========================================
- Hits         9666     9541     -125     
+ Misses       5895     5864      -31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stormslowly stormslowly merged commit a485358 into master Sep 23, 2024
20 of 21 checks passed
@stormslowly stormslowly deleted the bug/revert_import_namespace branch September 23, 2024 01:04
Jinbao1001 added a commit that referenced this pull request Sep 24, 2024
Jinbao1001 added a commit that referenced this pull request Oct 12, 2024
* Revert "revert:  import namespace optimize  (#1606)"

This reverts commit a485358.

* fix: hash not stable

* fix: deps sort not stable

* fix: deps sort not stable

* fix: deps sort not stable

* fix: delete binding

* fix: var name

* chore: iter to into

* fix: format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant