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.

Sign up for GitHub

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: legacy interop #1638

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: legacy interop #1638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c65d900
feat: legacy interop
Jinbao1001 Oct 15, 2024
beb113a
fix: default value
Jinbao1001 Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/mako/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ pub struct Config {
pub clean: bool,
pub node_polyfill: bool,
pub ignores: Vec<String>,
#[serde(rename = "legacyInterop")]
pub legacy_interop: bool,
#[serde(
rename = "_minifish",
deserialize_with = "deserialize_minifish",
Expand Down
1 change: 1 addition & 0 deletions crates/mako/src/config/mako.config.default.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"graphviz": false
}
},
"legacyInterop": false,
"useDefineForClassFields": true,
"emitDecoratorMetadata": false,
"watch": { "ignorePaths": [], "_nodeModulesRegexes": [] },
Expand Down
28 changes: 24 additions & 4 deletions crates/mako/src/plugins/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
let helpers = SwcHelpers::full_helpers()
.into_iter()
.map(|source| {
let code = Self::get_swc_helper_code(&source).unwrap();
let code =
Self::get_swc_helper_code(&source, context.config.legacy_interop).unwrap();
Comment on lines +49 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

避免使用 unwrap() 以防止潜在的崩溃

在第 49-50 行中，调用了 get_swc_helper_code 方法并直接使用了 unwrap()。如果该方法返回 Err，程序将会发生崩溃。建议使用 ? 运算符或匹配处理错误，以提高代码的稳健性。

let module_id: ModuleId = source.into();
let module_id = module_id.generate(context);
format!("\"{}\": {}", module_id, code)
Expand All @@ -65,7 +66,22 @@
))
}

fn get_swc_helper_code(path: &str) -> Result<String> {
fn modify_legacy_code(origin_code: &str, legacy_interop: bool) -> String {
let mut new_code = origin_code.to_string();
if legacy_interop {
new_code = new_code.replace(

Check warning on line 72 in crates/mako/src/plugins/runtime.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/plugins/runtime.rs#L72 

Added line #L72 was not covered by tests
"// ### legacy_interop ###",
r#"
if (typeof obj === "function") {
return obj
};
"#,
);
}
new_code
}

fn get_swc_helper_code(path: &str, legacy_interop: bool) -> Result<String> {
let code = match path {
"@swc/helpers/_/_interop_require_default" => r#"
function(module, exports, __mako_require__) {
Expand Down Expand Up @@ -93,7 +109,8 @@
}
}
"#.trim(),
"@swc/helpers/_/_interop_require_wildcard" => r#"
"@swc/helpers/_/_interop_require_wildcard" => {
let origin = r#"
function(module, exports, __mako_require__) {
__mako_require__.d(exports, "__esModule", {
value: true
Expand Down Expand Up @@ -125,6 +142,7 @@
if (obj === null || typeof obj !== "object" && typeof obj !== "function") return {
default: obj
};
// ### legacy_interop ###
var cache = _getRequireWildcardCache(nodeInterop);
if (cache && cache.has(obj)) return cache.get(obj);
var newObj = {};
Expand All @@ -139,7 +157,9 @@
return newObj;
}
}
"#.trim(),
"#.trim();
&Self::modify_legacy_code(origin, legacy_interop)
},
Comment on lines +161 to +162
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

小心字符串替换的可靠性

在第 161-162 行中，使用了 modify_legacy_code 方法对 origin 代码进行字符串替换。依赖特定字符串（如 // ### legacy_interop ###）进行替换可能在上游代码改变时失效。建议使用更可靠的方法，如解析代码的 AST，来定位并修改目标代码片段，以提高代码的鲁棒性。

"@swc/helpers/_/_export_star" => r#"
function(module, exports, __mako_require__) {
__mako_require__.d(exports, "__esModule", {
Expand Down
6 changes: 6 additions & 0 deletions e2e/fixtures/config.legacy_interop/expect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const { injectSimpleJest } = require("../../../scripts/test-utils");

injectSimpleJest()

require("./dist/index.js");

3 changes: 3 additions & 0 deletions e2e/fixtures/config.legacy_interop/mako.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"legacyInterop": true
}
1 change: 1 addition & 0 deletions e2e/fixtures/config.legacy_interop/src/a.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = () => 1
10 changes: 10 additions & 0 deletions e2e/fixtures/config.legacy_interop/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// @ts-ignore
import * as a from './a'




it('should work when config legacyInterop',()=>{
expect(a()).toBe(1)
});
Comment on lines +7 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议改进测试用例的实现和文档

当前的测试用例虽然功能正确，但可以通过以下方式进行改进：

  1. 测试描述可以更具体，明确说明正在测试遗留互操作性的哪个方面。
  2. 添加注释解释 a() 函数的预期行为及其返回值的含义。
  3. 考虑添加更多断言来全面测试遗留互操作性功能。

建议修改如下：

it('应该在启用 legacyInterop 配置时正确导入和使用模块 a', () => {
  // 验证模块 a 被正确导入并按预期工作
  expect(a()).toBe(1);
  
  // 可以添加更多断言来测试其他方面的行为
  // 例如：expect(typeof a).toBe('function');
});


Loading