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] egg-core增加应用启动阶段 #2520

Closed
killagu opened this issue May 8, 2018 · 48 comments
Closed

[RFC] egg-core增加应用启动阶段 #2520

killagu opened this issue May 8, 2018 · 48 comments

Comments

@killagu
Copy link
Contributor

killagu commented May 8, 2018

背景描述

有的插件可能需要异步操作后才能加载, 需要使用beforeStart来执行异步操作,
挂载到app上。

同时应用启动前, 又在beforeStart中使用了这样的插件, 就可能不能正常启动。

例如:

// pluginA/index.js

app.beforeStart(async ()=>{
  const bar = await pull();
  app.addSingleton('foo', (config,app) => {
    return new Foo(bar);
  });
});
// app.js

app.beforeStart(async ()=>{
  await app.foo.foooo(); // app.foo 可能还没注入
});

解决方案

EggCore上增加appReady。

beforeAppStart(scope) {
  const done = this.appReady.readyCallback(name);

  process.nextTick(() => {
    this.ready()
    .then(()=>utils.callFn(scope))
    .then(() => done(), done);
  });
}

服务器启动时也应该修改为app.appReady.ready(startServer);

定义一个新目录, 暂时命名为app/checker

在应用启动阶段(所有的beforeStart都执行完毕),

调用app/checker目录下的checker实现, 调用完成后调用通过app.ready注册的回调。

现征集该目录的命名, 暂有两个选项:

  • checker
  • boot

cc: @gxcsoccer @XadillaX @popomore @coolme200

@egg-bot
Copy link

egg-bot commented May 8, 2018

Translation of this issue:


[RFC] egg-core increase appReady

Background description

Some plugins may need to be asynchronous before they can be loaded. They need to use beforeStart to perform asynchronous operations.
Mount it to ʻapp`.

At the same time, before the application starts, if such a plugin is used in beforeStart, it may not start normally.

E.g:

// pluginA/index.js

app.beforeStart(async ()=>{
  Const bar = await pull();
  app.addSingleton('foo', (config,app) => {
    Return new Foo(bar);
  });
});
// app.js

app.beforeStart(async ()=>{
  Await app.foo.foooo(); // app.foo may not be injected yet
});

solution

Add appReady to EggCore.

beforeAppStart(scope) {
  Const done = this.appReady.readyCallback(name);

  process.nextTick(() => {
    This.ready()
    .then(()=>utils.callFn(scope))
    .then(() => done(), done);
  });
}

The server should also be modified to ʻapp.appReady.ready(startServer);`

@coolme200
Copy link
Contributor

app.appReady 下面还有几个 API?

@killagu
Copy link
Contributor Author

killagu commented May 8, 2018

app.appReady这个就是个ready-callback对象。

  • ready
  • readyCallback
  • readyDone

@tong3jie
Copy link
Contributor

tong3jie commented May 8, 2018

@killagu
你可以在应用启动后执行相关的服务,例如:app的API接口已经提供了ready方法,例如:

module.exports = app => {
  app.ready(() => {
    console.log('app is ready!');
    const ctx = app.createAnonymousContext();
    ctx.service.send.abc();
    ctx.service.send.abd();
    ctx.service.send.acd();
  });
};

@killagu
Copy link
Contributor Author

killagu commented May 8, 2018

@tong3jie 你讲的没错。ready是应用启动后执行相关的服务。

但是我讲的情况都是在应用启动前的, beforeStart的情况。

@atian25
Copy link
Member

atian25 commented May 8, 2018

addSingleton had support async since last month

@popomore
Copy link
Member

popomore commented May 8, 2018

两个想法

  1. 提供 afterReady,等现在的 ready 后出发 afterReady 再触发现在的 ready(也就是启动)

  2. 提供一个启动检查模式,在 app/start_checker 中定义一个类,实现 check 方法(实现和第一条类似)。

@killagu
Copy link
Contributor Author

killagu commented May 8, 2018

提供 afterReady,等现在的 ready 后出发 afterReady 再触发现在的 ready(也就是启动)

afterReady这个名字有歧义。其实是在ready之前触发的。

start_checker +1

@popomore
Copy link
Member

popomore commented May 9, 2018

可能还可以有 health checker

@killagu killagu self-assigned this May 11, 2018
killagu pushed a commit to eggjs/core that referenced this issue May 16, 2018
implement checker loader, load `app/checker`.
Checkers will be called after all readyCallbacks called,
then emit `app.ready(true)`.

Refs: eggjs/egg#2520
@killagu killagu changed the title [RFC] egg-core增加appReady [RFC] egg-core增加应用启动阶段 May 26, 2018
@killagu
Copy link
Contributor Author

killagu commented May 26, 2018

boot +1

@XadillaX
Copy link
Member

boot + 1

@popomore
Copy link
Member

starter 呢?

@dead-horse
Copy link
Member

定义一个新目录, 暂时命名为app/checker

直觉让我反对新加一个目录

@popomore
Copy link
Member

新加目录比新加 API 友好些,比如 schedule,notify 都是新增目录的

@dead-horse
Copy link
Member

这个功能比较反对新加目录的原因主要还是这些功能都是应用开发者基本用不到的。

egg 的启动,有点像 react 的生命周期的意思了。是否应该再完善一点,把启动时期的各个生命周期给定义下来:

- appDidLoad
- appWillStart
- appDidStart
- appWillClose

@atian25
Copy link
Member

atian25 commented May 26, 2018

life cycle +1

@dead-horse
Copy link
Member

而且 issue 中描述的这个问题,通过新增一个应用启动阶段,看似可以解决当下的问题,但是其实是在埋坑,最多只能算是一个临时解决方案。现在如果 B 依赖 A,把 B 放在启动阶段,A 放在 beforeStart 是可以解决了,那如果出现了 C 依赖 B, B 依赖 A 呢?C 放哪里?

本质上这还是执行顺序的问题,想清楚之前不建议加新的

@popomore
Copy link
Member

popomore commented May 26, 2018

如果 C 和 B 都是应用定义的话,那在应用里面可以写在一起。

本质还是应用开发者对 beforeStart 的真正启动顺序不理解,不改变 beforeStart 的前提下,需要提供应用启动的环节。增加 API 和增加目录都是具体的解决方案,应用启动的这个环节是要真正增加的。

@dead-horse
Copy link
Member

现在给应用层的 app.js 实在是太尴尬了。90% 的人是用做应用启动阶段的,但是还有部分的人在里面操作 middleware,这个又不能算启动阶段。

@popomore
Copy link
Member

我本身是不太建议在应用层用 app.js 的,因为对传统开发者来说这就是入口文件,感觉对顺序还是比较难理解的。

@killagu
Copy link
Contributor Author

killagu commented May 28, 2018

关于生命周期这个, 我觉得是这样的:

- app.beforeStart // 代表load的过程
- appDidStart: app.ready
- appWillClose: app.beforeClose

现在appDidLoadappWillStart是没有对应的实现, 但是这两个生命周期是APP层面就可以实现的。

要把原来的这些方法改造相比于增加一个目录来说成本太大了。

@dead-horse

@gxcsoccer
Copy link
Contributor

egg 的启动,有点像 react 的生命周期的意思了。是否应该再完善一点,把启动时期的各个生命周期给定义下来:

同意,但是觉得没必要像 react 那么复杂

@gxcsoccer
Copy link
Contributor

gxcsoccer commented Jun 19, 2018

我能接受的三个方案

1. 不改 API

应用里面的 app.beforeStart 放到所有其他 beforeStart 执行完以后再执行

  • 好处:不用新增加目录、API,也能解决最初的问题
  • 坏处:对于应用来说 beforeStart API 含义有点 breaking change,另外不能解决插件里类似问题

2. 加启动目录

如上描述

3. 规范插件,保证 app 上的实例挂载是同步的,通过 app.xxx.ready 来管理依赖

之前有问题的 case 是:

app.beforeStart(async function() {
  // 异步操作
  await fetchPWD();

  app.addSingleton('mysql', () => {
      // ...
      return instance;
  });
});

可以改成如下,确保 app.mysql 一开始就有

app.beforeStart(async function() {
  app.addSingleton('mysql', async () => {
      // 异步操作
      await fetchPWD();
      // ...
      return instance;
  });
});

@killagu
Copy link
Contributor Author

killagu commented Jun 21, 2018

@okoala boot 类是为了取代 app.js 现有的写法的, 框架层和应用层都能使用的。

提供的示例代码中 hook 方式更像是底层API, 实现的时候可能是使用类似的方法, 上层提供类的方式更友好一些。

面向应用开发者提供一种实现方式,达到统一的规范。

还有没看懂 userAfterCreate 这是什么方法?

运行时如何调用是实现细节了, 就不在此处讨论了。

@okoala
Copy link
Member

okoala commented Jun 21, 2018

@killagu userAfterCreate 表明是自定义的 hook,可以是业务层自己自定义,hook 是比较底层,但是扩展性就更广了。甚至你可以 hook 到某个插件中。

@killagu
Copy link
Contributor Author

killagu commented Jun 21, 2018

@okoala 你这样说就好像 messenger 了不是么, 一方去 messenger.on('foo'), 另一方去 messenger.send('foo'), 和我们讨论的生命周期不是一个概念了。

popomore pushed a commit that referenced this issue Jun 21, 2018
popomore pushed a commit that referenced this issue Jun 21, 2018
doc: add lifecyle doc (#2708)

Refs: #2520
killagu added a commit to node-modules/ready-callback that referenced this issue Jun 26, 2018
@popomore
Copy link
Member

killagu added a commit to eggjs/core that referenced this issue Jul 3, 2018
1. add `Boot` Class, developer can inherit from it.
2. add `startBoot`, application like egg should call
`startBoot` after all files loaded.

Refs: eggjs/egg#2520
killagu added a commit to eggjs/core that referenced this issue Jul 3, 2018
1. add `Boot` Class, developer can inherit from it.
2. add `startBoot`, application like egg should call
`startBoot` after all files loaded.

Refs: eggjs/egg#2520
@atian25
Copy link
Member

atian25 commented Jul 4, 2018

启动时间这里也需要加下打点, #1898 (comment)

killagu added a commit to eggjs/core that referenced this issue Jul 11, 2018
1. add `Boot` Class, developer can inherit from it.
2. add `startBoot`, application like egg should call
`startBoot` after all files loaded.

Refs: eggjs/egg#2520
killagu added a commit to eggjs/core that referenced this issue Jul 19, 2018
1. add `Boot` Class, developer can inherit from it.
2. add `startBoot`, application like egg should call
`startBoot` after all files loaded.

Refs: eggjs/egg#2520
killagu added a commit to eggjs/core that referenced this issue Aug 23, 2018
1. add `Boot` Class, developer can inherit from it.
2. add `startBoot`, application like egg should call
`startBoot` after all files loaded.

Refs: eggjs/egg#2520
killagu added a commit to eggjs/core that referenced this issue Aug 30, 2018
1. add `Boot` Class, developer can inherit from it.
2. add `startBoot`, application like egg should call
`startBoot` after all files loaded.

Refs: eggjs/egg#2520
popomore pushed a commit to eggjs/core that referenced this issue Sep 6, 2018
1. Add `Lifecycle` class, control EggCore lifecycle.
2. After config, extend, app.js, angent.js loaded, loader should call `initBoots`.
3. After all files loaded, loader should call `triggerDidLoad`.

Refs: eggjs/egg#2520
@atian25
Copy link
Member

atian25 commented Sep 6, 2018

文档是不是也要改改

killagu added a commit that referenced this issue Sep 6, 2018
killagu added a commit that referenced this issue Sep 11, 2018
atian25 pushed a commit that referenced this issue Sep 12, 2018
@atian25
Copy link
Member

atian25 commented Sep 12, 2018

land via #2972 and publish at #2989

@atian25 atian25 closed this as completed Sep 12, 2018
popomore pushed a commit that referenced this issue Sep 12, 2018
feat: support boot lifecyle (#2972)

Refs: #2520
@atian25
Copy link
Member

atian25 commented Sep 12, 2018

@atian25 atian25 mentioned this issue Sep 12, 2018
4 tasks
@ghost
Copy link

ghost commented Sep 12, 2018

@killagu:What about making 'configDidLoad' async?All the other methods are async.

@atian25:I'll help to do English trans if free.

@killagu
Copy link
Contributor Author

killagu commented Sep 12, 2018

@Maledong configDidLoad should be sync. It's same as

// app.js
module.exports = app => {
  // e.g. modify core middlewares order
};

It have be done before other files load, so the modifications can take effect.

hyj1991 pushed a commit to hyj1991/egg-core that referenced this issue Sep 16, 2021
1. Add `Lifecycle` class, control EggCore lifecycle.
2. After config, extend, app.js, angent.js loaded, loader should call `initBoots`.
3. After all files loaded, loader should call `triggerDidLoad`.

Refs: eggjs/egg#2520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants