Repository: artusjs/core
Pull request #218 (2022-12-05 07:33:56)

fix: should call config hook in idle callback
<!--
Thank you for your pull request. Please review below requirements.
Bug fixes and new features should include tests and possibly benchmarks.

感谢您贡献代码。请确认下列 checklist 的完成情况。
Bug 修复和新功能必须包含测试，必要时请附上性能测试。
-->

##### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] `npm test` passes
- [x] tests and/or benchmarks are included
- [ ] documentation is changed or added
- [x] commit message follows commit guidelines

##### Affected core subsystem(s)
<!-- Provide affected core subsystem(s). -->


##### Description of change
<!-- Provide a description of the change below this comment. -->

config 的 lifecycle hook 是 loader 事件触发，当不存在 config 文件的时候不会触发 loader 的 before/after 事件，导致 config 的 lifecycle hook 也不会触发

~~所以在 loader 事件中加个 idle 事件，用于当某个 loader 没有被触发时回调，从而解决 config loader 没有使用时也能触发 configWillLoad 及 configDidLoad~~

根据 DEFAULT_LOADER_LIST_WITH_ORDER 进行分组，分组后按顺序执行，保证内置的每个 loader 的 before/after 都会触发

<!--
- any feature?
- close https://github.com/artusjs/core/ISSUE_URL
-->

codecov-commenter: # [Codecov](https://codecov.io/gh/artusjs/core/pull/218?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
Base: **92.42**% // Head: **92.64**% // Increases project coverage by **`+0.22%`** :tada:
> Coverage data is based on head [(`91db06c`)](https://codecov.io/gh/artusjs/core/pull/218?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) compared to base [(`9f3b77c`)](https://codecov.io/gh/artusjs/core/commit/9f3b77cfb4ab96b4354468c1f58f9ad7158ea864?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> Patch coverage: 85.71% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #218      +/-   ##
==========================================
+ Coverage   92.42%   92.64%   +0.22%     
==========================================
  Files          55       55              
  Lines        1293     1306      +13     
  Branches      218      220       +2     
==========================================
+ Hits         1195     1210      +15     
+ Misses         98       96       -2     
```


| [Impacted Files](https://codecov.io/gh/artusjs/core/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/loader/factory.ts](https://codecov.io/gh/artusjs/core/pull/218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2xvYWRlci9mYWN0b3J5LnRz) | `93.06% <80.00%> (+0.36%)` | :arrow_up: |
| [src/loader/loader\_event.ts](https://codecov.io/gh/artusjs/core/pull/218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2xvYWRlci9sb2FkZXJfZXZlbnQudHM=) | `92.50% <83.33%> (+0.83%)` | :arrow_up: |
| [src/application.ts](https://codecov.io/gh/artusjs/core/pull/218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2FwcGxpY2F0aW9uLnRz) | `91.42% <100.00%> (+0.51%)` | :arrow_up: |
| [src/scanner/scan.ts](https://codecov.io/gh/artusjs/core/pull/218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3NjYW5uZXIvc2Nhbi50cw==) | `95.89% <100.00%> (+1.36%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/artusjs/core/pull/218?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/src/loader/factory.ts b/src/loader/factory.ts
index 2963164..a6bc1ca 100644
--- a/src/loader/factory.ts
+++ b/src/loader/factory.ts
@@ -1,6 +1,6 @@
 import * as path from 'path';
 import { isInjectable, Container } from '@artus/injection';
-import { ArtusInjectEnum, DEFAULT_LOADER, HOOK_FILE_LOADER, LOADER_NAME_META, ScanPolicy } from '../constant';
+import { ArtusInjectEnum, DEFAULT_LOADER, HOOK_FILE_LOADER, LOADER_NAME_META, ScanPolicy, DEFAULT_LOADER_LIST_WITH_ORDER } from '../constant';
 import {
   Manifest,
   ManifestItem,
@@ -65,23 +65,31 @@ export class LoaderFactory {
   }
 
   async loadItemList(itemList: ManifestItem[] = [], root?: string): Promise<void> {
-    let prevLoader = '';
+    const itemMap = new Map(DEFAULT_LOADER_LIST_WITH_ORDER.map(loaderName => [loaderName, []]));
+
+    // group by loader names
     for (const item of itemList) {
       item.path = root ? path.join(root, item.path) : item.path;
-      const curLoader = item.loader ?? DEFAULT_LOADER;
-      if (item.loader !== prevLoader) {
-        if (prevLoader) {
-          await this.loaderEmitter.emitAfter(prevLoader);
-        }
-        await this.loaderEmitter.emitBefore(curLoader);
-        prevLoader = curLoader;
+      item.loader = item.loader ?? DEFAULT_LOADER;
+      if (!itemMap.has(item.loader)) {
+        // compatible for custom loader
+        itemMap.set(item.loader, []);
       }
-      await this.loaderEmitter.emitBeforeEach(curLoader, item);
-      const result = await this.loadItem(item);
-      await this.loaderEmitter.emitAfterEach(curLoader, item, result);
+      itemMap.get(item.loader)!.push(item);
     }
-    if (prevLoader) {
-      await this.loaderEmitter.emitAfter(prevLoader);
+
+    // trigger loader
+    for (const [ loaderName, itemList ] of itemMap) {
+      await this.loaderEmitter.emitBefore(loaderName);
+
+      for (const item of itemList) {
+        const curLoader = item.loader;
+        await this.loaderEmitter.emitBeforeEach(curLoader, item);
+        const result = await this.loadItem(item);
+        await this.loaderEmitter.emitAfterEach(curLoader, item, result);
+      }
+
+      await this.loaderEmitter.emitAfter(loaderName);
     }
   }
 


Test patch:
diff --git a/test/fixtures/app_without_config/app.ts b/test/fixtures/app_without_config/app.ts
new file mode 100644
index 0000000..4c1e736
--- /dev/null
+++ b/test/fixtures/app_without_config/app.ts
@@ -0,0 +1,43 @@
+import { LifecycleHookUnit, LifecycleHook } from '../../../src/decorator';
+import { ApplicationLifecycle } from '../../../src/types';
+import { Container, Inject } from '@artus/injection';
+import LifecycleList from './lifecyclelist';
+
+@LifecycleHookUnit()
+export default class MyLifecycle implements ApplicationLifecycle {
+  @Inject()
+  container: Container;
+
+  @Inject()
+  lifecycleList: LifecycleList;
+  
+  @LifecycleHook()
+  async configWillLoad() {
+    this.lifecycleList.add('configWillLoad');
+  }
+
+  @LifecycleHook()
+  async configDidLoad() {
+    this.lifecycleList.add('configDidLoad');
+  }
+
+  @LifecycleHook()
+  async willReady() {
+    this.lifecycleList.add('willReady');
+  }
+
+  @LifecycleHook()
+  async  didLoad() {
+    this.lifecycleList.add('didLoad');
+  }
+
+  @LifecycleHook('didReady')
+  async didReady() {
+    this.lifecycleList.add('didReady');
+  }
+
+  @LifecycleHook()
+  async beforeClose() {
+    this.lifecycleList.add('beforeClose');
+  }
+}
diff --git a/test/fixtures/app_without_config/config/.gitkeep b/test/fixtures/app_without_config/config/.gitkeep
new file mode 100644
index 0000000..e69de29
diff --git a/test/fixtures/app_without_config/lifecyclelist.ts b/test/fixtures/app_without_config/lifecyclelist.ts
new file mode 100644
index 0000000..7647701
--- /dev/null
+++ b/test/fixtures/app_without_config/lifecyclelist.ts
@@ -0,0 +1,12 @@
+import { Injectable, ScopeEnum } from '../../../src';
+
+@Injectable({
+  scope: ScopeEnum.SINGLETON,
+})
+export default class LifecycleList {
+  lifecycleList: string[] = [];
+
+  async add(name: string) {
+    this.lifecycleList.push(name);
+  }
+}
diff --git a/test/fixtures/app_without_config/package.json b/test/fixtures/app_without_config/package.json
new file mode 100644
index 0000000..d42cf40
--- /dev/null
+++ b/test/fixtures/app_without_config/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "demo-app-repo-ts",
+  "main": "src/index.ts"
+}
diff --git a/test/fixtures/app_without_config/tsconfig.json b/test/fixtures/app_without_config/tsconfig.json
new file mode 100644
index 0000000..2aa8781
--- /dev/null
+++ b/test/fixtures/app_without_config/tsconfig.json
@@ -0,0 +1,8 @@
+{
+  "compilerOptions": {
+    "esModuleInterop": true,
+    "experimentalDecorators": true
+  },
+  "rootDir": "./",
+  "exclude": []
+}
\ No newline at end of file
diff --git a/test/lifecycle.test.ts b/test/lifecycle.test.ts
index 735df6f..b1c302a 100644
--- a/test/lifecycle.test.ts
+++ b/test/lifecycle.test.ts
@@ -2,6 +2,7 @@ import path from 'path';
 import assert from 'assert';
 import { createApp } from './utils';
 import LifecycleList from './fixtures/app_with_lifecycle/lifecyclelist';
+import WithoutConfigLifecycleList from './fixtures/app_without_config/lifecyclelist';
 
 describe('test/lifecycle.test.ts', () => {
   it('should lifecycle works without error', async () => {
@@ -88,4 +89,20 @@ describe('test/lifecycle.test.ts', () => {
       'app_beforeClose',
     ]);
   });
+
+  it('should trigger configDidLoad without config file', async () => {
+    const appWithoutConfigPath = path.resolve(__dirname, './fixtures/app_without_config');
+    const app = await createApp(appWithoutConfigPath);
+    const lifecycleList = app.container.get(WithoutConfigLifecycleList).lifecycleList;
+    await app.close();
+
+    assert.deepStrictEqual(lifecycleList, [
+      'configWillLoad',
+      'configDidLoad',
+      'didLoad',
+      'willReady',
+      'didReady',
+      'beforeClose',
+    ]);
+  });
 });