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

feat: disable webp to base64 #1602

Merged
merged 8 commits into from
Sep 25, 2024
Merged

feat: disable webp to base64 #1602

merged 8 commits into from
Sep 25, 2024

Conversation

Jinbao1001
Copy link
Member

@Jinbao1001 Jinbao1001 commented Sep 19, 2024

Summary by CodeRabbit

  • 新功能
    • 引入了新的配置选项,允许用户指定要排除的文件类型,以优化内联资源的处理。
    • 增加了对正则表达式的支持,以更灵活地管理内联资产。
  • 文档
    • 更新了配置文档,添加了关于内联限制和排除文件类型的新选项,包括示例说明。
    • 更新了中文文档以反映新增的配置选项。
  • 测试
    • 新增测试文件以验证构建过程中生成的特定文件。

Copy link
Contributor

coderabbitai bot commented Sep 19, 2024

Warning

Rate limit exceeded

@Jinbao1001 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 43 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 4a79ee0 and 9561ede.

Walkthrough

此拉取请求引入了一系列更改,主要集中在资产管理和内联处理上。通过在配置中添加正则表达式排除选项,用户可以灵活地控制哪些文件类型在处理时被排除。此外,更新了配置文档以反映这些更改,并新增了相关的测试文件。整体上,这些更改增强了对内联资源的配置能力。

Changes

文件路径 更改摘要
crates/mako/src/config/config.rs Config结构中添加了linline_excludes_regexes字段,更新了默认配置。
docs/config.md 添加了inlineLimitinlineExcludesRegexes配置选项,增强了资产处理的可配置性。
docs/config.zh-CN.md 更新了中文文档,添加了inlineExcludesRegexes配置选项。
e2e/fixtures/config.inline_excludes_regexes/mako.config.json 新增配置文件,定义内联排除特定文件类型的设置。
e2e/fixtures/config.inline_excludes_regexes/src/index.tsx 新增TypeScript文件,导入并记录图像资产。

Possibly related PRs

Suggested reviewers

  • stormslowly
  • sorrycc

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.

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: 3

Outside diff range and nitpick comments (2)
crates/mako/src/utils.rs (1)

54-57: 考虑优化错误处理

函数实现看起来不错!使用 cached 宏来缓存编译后的正则表达式可以提高性能。

但是,使用 unwrap() 方法假设正则表达式模式总是有效的,这可能会导致程序 panic。建议使用 Regex::new(re).map_err(...) 来更优雅地处理无效的正则表达式模式,例如:

#[cached(key = "String", convert = r#"{ re.to_string() }"#)]
pub fn create_cached_regex(re: &str) -> Result<Regex> {
    Regex::new(re).map_err(|e| anyhow!("Invalid regex pattern: {}", e))
}

这样,如果正则表达式模式无效,函数会返回一个 Result::Err,而不是直接 panic。

crates/mako/src/config/config.rs (1)

528-528: 为新字段点赞!建议添加注释说明该字段的作用。

添加 linline_excludes_regexes 字段,允许用户灵活配置需要排除的内联元素,提高了内联处理的可定制性。建议添加注释,清晰说明该字段的作用,以便其他开发人员快速理解。

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

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

Files selected for processing (9)
  • crates/mako/src/build/load.rs (3 hunks)
  • crates/mako/src/config/config.rs (2 hunks)
  • crates/mako/src/resolve.rs (6 hunks)
  • crates/mako/src/utils.rs (2 hunks)
  • docs/config.md (1 hunks)
  • docs/config.zh-CN.md (1 hunks)
  • e2e/fixtures/config.inline_excludes_regexes/expect.js (1 hunks)
  • e2e/fixtures/config.inline_excludes_regexes/mako.config.json (1 hunks)
  • e2e/fixtures/config.inline_excludes_regexes/src/index.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
  • e2e/fixtures/config.inline_excludes_regexes/src/index.tsx
Additional context used
LanguageTool
docs/config.md

[grammar] ~406-~406: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...xcludes assets file extension list that don't needs to be converted to base64 forma...

(SINGULAR_NOUN_AGREEMENT_WHO_DONT)


[grammar] ~406-~406: Did you mean “need”?
Context: ...s assets file extension list that don't needs to be converted to base64 format. e....

(DONT_NEEDS)

Additional comments not posted (14)
e2e/fixtures/config.inline_excludes_regexes/mako.config.json (1)

3-3: 属性设置合理。

inlineLimit 属性为内联内容设置了一个合理的最大大小阈值。1MB 的限制在提供灵活性的同时,也能防止内联过大的内容。

e2e/fixtures/config.inline_excludes_regexes/expect.js (4)

1-3: 代码看起来不错!

引入必要的模块和函数,并正确地解析构建结果以提取文件对象,为后续的断言做好准备。


5-5: 代码看起来很好!

将文件名提取为一个数组,并连接成一个字符串,以便于后续使用正则表达式进行匹配。这是一种常见且有效的方法。


7-8: 很好的注释!

注释清晰地说明了后续断言的目的,有助于提高代码的可读性和可维护性。保持这种注释习惯是一个好主意。


8-8: 断言逻辑正确!

使用 assert.match 检查文件名字符串是否匹配特定的正则表达式模式,以验证构建输出中包含预期的 WebP 图像文件。正则表达式模式构造正确,能够匹配所需的文件名格式。断言的描述参数清晰地说明了预期行为,有助于理解测试失败的原因。

crates/mako/src/build/load.rs (3)

7-7: LGTM!

引入 regex 包以支持正则表达式的使用。


18-18: 请确认 create_cached_regex 函数的实现。

utils 模块中导入了 create_cached_regex 函数,该函数可能用于创建和缓存正则表达式以优化性能。但是,在提供的上下文中没有找到该函数的实际实现。

请提供 create_cached_regex 函数的实现,以便我可以验证其正确性和性能影响。


262-275: 代码逻辑正确,增强了资产管理的灵活性。

这部分代码引入了一种新机制,使用正则表达式来处理排除项。它从配置的 linline_excludes_regexes 创建了一个编译后的正则表达式模式列表,用于检查文件扩展名是否与任何指定的模式匹配。如果找到匹配项,即使资产满足内联的大小标准,也不会将其转换为 base64 字符串。控制流程进行了调整,以结合这个新的检查。

这些更改通过允许某些文件类型根据用户定义的正则表达式模式绕过内联处理,增加了资产管理的灵活性。现在,在决定是否发出资产时,同时考虑了文件大小和正则表达式匹配。

代码使用 create_cached_regex 函数来编译和缓存正则表达式模式,以优化性能。总的来说,这些更改在逻辑上是正确的,并改进了功能。

docs/config.md (1)

395-398: 配置项看起来不错!

新增的 inlineLimit 配置项文档说明清晰,类型 string[] 也很合适,空数组作为默认值没问题。

crates/mako/src/resolve.rs (4)

9-9: LGTM!

导入 regex crate 的 Captures 结构体是合理的,可能在代码中用于捕获正则表达式匹配。


24-24: LGTM!

crate::utils 模块导入 create_cached_regex 函数是合理的。根据函数名,它可能创建一个带缓存的正则表达式对象以提高性能。AI 生成的摘要提到,此实用程序函数替换了先前使用的 create_external_regex 函数。


Line range hint 111-124: LGTM!

代码段正确地使用 create_cached_regex 函数创建用于匹配子路径的正则表达式对象。它检查子路径是否与外部配置中定义的任何排除模式匹配。正则表达式模式是使用 key 变量动态构造的。排除检查是通过迭代排除模式并与子路径匹配来正确执行的。


Line range hint 144-160: LGTM!

代码段正确地使用 create_cached_regex 函数创建正则表达式对象,用于将子路径与定义的规则进行匹配。它使用 replace_all 方法替换目标模板中捕获的组。正则表达式模式直接从 rule 对象中使用。使用 replace_all 方法和 caps 变量在目标模板中正确替换捕获的组。

crates/mako/src/config/config.rs (1)

741-741: 赞同在默认配置中初始化新字段。

DEFAULT_CONFIG 常量中为 linline_excludes_regexes 提供了一个空数组作为默认值,与该字段的类型 Vec<String> 相匹配。这样可以确保在用户未提供自定义配置的情况下,该字段具有合理的默认行为。

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: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c4d442b and 4a79ee0.

Files selected for processing (4)
  • crates/mako/src/config/config.rs (2 hunks)
  • docs/config.md (1 hunks)
  • docs/config.zh-CN.md (1 hunks)
  • e2e/fixtures/config.inline_excludes_regexes/mako.config.json (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • crates/mako/src/config/config.rs
  • e2e/fixtures/config.inline_excludes_regexes/mako.config.json
Additional context used
LanguageTool
docs/config.md

[grammar] ~406-~406: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...xcludes assets file extension list that don't needs to be converted to base64 forma...

(SINGULAR_NOUN_AGREEMENT_WHO_DONT)


[grammar] ~406-~406: Did you mean “need”?
Context: ...s assets file extension list that don't needs to be converted to base64 format. e....

(DONT_NEEDS)

Additional comments not posted (2)
docs/config.zh-CN.md (1)

401-416: 新的配置选项看起来不错!

添加 inlineExcludesRegexes 配置为控制哪些文件类型不被转换为 base64 格式提供了灵活性。空数组作为默认值是合适的。

示例清晰地展示了如何使用此配置排除 webp 文件。

docs/config.md (1)

395-414: 先前的评审意见仍然适用。

之前的评审意见指出了 inlineExcludesRegexes 的命名与其类型定义和示例值之间的不一致:

  • 名称 inlineExcludesRegexes 暗示它接受正则表达式,但类型定义为 string[],示例值 ["webp"] 也是一个字符串数组。
  • 建议重命名为 inlineExcludesExtensions,以更好地与指定文件扩展名的实际功能相匹配。

这个不一致性在当前的代码更改中仍然存在,先前的评论依然有效。建议按照先前的评论进行重命名,以消除歧义。

Tools
LanguageTool

[grammar] ~406-~406: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...xcludes assets file extension list that don't needs to be converted to base64 forma...

(SINGULAR_NOUN_AGREEMENT_WHO_DONT)


[grammar] ~406-~406: Did you mean “need”?
Context: ...s assets file extension list that don't needs to be converted to base64 format. e....

(DONT_NEEDS)

@umijs umijs deleted a comment from coderabbitai bot Sep 19, 2024
@umijs umijs deleted a comment from coderabbitai bot Sep 19, 2024
@umijs umijs deleted a comment from coderabbitai bot Sep 19, 2024
@umijs umijs deleted a comment from coderabbitai bot Sep 19, 2024
@umijs umijs deleted a comment from coderabbitai bot Sep 19, 2024
@umijs umijs deleted a comment from coderabbitai bot Sep 19, 2024
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 62.13%. Comparing base (457f3ed) to head (9561ede).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
crates/mako/src/build/load.rs 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1602      +/-   ##
==========================================
+ Coverage   62.11%   62.13%   +0.01%     
==========================================
  Files         129      129              
  Lines       15561    15571      +10     
==========================================
+ Hits         9666     9675       +9     
- Misses       5895     5896       +1     

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

@xusd320 xusd320 merged commit 9a4c4d8 into master Sep 25, 2024
21 checks passed
@xusd320 xusd320 deleted the fix/disable-webp-to-base64 branch September 25, 2024 07:42
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.

2 participants