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

[RFC]: loader 阶段怎么执行hook #130

Open
JerrysShan opened this issue Jul 11, 2022 · 15 comments
Open

[RFC]: loader 阶段怎么执行hook #130

JerrysShan opened this issue Jul 11, 2022 · 15 comments

Comments

@JerrysShan
Copy link
Collaborator

背景: 现在 scan 扫描的结果每个都是单个 item ,导致想要执行一些 hook 操作的时候不知道在哪个时机触发,例如想要 config 文件加载前执行 'configWillLoad' ,加载完之后触发 'configDidLoad' 。

目前能想到的解决方案就是对 scan 扫描结果进行归类,但是这个方案之前被否过,针对这个问题大家看看有没有什么好的解决方案

@hyj1991 @noahziheng @DuanPengfei

@atian25
Copy link
Member

atian25 commented Jul 11, 2022

是为了静态扫描获取最终配置么?

Egg 之前也考虑过类似的事,因为需要静态分析,做了很多 AST,最终发现,还不如直接 new Egg() 启动一下来获取配置更直接更便捷, T_T

@JerrysShan
Copy link
Collaborator Author

是为了静态扫描获取最终配置么?

是为了获取最终合并的配置结果,然后根据配置做些操作

@hyj1991
Copy link
Member

hyj1991 commented Jul 11, 2022

loader 阶段执行 lifecycle 的场景是?

生命周期钩子现在的设计里是作用于启动期的,现在的 emit hook 时机也是正确的

@noahziheng
Copy link
Member

@hyj1991 此处的问题是在 LoaderFactory 中现在依靠 loadItemList 的遍历边界完成了 configWillLoad/configDidLoad 的钩子触发和配置的合并,这个当时就说过是临时方案,边界 case 会很多,可能不会太准确

image

@JerrysShan 的方案是将 manifest 从有序大数组再抽象一级为对象/map(key 为 loaderName,value 为 itemList),代价上我评估主要是改造成本,同时对于 loader 的保证执行顺序和扩展等逻辑需要在 LoaderFactory 内复现(loaderListGenerator)

image

@hyj1991
Copy link
Member

hyj1991 commented Jul 12, 2022

@noahziheng 在 scan 阶段触发这些钩子也不会有啥问题,我是想问有什么场景需要在 scan 阶段使用到 config hook?

@atian25
Copy link
Member

atian25 commented Jul 12, 2022

是为了静态扫描获取最终配置么?

是为了获取最终合并的配置结果,然后根据配置做些操作

那应该一样的问题,很难完整实现。举个例子,如果有一个插件在 configWillLoad 里面去请求远程配置拿数据,这样的话,一串的中间件都需要关联启动了,相当于启动应用了,这种场景下,还不如直接 new 一个 Artus 启动但关闭 HTTP 服务之类的。

@JerrysShan
Copy link
Collaborator Author

@noahziheng 在 scan 阶段触发这些钩子也不会有啥问题,我是想问有什么场景需要在 scan 阶段使用到 config hook?

不是scan 阶段用到 config hook ,而是现阶段 loader hook 执行时机依赖 scanner 扫描后生成的manifest 里面的顺序,如果发现
前后两个item 的 loader 不一样,就执行相应的loader before 和 after hook;

现阶段 scanner 扫描生成的顺序,依赖下面这个常量的顺序,如果支持自定义loader ,假如把下面这个常量的顺序调整了,顺序就没办法得到保证,现在缺乏一种机制保证 loader 的顺序
image

@hyj1991
Copy link
Member

hyj1991 commented Jul 12, 2022

@JerrysShan 这个问题首先要明确启动期一定有顺序的,和顺序无所谓的

  • 一定要有顺序的
    • exception
    • config

这两个必须在 itemMap 的顶部


  • 无所谓顺序的
    • lifecycle-hook-unit
    • plugin-meta
    • module

这个即可给任意调整顺序


  • 启动期目前仅设置容器对象但没实际用处的
    • plugin-config
    • framework-config
    • package-json

这些可以被任意调整顺序

@hyj1991
Copy link
Member

hyj1991 commented Jul 12, 2022

按照这个分类,可以设置分离的常量数组分开 concat,只要保证一定要保证顺序的在最前面即可,剩下的两类可以让用户自己覆盖或者调整顺序

@noahziheng
Copy link
Member

那应该一样的问题,很难完整实现。举个例子,如果有一个插件在 configWillLoad 里面去请求远程配置拿数据,这样的话,一串的中间件都需要关联启动了,相当于启动应用了,这种场景下,还不如直接 new 一个 Artus 启动但关闭 HTTP 服务之类的。

@atian25 此处的问题不是外部分析相关的,是内部实现中有关 loader 执行顺序的问题

@noahziheng
Copy link
Member

按照这个分类,可以设置分离的常量数组分开 concat,只要保证一定要保证顺序的在最前面即可,剩下的两类可以让用户覆盖

这个地方你之前说可能会有需要在所有 loader 执行前插入 loader 的诉求,所以 loaderListGenerator 的实现是可以各种覆盖的,现在这个诉求还有吗?

@JerrysShan
Copy link
Collaborator Author

问题就在于启动时是有顺序要求的,但是这个顺序依据的是 scanner 扫描后的结果,如果扫描的顺序有问题就会造成启动的顺序没办法保证 (也可以不当成问题,我们严格要求自定义 loader 不能打乱扫描的顺讯)

@hyj1991
Copy link
Member

hyj1991 commented Jul 12, 2022

这个地方你之前说可能会有需要在所有 loader 执行前插入 loader 的诉求,所以 loaderListGenerator 的实现是可以各种覆盖的,现在这个诉求还有吗?

插入 loader 的需求肯定在,但是不会插入 exceptionconfig 这样的,后面这些顺序是无所谓的

@hyj1991
Copy link
Member

hyj1991 commented Jul 12, 2022

问题就在于启动时是有顺序要求的,但是这个顺序依据的是 scanner 扫描后的结果,如果扫描的顺序有问题就会造成启动的顺序没办法保证 (也可以不当成问题,我们严格要求自定义 loader 不能打乱扫描的顺讯)

启动时顺序依赖 manifest,但是除了 exceptionconfig 剩下的 loader 模块顺序不管怎么调整,都不会对启动造成影响,所以我认为只要明确 scanner 无法覆盖 excerptionconfig loader 定义就可以了

@noahziheng
Copy link
Member

语音讨论结论,顺序实现不变,改动点如下:

  • 增加防呆设计,Scanner.loaderListGenerator 对于必须保证顺序的 loader 单独一个常量数组,不参与用户定制过程(MUST_TOP_LOADER_LIST.concat(loaderListGenerator(NORMAL_DEFAULT_LOADER_LIST))
  • 现行 before/after 触发机制不改,对于 Config 的特例,增加处理:
    • ConfigurationHandler 增加无 Config 的边界判断
    • Scanner 未扫描到 Config 文件存在时放个特殊占位符,确保 configWillLoad/configDidLoad 仍能触发

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

No branches or pull requests

4 participants