-
Notifications
You must be signed in to change notification settings - Fork 77
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: add enforce plugin hook #1646
Conversation
Walkthrough此次更改在多个文件中引入了新的可选字段 Changes
Possibly related PRs
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? 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1646 +/- ##
==========================================
- Coverage 55.57% 55.56% -0.01%
==========================================
Files 172 172
Lines 17415 17418 +3
==========================================
Hits 9678 9678
- Misses 7737 7740 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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 (16)
crates/binding/src/js_plugin.rs (1)
37-38
: 方法实现更新合理,建议小优化
name
方法的更新允许自定义插件名称,同时保留了默认值,这是一个很好的改进。使用as_deref()
和unwrap_or()
是处理Option<String>
的简洁而惯用的方式。建议考虑使用
Option::as_ref()
代替as_deref()
,这样可以避免不必要的解引用:self.name.as_ref().map(|s| s.as_str()).unwrap_or("js_plugin")这个小改动可能会在某些情况下略微提高性能。
crates/binding/src/js_hook.rs (1)
11-11
: 新字段enforce
已添加,但需要更多上下文新增的
enforce
字段看起来是一个有用的扩展。然而,有几点需要注意:
这个新字段的目的和用途并不明确。建议为其添加文档注释,解释其用途和预期使用方式。
考虑是否需要更新其他方法或结构体以利用这个新字段。例如,
TsFnHooks::new
方法目前没有使用enforce
字段。您能否提供更多关于
enforce
字段的信息,以及它将如何在更广泛的系统中使用?packages/mako/binding.d.ts (2)
8-8
: 新增的enforce
属性看起来不错
enforce
属性的添加增强了插件系统的灵活性。这个可选的字符串类型属性可能用于控制插件的执行顺序。建议在相关文档中详细说明
enforce
属性的用法和可能的值,以帮助开发者正确使用这个新特性。
56-56
: 新增的buildEnd
方法是个很好的补充
buildEnd
方法的添加为插件提供了在构建结束时执行异步操作的能力。这个可选的异步方法增强了插件系统的功能性。建议在文档中详细说明
buildEnd
方法的执行时机和用途,以及如何在插件中正确实现这个方法。crates/mako/src/plugin.rs (2)
53-55
: 新增enforce
方法需要更多上下文和文档这个新增的
enforce
方法看起来为插件提供了一种可选的强制机制。然而,其目的和用法并不明确。建议:
- 请提供更多关于这个方法的上下文信息,解释它的用途和预期使用场景。
- 为这个方法添加文档注释,说明其功能、返回值的含义,以及如何正确实现它。
- 考虑在某个具体的插件实现中提供一个示例,展示如何有效地使用这个方法。
53-55
:enforce
方法的实际应用需要进一步考虑新增的
enforce
方法目前在PluginDriver
结构体或文件的其他部分中没有被使用。这可能表明该功能尚未完全实现。建议:
- 考虑如何在
PluginDriver
中使用这个新方法。例如,可能需要在插件初始化或执行过程中检查enforce
的返回值。- 如果这是一个预留的接口,请在代码注释中说明未来的使用计划。
- 确保在实现这个功能时,所有相关的代码都得到相应的更新,以充分利用这个新增的方法。
docs/config.zh-CN.md (5)
Line range hint
332-420
: 新增的实验性配置选项增强了 Mako 的功能这些新增的实验性配置选项(detectLoop、requireContext、webpackSyntaxValidate 和 magicComment)为 Mako 提供了更多高级功能和调试能力。这些添加是有价值的,特别是:
detectLoop
可以帮助开发者识别和可视化依赖循环。webpackSyntaxValidate
提高了与 webpack 特定语法的兼容性。magicComment
支持类似 webpack 的魔法注释,增加了灵活性。这些新功能对于高级用户和复杂项目来说非常有用。
建议在每个实验性配置的描述中添加一个简短的使用场景说明,以帮助用户更好地理解何时应该使用这些功能。例如,对于
detectLoop
,可以添加:"适用于需要优化依赖结构或排查循环依赖问题的项目"。
562-562
: 插件配置中新增enforce
属性增强了灵活性在插件配置中新增的
enforce
属性是一个很好的改进,它允许用户更精确地控制插件的执行顺序。这种方式类似于 webpack 的插件系统,可以提高从 webpack 迁移过来的用户的熟悉度。建议在文档中添加更详细的说明,解释 "pre" 和 "post" 如何影响插件的执行顺序。例如:
- "pre":在主要编译过程之前执行的插件
- "post":在主要编译过程之后执行的插件
这将帮助用户更好地理解如何利用这个新特性来优化他们的构建流程。
Line range hint
729-729
: px2rem 配置新增mediaQuery
选项提供更多控制在 px2rem 配置中新增的
mediaQuery
选项是一个有用的添加,它允许用户控制是否在媒体查询中转换 px 单位。将默认值设置为false
保持了向后兼容性,这是一个明智的选择。为了使文档更加完整和易于理解,建议添加一个使用
mediaQuery
选项的示例。例如:{ "px2rem": { "rootValue": 16, "mediaQuery": true } }同时,可以简要解释启用此选项的潜在用例,如响应式设计中需要将媒体查询中的 px 值也转换为 rem 的情况。
Line range hint
810-824
: Sass 配置更新提供了更好的类型支持和文档Sass 配置的更新是一个很好的改进:
- 使用
Options<'async'>
类型表明 Mako 现在支持异步 Sass 编译,这可能会提高构建性能。- 添加了 Sass 官方文档的链接,方便用户查阅完整的配置选项。
- 提供了一个简单的配置示例,有助于用户快速上手。
- 关于安装 Sass 包的提示非常有用,可以避免用户在使用此功能时遇到问题。
为了进一步提高文档的实用性,建议添加一个更复杂的配置示例,展示如何设置多个 Sass 选项。例如:
{ "sass": { "sourceMap": true, "outputStyle": "compressed", "includePaths": ["./src/styles"] } }这将帮助用户了解如何组合多个选项来满足更复杂的需求。
Line range hint
1-924
: 配置文档整体结构清晰,内容全面这份 Mako 配置文档整体上做得非常好:
- 结构清晰,每个配置选项都有明确的标题。
- 详细说明了每个选项的类型、默认值,并经常提供示例。
- 涵盖了广泛的配置选项,从基本设置到高级功能。
这份文档对于不同经验水平的用户都应该很有帮助。
为了进一步提高文档质量,建议考虑以下改进:
- 在文档开头添加一个目录,方便用户快速导航到特定的配置选项。
- 对于一些复杂的配置选项(如
codeSplitting
、optimization
),可以添加更多实际使用场景的示例。- 考虑添加一个"最佳实践"部分,提供一些常见配置组合的建议。
- 对于实验性功能,可以添加一个警告,提醒用户这些功能可能在未来版本中发生变化。
这些改进将使文档更加用户友好,并帮助用户更好地理解和利用 Mako 的各种功能。
docs/config.md (5)
564-564
: 新增enforce
属性的文档说明很好为
plugins
配置添加enforce
属性的文档说明非常有用。这个新特性允许用户更精确地控制插件的执行顺序,这在某些构建场景中可能非常重要。建议在文档中添加一个简短的示例,展示如何使用
enforce
属性,以帮助用户更好地理解其用法。例如:{ "plugins": [ { "name": "my-pre-plugin", "enforce": "pre" }, { "name": "my-post-plugin", "enforce": "post" } ] }
Line range hint
1010-1037
: 关于resolve
配置的补充说明非常有价值新增的两条注意事项对于正确配置
resolve.alias
非常重要,可以帮助用户避免常见的错误。为了提高可读性,建议将这两条注意事项格式化为一个列表。例如:
注意: 1. 如果要为目录创建别名,请不要添加 `/*` 后缀,我们目前不支持这种方式。 2. 如果要为本地路径创建别名,请确保添加 `./` 前缀。否则,它将被视为依赖模块。 示例:这样的格式可以使文档更加清晰和易于阅读。
Line range hint
1039-1044
: 新增rscClient
配置选项很有用添加
rscClient
配置选项为 React Server Components (RSC) 的客户端行为提供了更多控制。logServerComponent
属性允许用户选择如何处理服务器组件的日志,这在开发和调试过程中可能非常有用。建议为这个新的配置选项添加更多上下文信息和使用示例。例如:
### rscClient - 类型:`{ logServerComponent: 'error' | 'ignore' } | false` - 默认值:`false` RSC 客户端相关配置。 - `logServerComponent`:控制如何处理服务器组件的日志。 - `'error'`:将服务器组件的日志作为错误处理。 - `'ignore'`:忽略服务器组件的日志。 示例: ```json { "rscClient": { "logServerComponent": "error" } }这将帮助用户更好地理解这个配置选项的用途和如何使用它。
Line range hint
1046-1055
: 新增rscServer
配置选项提供了重要的 RSC 服务器端控制
rscServer
配置选项的添加为 React Server Components (RSC) 的服务器端行为提供了重要的控制。这包括客户端组件模板的配置和 CSS 输出的控制,这些在 RSC 架构中都是关键的方面。为了使文档更加完整和易于理解,建议进行以下改进:
- 为
clientComponentTpl
提供一个具体的示例,展示如何使用{{path}}
和{{id}}
占位符。- 解释
emitCSS
选项的具体作用和影响。- 添加一个完整的配置示例。
例如:
### rscServer - 类型:`{ clientComponentTpl: string, emitCSS: boolean } | false` - 默认值:`false` RSC 服务器相关配置。 子配置项: - `clientComponentTpl`:客户端组件模板。使用 `{{path}}` 表示组件的路径,使用 `{{id}}` 表示模块的 ID。 - `emitCSS`:是否输出 CSS 组件。当设置为 `true` 时,服务器将生成独立的 CSS 文件。 示例: ```json { "rscServer": { "clientComponentTpl": "loadComponent('{{path}}', '{{id}}')", "emitCSS": true } }在这个示例中,
clientComponentTpl
定义了如何加载客户端组件,而emitCSS
设置为true
表示将生成独立的 CSS 文件。这些改进将帮助用户更好地理解和使用 `rscServer` 配置选项。 --- Line range hint `1057-1065`: **`stats` 配置的更新提供了更细粒度的控制** 在 `stats` 配置中添加 `modules` 选项是一个很好的改进。这允许用户更精确地控制生成的统计信息,特别是在需要分析模块大小但又担心影响构建速度的情况下非常有用。 为了使文档更加清晰,建议添加一个配置示例和更详细的说明。例如: ```markdown ### stats - 类型:`{ modules: boolean } | false` - 默认值:`false` 是否生成 stats.json 文件。 子配置项: - `modules`:是否生成模块信息。当需要分析模块大小时很有用,但可能会降低构建速度。 示例: ```json { "stats": { "modules": true } }
注意:启用
modules
选项可能会显著增加 stats.json 文件的大小和生成时间。只有在需要详细分析模块结构和大小时才建议启用此选项。这个额外的信息和示例将帮助用户更好地理解 `stats` 配置的用途和潜在影响。 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Files that changed from the base of the PR and between 60c2724a8b006840223f2a40e2ee01c8649b58bd and d9522945e493e86a9aafbea891cd2e1e199fb4d5. </details> <details> <summary>📒 Files selected for processing (9)</summary> * crates/binding/src/js_hook.rs (1 hunks) * crates/binding/src/js_plugin.rs (1 hunks) * crates/binding/src/lib.rs (2 hunks) * crates/mako/src/plugin.rs (1 hunks) * docs/config.md (1 hunks) * docs/config.zh-CN.md (1 hunks) * e2e/fixtures/plugins/expect.js (1 hunks) * e2e/fixtures/plugins/plugins.config.js (1 hunks) * packages/mako/binding.d.ts (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> </details> <details> <summary>🔇 Additional comments (7)</summary><blockquote> <details> <summary>e2e/fixtures/plugins/expect.js (1)</summary><blockquote> `17-17`: **变更看起来是正确的,但请确认整个代码库的一致性。** 这个断言的修改反映了转换钩子的预期行为发生了变化。这个更改看起来是有意为之的,并且与文件的整体目的(测试各种钩子和转换的正确执行)保持一致。 请运行以下脚本来验证这个变更是否与代码库的其他部分保持一致: 如果第一个搜索没有结果,而第二个搜索在适当的地方找到了结果,那么这个变更就是一致的。 <details> <summary>✅ Verification successful</summary> <!-- [monologue] [task verify_change_1/1] [observations] - The shell script executed two `rg` searches: one for the old transform hook name `transform_2_1` and one for the new name `transform_1_2`. - The search for `transform_2_1` returned no results, indicating that the old hook name is not used elsewhere in the codebase. - The search for `transform_1_2` returned one result in `e2e/fixtures/plugins/expect.js`, which aligns with the reviewed change. [analysis] - The absence of `transform_2_1` suggests that the old hook name has been fully replaced. - The presence of `transform_1_2` in the expected location confirms that the new hook name is correctly implemented. - No further occurrences of the old hook name imply that the change is consistent and does not introduce discrepancies. [actions] - Conclude that the change is verified and consistent with the rest of the codebase. - Tag the response appropriately as . --> **验证通过,未发现其他引用旧的转换钩子名称的地方。** 这个变更在代码库中是一致的,没有发现其他地方使用旧的转换钩子名称。 </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # 描述:检查是否有其他地方引用了旧的转换钩子名称 # 测试:搜索可能引用旧转换钩子名称的地方 rg --type js --type ts 'transform_2_1' # 测试:搜索新的转换钩子名称,以确保它在其他地方被正确使用 rg --type js --type ts 'transform_1_2'
Length of output: 191
e2e/fixtures/plugins/plugins.config.js (1)
Line range hint
50-60
: 新增的插件配置需要进一步说明这个新增的插件配置引入了
enforce: "pre"
属性,这可能会影响插件的执行顺序。以下是一些观察和建议:
- 新配置与现有的 transform 插件非常相似,但替换 "transform" 为 "transform_2" 而不是 "transform_1"。
enforce: "pre"
属性表明这个转换应该在其他转换之前执行。- 两个相似的 transform 插件可能会导致混淆,特别是在调试时。
建议:
- 考虑添加注释来解释为什么需要两个相似的 transform 插件,以及
enforce: "pre"
的具体用途。- 确保这种执行顺序不会对其他插件或整体构建过程产生意外影响。
- 如果可能,考虑将这两个相似的插件合并为一个更灵活的插件。
为了验证这个改动的影响,我们可以运行以下脚本:
这将帮助我们了解这个改动在更大的上下文中的影响。
✅ Verification successful
验证通过
enforce: "pre"
属性仅在e2e/fixtures/plugins/plugins.config.js
中使用,未发现对其他插件或构建过程产生影响。建议在配置中添加注释,以说明该属性的用途和必要性。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查是否有其他插件使用了 enforce 属性,以及是否有其他地方引用了这个配置文件 # 测试:搜索其他使用 enforce 属性的插件 echo "Searching for other plugins using the enforce property:" rg --type js 'enforce:' -C 5 # 测试:查找引用这个配置文件的地方 echo "Searching for references to this configuration file:" rg 'plugins\.config\.js' -C 5Length of output: 1467
crates/binding/src/js_plugin.rs (1)
31-32
: 结构体字段添加得当新增的
name
和enforce
字段增强了JsPlugin
结构体的灵活性。使用Option<String>
类型使这些字段成为可选项,这是一个很好的做法。字段命名清晰且具有描述性,有助于代码的可读性和维护性。
packages/mako/binding.d.ts (2)
11-11
:loadInclude
方法返回类型的更新是个好的改进将
loadInclude
方法的返回类型从bool
更新为boolean
提高了类型的一致性和标准化。这是一个小但重要的改进,有助于提高代码质量。
Line range hint
65-65
:transformInclude
方法返回类型的更新同样是个好的改进将
transformInclude
方法的返回类型从bool
更新为boolean
也提高了类型的一致性和标准化。这个改变与loadInclude
的更新一致,有助于维护整个接口的类型一致性。
crates/binding/src/lib.rs (2)
3-3
: 导入 Ordering 符合需求引入
std::cmp::Ordering
为插件排序提供了必要的比较功能。
188-192
: JsPlugin 结构体更新正确
JsPlugin
结构体新增了name
和enforce
字段,并正确地从hooks
中克隆这些值,符合功能扩展的需求。
part of #1238
Summary by CodeRabbit
新功能
JsHooks
和JsPlugin
结构中添加了新的可选字段enforce
和name
,增强了插件的灵活性。enforce
方法到Plugin
接口,允许实现类型指定强制机制。enforce
属性,支持插件作为 "pre" 或 "post" 钩子。文档