-
Notifications
You must be signed in to change notification settings - Fork 85
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
fix/inter op missing and async module impl #943
Conversation
crates/mako/src/swc_helpers.rs
Outdated
lazy_static! { | ||
static ref HAHSED_HELPERS: HashMap<String, String> = [ | ||
( | ||
"d3__vuQ2".to_string(), |
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.
为啥要写死 hash?
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.
离线计算的 @swc/helpers/_/_interop_require_default 的 hashed module id,最终 replace 的 id。
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.
为了性能的话感觉没必要。
@@ -33,69 +70,21 @@ impl SwcHelpers { | |||
|
|||
// for watch mode | |||
pub fn full_helpers() -> HashSet<String> { |
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.
先让 prod 也走 full_helpers 来解吧,感觉为了这个引入复杂度不值得,同时感觉应该会有更简单的解。
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.
按需我觉得还是需要的,现在 webpack 一句 console.log(1) bundle 下 100个字节左右,mako 2000个。
按需本来就是复杂度,目前的复杂度也可以接受。
更好的解法就是,module graph 上记录模块是如何被 import/export 的(之后的各种 LTO 的时候也要修剪好这些边),最后 generate entry 时就能推断出来需要哪些 interop 了。
最坏情况是 O(n) n 是模块数,如果是一般的项目,很快就能推断是 三个 interop 都用了就能快速退出了。
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.
按需我也觉得需要,只不过在复杂度和功能之间,我宁愿选择先简单点苟着,因为这个带来的问题很小,然后等更好的最终方案。
@@ -15,3 +15,5 @@ assert.match( | |||
moduleReg("src/b.ts", `__mako_require__._async.+`, true), | |||
"top level await module should have __mako_require__._async" | |||
); | |||
|
|||
require("./dist/index.js"); |
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.
这么写看起来有效,但感觉有点 hack。最好还是把断言都放回到 expect.js 里。
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.
expect.js 中无法方便的断言运行时的问题;我的观点是最好增加运行时的断言。
webpack 的用例都是这么写的。
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.
同意「增加运行时断言」,但相比 require("./dist/index.js")
感觉应该有更好的设计。(这个没想好之前也可以先这么用)
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.
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.
可以直接利用一个真正的测试框架 比如 jest,只是他的背后隐藏了 require 类似的操作。
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.
context 上的 swc_helpers 属性可以删了。
|
close #933 and close #934
for 933
使用全量的 swc interop helpers for: https://github.com/umijs/mako/pull/943/files#r1522337996
支持深度可配置 require 检测 transformfor 934
调整原来 async module 实现