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: hmr supports linked npm packages changes #864

Merged
merged 5 commits into from
Jan 29, 2024

Conversation

zhangpanweb
Copy link
Contributor

@zhangpanweb zhangpanweb commented Jan 14, 2024

支持 npm包 link 的热更。通过在构建后,获取所有的依赖路径并添加到watch列表中实现。

存在的一个问题是,在 link 的 npm包中,引用一个不存在的文件,dev启动后,再添加一个这个文件,此时热更会失效,不会重新 resolve 到这个新添加的文件。原因是:resolve 一个 link npm包中不存在的文件,此文件会成为一个 missing_dep, missing_dep 在 nodejs_resolver 中没办法获取可能的 resolve 路径,所以此路径不会被添加到 watch 列表中,后续此文件的添加,不会触发文件变更事件,触发hmr。考虑在替换 oxc_resolver 后解决,oxc_resolver 能够获取 resolve 过程中 missing_dep 可能 resolve 路径,获取后添加到 watch 列表中即可

@@ -32,6 +40,25 @@ impl Watch {
}
Ok(())
})?;

let module_graph = compiler.context.module_graph.read().unwrap();
let (dependencies, _) = module_graph.toposort();
Copy link
Member

Choose a reason for hiding this comment

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

topsort 是必要的吗? 是不是只要遍历所有模块就够了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

哦哦是的,我改一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已更新

@stormslowly
Copy link
Member

missing 的思路是不是可以这么解解看

https://github.com/umijs/mako/pull/864/files#diff-b19ac29cec540185d7265ee30a42c0a6cc8e12b484363c0a6733880a54673620R55

不添加watch 的文件路径了,添加 watch 文件所在的目录( NonRecursive) 这样的 missing 的添加回来 是不是就可以感知到了。

@zhangpanweb
Copy link
Contributor Author

missing_dep 跟引入的 module不一定在一个目录。比如下面的情况, b.js 如果是 miss_dep, 通过watch a 目录,没办法感知到 b.js 的添加

// src/a/a.js
import { b } from '../b/b';
console.log(b);

// src/b/b.js
const b = "b";
export { b };

@sorrycc
Copy link
Member

sorrycc commented Jan 15, 2024

这是对每个模块单独做 watch?所有 node_modules 下的文件都会包含吗?感觉几万个模块的项目可能会挂,找个真实项目试试?比如 yuyan_assets。

@zhangpanweb
Copy link
Contributor Author

zhangpanweb commented Jan 15, 2024

是的,对 module_graph 中的每个模块watch。open-yuyan-assets 试了下 11477 个模块,正常。其他找到的项目,模块树都比这个少。我的电脑, 通过 launchctl limit maxfiles 获取的最大打开文件树,软限制 256,硬限制 unlimited

之前有考虑,是否可以加一个类似 extraWatchFiles 的参数,用于用户自己配置需要额外 watch 的文件或目录。这样可以避免达到最大打开数量限制的问题,也可以解决上面对于 missing deps 的 watch 问题。缺点是需要用户自己去配置

@sorrycc
Copy link
Member

sorrycc commented Jan 16, 2024

补充我的想法,

1、项目文件已经 watch 过了,再加 watch 是否重复?
2、现在 watch 只在初始化时执行,所以不是 missing 的场景,正常「先添加一个文件,再 require 他」的场景,新增的文件也不会被 watch
3、open-yuyan-assets 太小,试试 yuyan/yuyan-assets

@zhangpanweb
Copy link
Contributor Author

更新了一版:

  • watch 时,获取所有 module,根据 module 获取所在的 package,如果此 package 不在 root 目录内,则进行 watch
  • 在 rebuild 之后,根据 module_graph 刷新一次 watch

采用这种方式更新的原因:

  • 用 webpack-dev-server + webpack 试了下,也没有支持 node_modules 内部 package 变化的热更。如果也不支持,上述的方式是比较方便的方式。webpack 采用的是 fs.watch 的方式监听的文件变化,起了个简单项目看,监听的都是文件,没有目录
  • rebuild,解决包第一层文件没有被 watch 到(比如 项目文件夹跟 package.json 平级的文件)的问题 以及 过程中引入其他可能的依赖
  • 如果直接 watch module 文件,而不是目录,上面 missing dep 的问题目前不好解

Copy link
Member

@sorrycc sorrycc left a comment

Choose a reason for hiding this comment

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

用 yuyanAssets,算一下 watch 在初始执行和增量执行的消耗分别有多少(附上测试机型),尤其是后者。

{
self.watch_dir_recursive(
dir.into(),
&[".git", "node_modules", ".DS_Store", ".node"],
Copy link
Member

Choose a reason for hiding this comment

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

这个和前面的冲突了,提一下,可能经常要调整。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

提为了一个变量

let dir = dir.dir().as_ref();
// not in root dir or is root's parent dir
if dir.strip_prefix(self.root).is_err()
&& self.root.clone().strip_prefix(dir).is_err()
Copy link
Member

Choose a reason for hiding this comment

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

这个 clone 看能不能省,因为每个 module 都会跑一遍,几万个 module 应该还是有消耗的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已更新

@zhangpanweb
Copy link
Contributor Author

做了一些优化后测试了一下,测试机型 MacBoock M2 pro

  • 初始化测试波动非常大,整体消耗首次会多一倍
  • 增量执行,modify 下耗时非常小
  • 整体时间,watch 上耗时最多,特别是对文件夹的 recursive watch

更新前:
image

更新后:
image

@sorrycc
Copy link
Member

sorrycc commented Jan 22, 2024

@zhangpanweb 改之前的 watch 是否耗时很少?

@zhangpanweb
Copy link
Contributor Author

改之前,watch 跑了几次,88ms、103ms、93ms,平均90几ms

@sorrycc sorrycc merged commit e161685 into master Jan 29, 2024
8 checks passed
@delete-merged-branch delete-merged-branch bot deleted the feat/hmr-support-link-package branch January 29, 2024 03:21
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.

3 participants