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

WIP: feat: add new unit called bootstrap #144

Closed
wants to merge 3 commits into from

Conversation

noahziheng
Copy link
Member

现行的机制中,“服务器启动”这一行为在 willReady 钩子中完成,但受插件依赖关系计算的影响,插件加载顺序不受主观控制(仅遵循相互依赖);

在这一前提条件下,协议实现并不能保证自己的启动钩子在所有插件最后执行,于是 willReady 将可能不是 "will";

为了保证逻辑清晰,Lifecycle 完全符合预期,有两种有效思路:

  • willReadydidReady 增加 serverStart LifecycleHook,会存在问题:
    • emitHook 现版本实现允许多次触发
    • 一旦向用户暴露,可能产生误用,且 Hook 可存在多个,仍有顺序问题
  • 实现 RFC 上层框架的启动方式建议 #74 中描述的 Bootstrap 机制,略有改动:
    • 考虑到单一 Application 内只存在一种协议实现(单一 Trigger 覆盖),Bootstrap 亦可为全局单例

综上,当前 PR 实现了方案 2

PS: 时间原因仅改动了 app-koa-with-ts,其他覆盖有待加强

// Notify protocol-implementer that the server can be started
let bootstrap: Bootstrap;
try {
bootstrap = await this.container.getAsync(ArtusInjectEnum.Bootstrap);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个行为要不要交给上层框架去 override,现在约定了上层框架必须去实现一个这样的 class,感觉不是特别好

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Collaborator

Choose a reason for hiding this comment

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

不过这样实现也行,相当于artus/core支持了两种启动方式。协议推荐提供ArtusInjectEnum.Bootstrap

Copy link
Collaborator

Choose a reason for hiding this comment

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

现在的问题是回归到生命周期的执行阶段上,现在是触发的时机不合适,有自定义的需求,是不是上层有这样的需求自定义

Copy link
Member

Choose a reason for hiding this comment

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

DefineBootstrap 可以和 LifecycleHookUnit 一样直接定义 loader,可以不用约定上层的具体 id

@noahziheng noahziheng changed the title feat: add new unit called bootstrap WIP: feat: add new unit called bootstrap Jul 15, 2022
@noahziheng
Copy link
Member Author

UPDATE: 考虑这个方案还是引入了新的复杂度,先暂缓一轮,后面考虑 Lifecycle 增强能否 cover 这部分问题

@noahziheng noahziheng closed this Jul 18, 2022
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.

5 participants