feat: add transform plugin hook #1637
Conversation
Walkthrough此拉取请求的更改引入了 Changes
Possibly related PRs
Suggested reviewers
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? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (12)
e2e/fixtures/plugins/src/index.tsx (1)
7-9: 代码更改看起来不错！新添加的代码符合预期，引入了transform模块并进行了日志记录。这与PR的目标一致。
为了提高代码的可读性，建议在新添加的部分前后各添加一个空行，以便更好地分隔不同的代码块。例如：
console.log(require('resolve_id_external')); + // transform console.log(require('./transform')); +这样可以使代码结构更清晰，便于阅读和维护。
e2e/fixtures/plugins/expect.js (1)
15-17: 新增的断言看起来不错！新添加的断言正确地测试了转换钩子的功能。它与文件中现有的断言风格保持一致，并且逻辑清晰。
为了提高可读性，建议在新增的断言之前添加一个空行，以便更好地分隔不同的测试组。您可以应用以下更改：
assert(content.includes(`module.exports = resolve_id_external;`), `resolve_id hook with external works`); + // transform hook assert(content.includes(`console.log('transform_2_1');`), `transform hook works`);e2e/fixtures/plugins/plugins.config.js (1)
38-48: 新增 transform 方法这个新增的 transform 方法为特定文件提供了转换功能，这是一个很好的扩展。几点建议：
- 考虑使用更具描述性的变量名，如
originalCode替代code。- 日志输出在生产环境中可能会产生大量无用信息，建议添加一个调试模式开关。
- 可以考虑使用正则表达式来进行更复杂的内容替换。
建议对代码进行如下优化：
async transform(originalCode, id) { if (id.endsWith("transform.ts")) { if (process.env.DEBUG) { console.log("transform", originalCode, id); } return { content: originalCode.replace(/\btransform\b/g, "transform_1"), type: "ts", }; } }这样可以提高代码的可读性和灵活性。
packages/mako/binding.d.ts (2)
60-63: LGTM！建议对类型定义进行微小改进。
transform方法的添加看起来很好，符合接口的现有模式。它为转换操作提供了足够的灵活性。为了提高可读性和类型安全性，建议将输入和输出类型提取为单独的接口。例如：
interface TransformInput { content: string; type: 'css' | 'js'; } interface TransformOutput { content: string; type: 'css' | 'js'; } transform?: ( content: TransformInput, path: string, ) => Promise<TransformOutput | void> | void;这样可以使类型更加明确，并且在将来需要修改输入或输出结构时更容易维护。
80-83: 新接口看起来不错，但可以考虑进行一些改进。
LoadTransformResult接口的添加是好的，结构简单明了。然而，有几点建议可以提高代码的一致性和清晰度：
考虑将
type属性限制为 'css' | 'js'，以与transform方法保持一致。可以添加注释解释这个接口的用途，特别是它与
transform方法的关系。考虑重命名接口，例如
TransformResult，以更好地反映其用途（如果它确实是与transform方法相关的）。建议的修改如下：
/** * 表示转换操作的结果。 * 这个接口与 JsHooks 中的 transform 方法相关。 */ export interface TransformResult { content: string; type: 'css' | 'js'; }这些改变将提高代码的一致性和可读性。
docs/config.zh-CN.md (6)
575-575: 新增配置项文档清晰明了新增的配置项如
experimental.magicComment、rscClient和rscServer等，文档说明详细且易于理解。这些新功能将为开发者提供更多的灵活性和控制力。建议：考虑在文档的开头添加一个变更日志或新特性概述，以便用户快速了解最新的更新。
575-575: 插件系统得到增强，建议添加使用示例新增的
transform钩子函数为插件系统提供了更强大的内容转换能力，这是一个很好的改进。建议：为了帮助开发者更好地理解和使用这个新特性，可以考虑添加一个简单的使用示例，展示如何在插件中实现
transform钩子函数。例如：{ plugins: [ { name: 'my-transform-plugin', transform: async (content, id) => { // 示例：将所有的 'foo' 替换为 'bar' const transformedContent = content.replace(/foo/g, 'bar'); return { content: transformedContent, type: 'js' }; } } ] }这样的示例将有助于开发者快速掌握新功能的使用方法。
575-575: Sass配置文档更新，建议提供更多选项说明文档中新增了关于安装
sass包的提示，这对用户很有帮助。同时，配置类型更新为Options<'async'>提供了更清晰的类型定义。建议：考虑添加更多关于可用的sass选项的详细说明。例如，可以列出一些常用的选项及其作用，如：
{ sass: { outputStyle: 'compressed', // 输出压缩的CSS includePaths: ['./node_modules'], // 设置Sass导入的搜索路径 precision: 6 // 设置小数精度 } }这将帮助开发者更全面地了解和使用sass配置。
575-575: publicPath配置说明得到改进，建议添加实际使用示例文档中新增了关于"runtime"特殊值和
__mako_public_path__的说明，这对需要动态配置publicPath的用户来说非常有价值。建议：为了使用户更好地理解这些新特性，可以考虑添加一个实际的使用示例。例如：
// 配置文件中 { publicPath: "runtime" } // 在应用的入口文件中 if (process.env.NODE_ENV === 'production') { __mako_public_path__ = 'https://cdn.example.com/assets/'; } else { __mako_public_path__ = '/dev-assets/'; }这个例子展示了如何根据环境动态设置publicPath，将帮助用户更好地理解和应用这个功能。
575-575: 别名配置说明更加清晰，建议补充解释文档中新增的两个重要说明有效地澄清了别名配置的常见误区，这对于防止配置错误非常有帮助。
建议：为了让用户更好地理解这些建议的原因，可以考虑添加简短的解释。例如：
关于不添加
/*后缀：
"我们不支持添加/*后缀是因为这可能会导致解析冲突和不可预期的行为。直接使用目录路径可以确保更一致的模块解析。"关于添加
./前缀：
"添加./前缀是为了明确指示这是一个本地路径，而不是一个npm包。这有助于防止潜在的命名冲突和错误的模块解析。"这些解释将帮助用户理解配置建议背后的原因，从而更好地遵循最佳实践。
575-575: 文档结构清晰，建议进一步提高一致性整体而言，文档结构良好，各个配置项的说明清晰明了，代码示例也很有帮助。格式的一致性贯穿整个文档，这有助于提高可读性。
建议：为了进一步提高文档质量，可以考虑以下几点：
对于所有配置项，尽可能提供默认值。这将帮助用户更好地理解每个选项的初始状态。
为每个配置项提供至少一个简单的使用示例。虽然大多数选项已经有了示例，但确保所有选项都有示例将使文档更加完整。
考虑添加一个"最佳实践"部分，汇总一些常见的配置组合或使用技巧。这将帮助新用户快速上手，也能为有经验的用户提供优化建议。
这些小改进将使文档更加全面和一致，进一步提升用户体验。
crates/binding/src/js_plugin.rs (1)
126-130: 优化match匹配以处理更多的内容类型。在处理
content时，目前只考虑了JsContent和CssContent。如果未来会有更多类型的内容，建议使用模式匹配来处理，或者为未知类型提供默认行为。可以考虑：
let content_str = match content { Content::Js(js_content) => js_content.content.clone(), Content::Css(css_content) => css_content.clone(), + other => { + eprintln!("Unsupported content variant: {:?}", other); + return Ok(None); + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- crates/binding/src/js_hook.rs (4 hunks)
- crates/binding/src/js_plugin.rs (2 hunks)
- crates/mako/src/build.rs (1 hunks)
- crates/mako/src/plugin.rs (2 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)
- e2e/fixtures/plugins/src/index.tsx (1 hunks)
- e2e/fixtures/plugins/src/transform.ts (1 hunks)
- packages/mako/binding.d.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- e2e/fixtures/plugins/src/transform.ts
🧰 Additional context used
🔇 Additional comments (12)
e2e/fixtures/plugins/plugins.config.js (3)
4-10: 代码风格更新已批准这些更改统一了字符串文字的格式，使用双引号替代了单引号。这提高了代码的一致性和可读性。
14-21: 代码格式统一性得到改善这部分的修改延续了之前的风格更新，将字符串文字从单引号改为双引号。这种一致性有助于提高代码的可维护性。
25-36: 代码结构优化和格式统一resolveId 方法的重构提高了可读性：
- 使用双引号统一了字符串格式。
- 返回对象的结构更清晰。
- 使用模板字符串来调用 path.join 是一个很好的做法。
这些更改使代码更易于理解和维护，同时保持了原有的功能。
crates/mako/src/build.rs (1)
274-279: 🛠️ Refactor suggestion新增的转换步骤可能影响性能和行为
这个新增的转换步骤引入了一些值得注意的变化：
- 内容现在是可变的，这可能会影响后续的操作。
- 新的
load_transform方法允许在进一步处理之前对内容进行额外的处理。- 使用
plugin_driver表明这个转换可以通过插件进行自定义。建议：
- 考虑添加性能测试，以评估这个额外步骤对构建时间的影响。
- 确保在文档中清楚地说明这个新的转换步骤，以及如何通过插件系统来利用它。
- 考虑添加一个配置选项，允许在不需要时跳过这个转换步骤，以优化性能。
为了验证这个变更的影响，我们可以运行以下脚本：
docs/config.md (4)
577-577: 新增了 transform 插件钩子，增强了内容转换能力在
JSHooks接口中添加了新的transform钩子函数。这个钩子允许在构建过程中对内容进行更灵活的转换。使用示例：
{ transform: async (content: string, id: string) => { // 在这里对内容进行转换 const transformedContent = someTransformFunction(content); return { content: transformedContent, type: 'js' }; } }请注意，这个新钩子可能会影响构建管道，使用时需要谨慎。
577-577: px2rem 配置选项得到了显著扩展，提供了更精细的控制
px2rem配置新增了以下选项：
propBlackList：属性黑名单propWhiteList：属性白名单selectorBlackList：选择器黑名单selectorWhiteList：选择器白名单selectorDoubleList：选择器双倍 rem 列表这些新选项允许您更精确地控制 px 到 rem 的转换过程。特别值得注意的是，选择器列表现在支持正则表达式，例如：
"selectorBlackList": [".a", "/.__CustomClass_/"]使用这些新选项时，请确保仔细考虑您的 CSS 结构，以获得最佳的转换结果。
577-577: resolve 配置中的别名使用说明得到了重要更新在
resolve配置的说明中，新增了两个重要提示：
目录别名不支持
/*后缀：{ resolve: { alias: [ - ["@/src/*", "./src/*"], + [ "@/src", "./src"], ], }, }本地路径别名必须添加
./前缀：{ resolve: { alias: [ - ["@/src", "src"], + ["@/src", "./src"], ], }, }这些更新有助于防止常见的别名配置错误。请确保在设置别名时遵循这些新的指导原则，以确保您的别名正确解析。
577-577: watch 配置新增了忽略路径的示例在
watch配置部分，新增了一个有用的示例，展示了如何忽略特定路径：{ watch: { ignorePaths: ["foo"], }, }这个示例演示了如何忽略根目录下的
foo目录。这对于需要在开发过程中排除某些目录不被监视的用户来说非常有用。您可以根据项目需求，轻松地调整ignorePaths数组来排除多个目录或文件。crates/binding/src/js_hook.rs (1)
137-142: 检查TransformResult结构体的字段类型和名称在定义
TransformResult结构体时，确保字段与预期的 TypeScript 类型匹配。当前定义为：pub struct TransformResult { pub content: String, #[napi(js_name = "type")] pub content_type: String, }其中，使用了
#[napi(js_name = "type")]来映射字段名。但要注意，type在 Rust 中是关键字，不能作为字段名。当前的处理是正确的，确保了在导出到 JavaScript 时，字段名为type。请确认这一映射是否符合预期。crates/binding/src/js_plugin.rs (2)
15-16: 确认新添加的属性是否在其他地方正确使用。
JsPlugin结构体中新增了hooks属性，请确保在代码的其他部分正确地初始化和使用了该属性，以避免潜在的None错误或未初始化问题。
132-152: 检查transform钩子的返回值处理。在调用
hook.call后，对返回的result进行了解析。请确保所有可能的返回情况都已被正确处理，尤其是None的情况，以及result.content_type可能为空或无效的情况。运行以下脚本以验证所有可能的返回值情况：
crates/mako/src/plugin.rs (1)
61-68: 添加新方法load_transform到Plugintrait
load_transform方法的定义合理，参数类型和返回值符合预期，为插件提供了内容转换的钩子。
| { | ||
| async transform(code, id) { | ||
| if (id.endsWith("transform.ts")) { | ||
| console.log("transform", code, id); | ||
| return { | ||
| content: code.replace("transform", "transform_2"), | ||
| type: "ts", | ||
| }; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
重复的 transform 方法
这个 transform 方法与前一个几乎相同，只是替换的内容不同。这种重复可能导致维护问题。建议重构以提高代码的可维护性和灵活性。
建议将两个 transform 方法合并为一个可配置的方法：
function createTransform(replacement) {
return async function transform(originalCode, id) {
if (id.endsWith("transform.ts")) {
if (process.env.DEBUG) {
console.log("transform", originalCode, id);
}
return {
content: originalCode.replace(/\btransform\b/g, replacement),
type: "ts",
};
}
};
}
module.exports = [
// ... 其他插件配置 ...
{
transform: createTransform("transform_1"),
},
{
transform: createTransform("transform_2"),
},
];这种方法可以减少代码重复，并使未来添加更多类似的转换更加容易。
| #[napi( | ||
| ts_type = "(content: { content: string, type: 'css' | 'js' }, path: string) => Promise<{ content: string, type: 'css' | 'js' } | void> | void;" | ||
| )] | ||
| pub transform: Option<JsFunction>, |
There was a problem hiding this comment.
建议检查 transform 方法的参数类型匹配
在 JsHooks 中，transform 的 TypeScript 类型定义为：
(content: { content: string, type: 'css' | 'js' }, path: string) => Promise<{ content: string, type: 'css' | 'js' } | void> | void;然而，在 Rust 中，TsFnHooks 的 transform 对应的类型为：
Option<ThreadsafeFunction<(String, String), Option<TransformResult>>>这意味着 Rust 端的参数是 (String, String)，即两个字符串，但根据 TypeScript 类型定义，应该接收一个对象和一个字符串。建议在 Rust 端调整参数类型，使其与 TypeScript 类型定义一致，例如将参数类型修改为 (LoadResult, String)，并相应地修改 ThreadsafeFunction 的泛型参数。
| @@ -74,6 +78,7 @@ | |||
| pub resolve_id: | |||
| Option<ThreadsafeFunction<(String, String, ResolveIdParams), Option<ResolveIdResult>>>, | |||
| pub _on_generate_file: Option<ThreadsafeFunction<WriteFile, ()>>, | |||
| pub transform: Option<ThreadsafeFunction<(String, String), Option<TransformResult>>>, | |||
There was a problem hiding this comment.
调整 transform 的 ThreadsafeFunction 参数类型
如上所述，transform 的 ThreadsafeFunction 参数类型应与 TypeScript 定义保持一致，建议将类型修改为：
Option<ThreadsafeFunction<(LoadResult, String), Option<TransformResult>>>这样可以确保在调用时，参数类型匹配，避免潜在的类型不安全问题。
| transform: hooks.transform.as_ref().map(|hook| unsafe { | ||
| ThreadsafeFunction::from_napi_value(env.raw(), hook.raw()).unwrap() | ||
| }), |
There was a problem hiding this comment.
修改 transform 的初始化以匹配新的参数类型
在 TsFnHooks::new 方法中，初始化 transform 时，需要确保类型匹配。如果调整了 transform 的类型，需要相应地修改初始化代码。例如：
transform: hooks.transform.as_ref().map(|hook| unsafe {
ThreadsafeFunction::from_napi_value(env.raw(), hook.raw()).unwrap()
}),请确保映射的函数类型与新的参数类型一致。
crates/binding/src/js_plugin.rs
Outdated
| use crate::js_hook::{ | ||
| LoadResult, TransformResult, ResolveIdParams, ResolveIdResult, TsFnHooks, WriteFile, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议：导入语句的排序和组织。
在第10-12行，导入的模块列表可能需要重新组织，以提高可读性和维护性。考虑按照模块名称的字母顺序或功能相关性对导入进行排序。
| fn load_transform( | ||
| &self, | ||
| content: &mut Content, | ||
| path: &str, | ||
| _context: &Arc<Context>, | ||
| ) -> Result<Option<Content>> { | ||
| if let Some(hook) = &self.hooks.transform { | ||
| let content_str = match content { | ||
| Content::Js(js_content) => js_content.content.clone(), | ||
| Content::Css(css_content) => css_content.clone(), | ||
| _ => return Ok(None), | ||
| }; | ||
|
|
||
| let result: Option<TransformResult> = hook.call((content_str, path.to_string()))?; | ||
|
|
||
| if let Some(result) = result { | ||
| match result.content_type.as_str() { | ||
| "js" | "ts" => { | ||
| return Ok(Some(Content::Js(JsContent { | ||
| content: result.content, | ||
| is_jsx: false, | ||
| }))) | ||
| } | ||
| "jsx" | "tsx" => { | ||
| return Ok(Some(Content::Js(JsContent { | ||
| content: result.content, | ||
| is_jsx: true, | ||
| }))) | ||
| } | ||
| "css" => return Ok(Some(Content::Css(result.content))), | ||
| _ => return Err(anyhow!("Unsupported content type: {}", result.content_type)), | ||
| } | ||
| } | ||
| } | ||
| Ok(None) | ||
| } |
There was a problem hiding this comment.
新增的load_transform方法功能正确，但需要注意错误处理。
load_transform方法正确地实现了内容转换的逻辑，但在处理未知的content_type时，直接返回了一个错误。建议提供更详细的错误信息，或者考虑默认处理，以提高代码的鲁棒性。
可以应用以下差异来改进错误处理：
_ => return Err(anyhow!("Unsupported content type: {}", result.content_type)),
+ _ => {
+ eprintln!("Warning: Unsupported content type '{}', skipping transformation.", result.content_type);
+ return Ok(None);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn load_transform( | |
| &self, | |
| content: &mut Content, | |
| path: &str, | |
| _context: &Arc<Context>, | |
| ) -> Result<Option<Content>> { | |
| if let Some(hook) = &self.hooks.transform { | |
| let content_str = match content { | |
| Content::Js(js_content) => js_content.content.clone(), | |
| Content::Css(css_content) => css_content.clone(), | |
| _ => return Ok(None), | |
| }; | |
| let result: Option<TransformResult> = hook.call((content_str, path.to_string()))?; | |
| if let Some(result) = result { | |
| match result.content_type.as_str() { | |
| "js" | "ts" => { | |
| return Ok(Some(Content::Js(JsContent { | |
| content: result.content, | |
| is_jsx: false, | |
| }))) | |
| } | |
| "jsx" | "tsx" => { | |
| return Ok(Some(Content::Js(JsContent { | |
| content: result.content, | |
| is_jsx: true, | |
| }))) | |
| } | |
| "css" => return Ok(Some(Content::Css(result.content))), | |
| _ => return Err(anyhow!("Unsupported content type: {}", result.content_type)), | |
| } | |
| } | |
| } | |
| Ok(None) | |
| } | |
| fn load_transform( | |
| &self, | |
| content: &mut Content, | |
| path: &str, | |
| _context: &Arc<Context>, | |
| ) -> Result<Option<Content>> { | |
| if let Some(hook) = &self.hooks.transform { | |
| let content_str = match content { | |
| Content::Js(js_content) => js_content.content.clone(), | |
| Content::Css(css_content) => css_content.clone(), | |
| _ => return Ok(None), | |
| }; | |
| let result: Option<TransformResult> = hook.call((content_str, path.to_string()))?; | |
| if let Some(result) = result { | |
| match result.content_type.as_str() { | |
| "js" | "ts" => { | |
| return Ok(Some(Content::Js(JsContent { | |
| content: result.content, | |
| is_jsx: false, | |
| }))) | |
| } | |
| "jsx" | "tsx" => { | |
| return Ok(Some(Content::Js(JsContent { | |
| content: result.content, | |
| is_jsx: true, | |
| }))) | |
| } | |
| "css" => return Ok(Some(Content::Css(result.content))), | |
| _ => { | |
| eprintln!("Warning: Unsupported content type '{}', skipping transformation.", result.content_type); | |
| return Ok(None); | |
| } | |
| } | |
| } | |
| } | |
| Ok(None) | |
| } |
crates/mako/src/plugin.rs
Outdated
|
|
||
| pub fn load_transform( | ||
| &self, | ||
| content: &mut Content, | ||
| path: &str, | ||
| context: &Arc<Context>, | ||
| ) -> Result<Content> { | ||
| let mut content = content.clone(); | ||
| for plugin in &self.plugins { | ||
| if let Some(transformed) = plugin.load_transform(&mut content, path, context)? { | ||
| content = transformed; | ||
| } | ||
| } | ||
| Ok(content) | ||
| } |
There was a problem hiding this comment.
load_transform 方法的返回类型与 Plugin trait 中的定义不一致
在 Plugin trait 中，load_transform 方法的返回类型是 Result<Option<Content>>，而在 PluginDriver 中实现的返回类型是 Result<Content>。为了保持一致性，建议修改 PluginDriver 中的方法返回类型。
可以将 PluginDriver 中的 load_transform 方法的返回类型修改为 Result<Option<Content>>：
- ) -> Result<Content> {
+ ) -> Result<Option<Content>> {并在方法结束时返回 Ok(Some(content))：
- Ok(content)
+ Ok(Some(content))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn load_transform( | |
| &self, | |
| content: &mut Content, | |
| path: &str, | |
| context: &Arc<Context>, | |
| ) -> Result<Content> { | |
| let mut content = content.clone(); | |
| for plugin in &self.plugins { | |
| if let Some(transformed) = plugin.load_transform(&mut content, path, context)? { | |
| content = transformed; | |
| } | |
| } | |
| Ok(content) | |
| } | |
| pub fn load_transform( | |
| &self, | |
| content: &mut Content, | |
| path: &str, | |
| context: &Arc<Context>, | |
| ) -> Result<Option<Content>> { | |
| let mut content = content.clone(); | |
| for plugin in &self.plugins { | |
| if let Some(transformed) = plugin.load_transform(&mut content, path, context)? { | |
| content = transformed; | |
| } | |
| } | |
| Ok(Some(content)) | |
| } |
crates/mako/src/plugin.rs
Outdated
| path: &str, | ||
| context: &Arc<Context>, | ||
| ) -> Result<Content> { | ||
| let mut content = content.clone(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
避免不必要的克隆操作
在第 396 行，let mut content = content.clone(); 克隆了 content，这可能导致性能开销。考虑直接使用可变引用来修改 content，以避免不必要的克隆。
可以修改方法签名和实现，直接使用可变引用：
- pub fn load_transform(
- &self,
- content: &mut Content,
- path: &str,
- context: &Arc<Context>,
- ) -> Result<Content> {
- let mut content = content.clone();
+ pub fn load_transform(
+ &self,
+ content: &mut Content,
+ path: &str,
+ context: &Arc<Context>,
+ ) -> Result<Option<Content>> {并在循环中直接修改 content：
for plugin in &self.plugins {
- if let Some(transformed) = plugin.load_transform(&mut content, path, context)? {
- content = transformed;
+ if let Some(transformed) = plugin.load_transform(content, path, context)? {
+ *content = transformed;
}
}
- Ok(content)
+ Ok(Some(content.clone()))这样可以避免克隆操作，提高性能。
Committable suggestion was skipped due to low confidence.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1637 +/- ##
==========================================
+ Coverage 55.54% 55.57% +0.02%
==========================================
Files 172 172
Lines 17401 17414 +13
==========================================
+ Hits 9666 9678 +12
- Misses 7735 7736 +1 ☔ View full report in Codecov by Sentry. |
| let content_str = match content { | ||
| Content::Js(js_content) => js_content.content.clone(), | ||
| Content::Css(css_content) => css_content.clone(), | ||
| _ => return Ok(None), | ||
| }; | ||
|
|
||
| let result: Option<TransformResult> = hook.call((content_str, path.to_string()))?; | ||
|
|
||
| if let Some(result) = result { | ||
| match result.content_type.as_str() { | ||
| "js" | "ts" => { | ||
| return Ok(Some(Content::Js(JsContent { | ||
| content: result.content, | ||
| is_jsx: false, | ||
| }))) | ||
| } | ||
| "jsx" | "tsx" => { | ||
| return Ok(Some(Content::Js(JsContent { | ||
| content: result.content, | ||
| is_jsx: true, | ||
| }))) | ||
| } | ||
| "css" => return Ok(Some(Content::Css(result.content))), | ||
| _ => return Err(anyhow!("Unsupported content type: {}", result.content_type)), | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议提取公共函数以减少重复代码
在 load 和 load_transform 方法中，对 content_type 的匹配逻辑非常相似。建议将这些重复的代码提取到一个辅助函数中，以提高代码的可维护性和可读性。
可以创建一个辅助函数，例如：
fn content_from_result(content_type: &str, content: String) -> Result<Option<Content>> {
match content_type {
"js" | "ts" => Ok(Some(Content::Js(JsContent {
content,
is_jsx: false,
}))),
"jsx" | "tsx" => Ok(Some(Content::Js(JsContent {
content,
is_jsx: true,
}))),
"css" => Ok(Some(Content::Css(content))),
_ => Err(anyhow!("Unsupported content type: {}", content_type)),
}
}然后在 load_transform 方法中调用：
let result: Option<TransformResult> = hook.call((content_str, path.to_string()))?;
if let Some(result) = result {
- match result.content_type.as_str() {
- "js" | "ts" => {
- return Ok(Some(Content::Js(JsContent {
- content: result.content,
- is_jsx: false,
- })))
- }
- "jsx" | "tsx" => {
- return Ok(Some(Content::Js(JsContent {
- content: result.content,
- is_jsx: true,
- })))
- }
- "css" => return Ok(Some(Content::Css(result.content))),
- _ => return Err(anyhow!("Unsupported content type: {}", result.content_type)),
- }
+ return content_from_result(result.content_type.as_str(), result.content);
}
part of #1238
Summary by CodeRabbit
新功能
transform字段，用于内容处理和返回 Promise。load_transform方法，实现内容转换功能。文档
transform方法说明，并澄清现有配置项。测试