Repository: artusjs/core (ts)

Issue:
framework和plugin配置里关于 path 信息处理的问题
现在framework和plugin 的配置里都有 path 或者 package 信息，
对于 package 的场合问题不大，因为有用到 require.resolve 。

而 path 的场合，是直接处理成相对/绝对路径，这里其实有点问题

比如我一个 framework 或 plugin 是不打算发布到 registry 的，那我肯定长期使用 path 的配置去引用，
那我建议需要参考这个包的 package .json 的 main 属性，而不是指定到具体的 dist 、 build 、libs 这样的目录位置。

Pull request description:
refactor(loader): format plugin config in factory && support inline package
resolve #228 

主要改动：
- 移除 PluginConfigLoader，统一由 ConfigLoader 处理
- 移除扫描器最终对 `plugin-config` 的无意义过滤逻辑，manifest 中将继续包括 `plugin` 相关配置
- ConfigLoader 针对 `plugin` 使用新增的 PluginFactory.formatPluginConfig 进行 `package` 和 `path` 的解析/格式化
- `path` 字段支持内连 npm 包获取入口
- 单测调整

注：Framework 因为机制有待讨论，暂未修改 cc @JerrysShan 

Patch:
diff --git a/src/constant.ts b/src/constant.ts
index 12acee3..c2143f2 100644
--- a/src/constant.ts
+++ b/src/constant.ts
@@ -53,7 +53,6 @@ export const EXCEPTION_FILENAME = 'exception.json';
 export const DEFAULT_LOADER_LIST_WITH_ORDER = [
   'exception',
   'exception-filter',
-  'plugin-config',
   'plugin-meta',
   'framework-config',
   'package-json',
diff --git a/src/loader/impl/config.ts b/src/loader/impl/config.ts
index 336757c..9c597f2 100644
--- a/src/loader/impl/config.ts
+++ b/src/loader/impl/config.ts
@@ -1,13 +1,14 @@
 import * as path from 'path';
 import { Container } from '@artus/injection';
 import ConfigurationHandler from '../../configuration';
-import { ArtusInjectEnum, PLUGIN_CONFIG_PATTERN, FRAMEWORK_PATTERN } from '../../constant';
+import { ArtusInjectEnum, FRAMEWORK_PATTERN } from '../../constant';
 import { DefineLoader } from '../decorator';
 import { ManifestItem, Loader, LoaderFindOptions } from '../types';
 import compatibleRequire from '../../utils/compatible_require';
 import { isMatch } from '../../utils';
 import { Application } from '../../types';
 import { getConfigMetaFromFilename } from '../utils/config_file_meta';
+import { PluginFactory } from '../../plugin';
 
 @DefineLoader('config')
 class ConfigLoader implements Loader {
@@ -28,7 +29,6 @@ class ConfigLoader implements Loader {
   static async is(opts: LoaderFindOptions): Promise<boolean> {
     return (
       this.isConfigDir(opts) &&
-      !isMatch(opts.filename, PLUGIN_CONFIG_PATTERN) &&
       !isMatch(opts.filename, FRAMEWORK_PATTERN)
     );
   }
@@ -41,7 +41,11 @@ class ConfigLoader implements Loader {
   async load(item: ManifestItem) {
     const { namespace, env } = getConfigMetaFromFilename(item.filename);
     let configObj = await this.loadConfigFile(item);
-    if (namespace) {
+    if (namespace === 'plugin') {
+      configObj = {
+        plugin: await PluginFactory.formatPluginConfig(configObj, item),
+      };
+    } else if (namespace) {
       configObj = {
         [namespace]: configObj,
       };
diff --git a/src/loader/impl/index.ts b/src/loader/impl/index.ts
index ec0e867..5a1b8e6 100644
--- a/src/loader/impl/index.ts
+++ b/src/loader/impl/index.ts
@@ -4,7 +4,6 @@ import ExceptionLoader from './exception';
 import ExceptionFilterLoader from './exception_filter';
 import LifecycleLoader from './lifecycle';
 import PluginMetaLoader from './plugin_meta';
-import PluginConfigLoader from './plugin_config';
 import FrameworkConfigLoader from './framework_config';
 import PackageLoader from './package';
 
@@ -15,7 +14,6 @@ export {
   ExceptionFilterLoader,
   LifecycleLoader,
   PluginMetaLoader,
-  PluginConfigLoader,
   FrameworkConfigLoader,
   PackageLoader,
 };
diff --git a/src/loader/impl/plugin_config.ts b/src/loader/impl/plugin_config.ts
deleted file mode 100644
index cbed336..0000000
--- a/src/loader/impl/plugin_config.ts
+++ /dev/null
@@ -1,44 +0,0 @@
-import { PLUGIN_CONFIG_PATTERN } from '../../constant';
-import { getPackagePath } from '../../plugin/common';
-import { PluginConfigItem } from '../../plugin/types';
-import { isMatch } from '../../utils';
-import { DefineLoader } from '../decorator';
-import { ManifestItem, Loader, LoaderFindOptions } from '../types';
-import { getConfigMetaFromFilename } from '../utils/config_file_meta';
-import ConfigLoader from './config';
-
-@DefineLoader('plugin-config')
-class PluginConfigLoader extends ConfigLoader implements Loader {
-  static async is(opts: LoaderFindOptions): Promise<boolean> {
-    if (this.isConfigDir(opts)) {
-      return isMatch(opts.filename, PLUGIN_CONFIG_PATTERN);
-    }
-    return false;
-  }
-
-  async load(item: ManifestItem) {
-    const { env } = getConfigMetaFromFilename(item.filename);
-    const configObj = await this.loadConfigFile(item);
-    for (const pluginName of Object.keys(configObj)) {
-      const pluginConfigItem: PluginConfigItem = configObj[pluginName];
-      if (pluginConfigItem.package) {
-        // convert package to path when load plugin config
-        if (pluginConfigItem.path) {
-          throw new Error(
-            `Plugin ${pluginName} config can't have both package and path at ${item.path}`,
-          );
-        }
-        const loaderState = item.loaderState as { baseDir: string };
-        pluginConfigItem.path = getPackagePath(pluginConfigItem.package, [loaderState?.baseDir]);
-        delete pluginConfigItem.package;
-        configObj[pluginName] = pluginConfigItem;
-      }
-    }
-    this.configurationHandler.setConfig(env, {
-      plugin: configObj,
-    });
-    return configObj;
-  }
-}
-
-export default PluginConfigLoader;
diff --git a/src/plugin/common.ts b/src/plugin/common.ts
index e9362e8..b1f5bca 100644
--- a/src/plugin/common.ts
+++ b/src/plugin/common.ts
@@ -1,4 +1,5 @@
 import path from 'path';
+import compatibleRequire from '../utils/compatible_require';
 import { PluginType } from './types';
 
 // A utils function that toplogical sort plugins
@@ -38,3 +39,22 @@ export function getPackagePath(packageName: string, paths?: string[]): string {
   const opts = paths ? { paths } : undefined;
   return path.resolve(require.resolve(packageName, opts), '..');
 }
+
+export async function getInlinePackageEntryPath(packagePath: string): Promise<string> {
+  const pkgJson = await compatibleRequire(`${packagePath}/package.json`);
+  let entryFilePath = '';
+  if (pkgJson.exports) {
+    if (Array.isArray(pkgJson.exports)) {
+      throw new Error(`inline package multi exports is not supported`);
+    } else if (typeof pkgJson.exports === 'string') {
+      entryFilePath = pkgJson.exports;
+    } else if (pkgJson.exports?.['.']) {
+      entryFilePath = pkgJson.exports['.'];
+    }
+  }
+  if (!entryFilePath && pkgJson.main) {
+    entryFilePath = pkgJson.main;
+  }
+  // will use package root path if no entry file found
+  return entryFilePath ? path.resolve(packagePath, entryFilePath, '..') : packagePath;
+}
diff --git a/src/plugin/factory.ts b/src/plugin/factory.ts
index e8cdd42..e529013 100644
--- a/src/plugin/factory.ts
+++ b/src/plugin/factory.ts
@@ -1,6 +1,9 @@
-import { topologicalSort } from './common';
+import path from 'path';
+import { ManifestItem } from '../loader';
+import { getInlinePackageEntryPath, getPackagePath, topologicalSort } from './common';
 import { Plugin } from './impl';
 import { PluginConfigItem, PluginCreateOptions, PluginMap, PluginType } from './types';
+import { exists } from '../utils/fs';
 
 export class PluginFactory {
   static async create(name: string, item: PluginConfigItem, opts?: PluginCreateOptions): Promise<PluginType> {
@@ -30,4 +33,27 @@ export class PluginFactory {
     }
     return pluginSortResult.map(name => pluginInstanceMap.get(name)!);
   }
+
+  static async formatPluginConfig(config: Record<string, PluginConfigItem>, manifestItem?: ManifestItem): Promise<Record<string, PluginConfigItem>> {
+    const newConfig: Record<string, PluginConfigItem> = {};
+    const loaderState = manifestItem?.loaderState as { baseDir: string };
+    for (const pluginName of Object.keys(config)) {
+      const pluginConfigItem: PluginConfigItem = config[pluginName];
+      if (pluginConfigItem.package) {
+        // convert package to path when load plugin config
+        if (pluginConfigItem.path) {
+          throw new Error(
+            `Plugin ${pluginName} config can't have both package and path at ${manifestItem?.path ?? 'UNKNOWN_PATH'}`,
+          );
+        }
+        pluginConfigItem.path = getPackagePath(pluginConfigItem.package, [loaderState?.baseDir]);
+        delete pluginConfigItem.package;
+      } else if (pluginConfigItem.path && await exists(path.resolve(pluginConfigItem.path, 'package.json'))) {
+        // plugin path is a npm package, need resolve main file
+        pluginConfigItem.path = await getInlinePackageEntryPath(pluginConfigItem.path);
+      }
+      newConfig[pluginName] = pluginConfigItem;
+    }
+    return newConfig;
+  }
 }
diff --git a/src/scanner/scan.ts b/src/scanner/scan.ts
index 456abab..4b37181 100644
--- a/src/scanner/scan.ts
+++ b/src/scanner/scan.ts
@@ -280,11 +280,6 @@ export class Scanner {
     }
     relative && items.forEach(item => (item.path = path.relative(appRoot, item.path)));
     return items.filter(item => {
-      // remove PluginConfig to avoid re-merge on application running
-      if (item.loader === 'plugin-config') {
-        return false;
-      }
-
       // remove other env config
       if (item.loader === 'config' || item.loader === 'framework-config') {
         const { env: filenameEnv } = getConfigMetaFromFilename(item.filename);


Test patch:
diff --git a/test/fixtures/application_specific/src/controller/conifg.ts b/test/fixtures/application_specific/src/controller/config.ts
similarity index 90%
rename from test/fixtures/application_specific/src/controller/conifg.ts
rename to test/fixtures/application_specific/src/controller/config.ts
index be9cca0..349c683 100644
--- a/test/fixtures/application_specific/src/controller/conifg.ts
+++ b/test/fixtures/application_specific/src/controller/config.ts
@@ -10,7 +10,7 @@ export default class Hello {
   async index(ctx: Context) {
     const { params: { config } } = ctx.input;
     return {
-      message: `get conifg succeed`,
+      message: `get config succeed`,
       config,
     };
   }
diff --git a/test/fixtures/artus_application/src/controller/conifg.ts b/test/fixtures/artus_application/src/controller/config.ts
similarity index 90%
rename from test/fixtures/artus_application/src/controller/conifg.ts
rename to test/fixtures/artus_application/src/controller/config.ts
index be9cca0..349c683 100644
--- a/test/fixtures/artus_application/src/controller/conifg.ts
+++ b/test/fixtures/artus_application/src/controller/config.ts
@@ -10,7 +10,7 @@ export default class Hello {
   async index(ctx: Context) {
     const { params: { config } } = ctx.input;
     return {
-      message: `get conifg succeed`,
+      message: `get config succeed`,
       config,
     };
   }
diff --git a/test/fixtures/plugins/plugin_with_entry_a/mock_lib/index.js b/test/fixtures/plugins/plugin_with_entry_a/mock_lib/index.js
new file mode 100644
index 0000000..e69de29
diff --git a/test/fixtures/plugins/plugin_with_entry_a/mock_lib/meta.json b/test/fixtures/plugins/plugin_with_entry_a/mock_lib/meta.json
new file mode 100644
index 0000000..f192413
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_a/mock_lib/meta.json
@@ -0,0 +1,3 @@
+{
+    "name": "plugin-with-entry-a"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_a/package.json b/test/fixtures/plugins/plugin_with_entry_a/package.json
new file mode 100644
index 0000000..215546e
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_a/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "@artus/test-plugin-with-entry-a",
+  "main": "mock_lib/index.js"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_b/mock_lib/index.js b/test/fixtures/plugins/plugin_with_entry_b/mock_lib/index.js
new file mode 100644
index 0000000..e69de29
diff --git a/test/fixtures/plugins/plugin_with_entry_b/mock_lib/meta.json b/test/fixtures/plugins/plugin_with_entry_b/mock_lib/meta.json
new file mode 100644
index 0000000..2dcd5c5
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_b/mock_lib/meta.json
@@ -0,0 +1,3 @@
+{
+    "name": "plugin-with-entry-b"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_b/package.json b/test/fixtures/plugins/plugin_with_entry_b/package.json
new file mode 100644
index 0000000..f62a509
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_b/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "@artus/test-plugin-with-entry-b",
+  "exports": "./mock_lib/index.js",
+  "main": "fake/index.js"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_c/mock_lib/index.js b/test/fixtures/plugins/plugin_with_entry_c/mock_lib/index.js
new file mode 100644
index 0000000..e69de29
diff --git a/test/fixtures/plugins/plugin_with_entry_c/mock_lib/meta.json b/test/fixtures/plugins/plugin_with_entry_c/mock_lib/meta.json
new file mode 100644
index 0000000..6680ac9
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_c/mock_lib/meta.json
@@ -0,0 +1,3 @@
+{
+    "name": "plugin-with-entry-c"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_c/package.json b/test/fixtures/plugins/plugin_with_entry_c/package.json
new file mode 100644
index 0000000..c87434e
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_c/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "@artus/test-plugin-with-entry-c",
+  "exports": {
+    ".": "./mock_lib/index.js"
+  },
+  "main": "fake/index.js"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_wrong/mock_lib/index.js b/test/fixtures/plugins/plugin_with_entry_wrong/mock_lib/index.js
new file mode 100644
index 0000000..e69de29
diff --git a/test/fixtures/plugins/plugin_with_entry_wrong/mock_lib/meta.json b/test/fixtures/plugins/plugin_with_entry_wrong/mock_lib/meta.json
new file mode 100644
index 0000000..867c31b
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_wrong/mock_lib/meta.json
@@ -0,0 +1,3 @@
+{
+    "name": "plugin-with-entry-wrong"
+}
\ No newline at end of file
diff --git a/test/fixtures/plugins/plugin_with_entry_wrong/package.json b/test/fixtures/plugins/plugin_with_entry_wrong/package.json
new file mode 100644
index 0000000..d8f8f75
--- /dev/null
+++ b/test/fixtures/plugins/plugin_with_entry_wrong/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "@artus/test-plugin-with-entry-wrong",
+  "exports": [
+    "./mock_lib/index.js",
+    "./mock_lib/index.json"
+  ]
+}
\ No newline at end of file
diff --git a/test/framework.test.ts b/test/framework.test.ts
index 376fb0e..902700a 100644
--- a/test/framework.test.ts
+++ b/test/framework.test.ts
@@ -38,7 +38,7 @@ describe('test/framework.test.ts', () => {
     //  check config loaded succeed
     const testResponseConfig = await axios.get(`http://127.0.0.1:${port}/config`);
     assert(testResponseConfig.status === 200);
-    assert(testResponseConfig.data.message === 'get conifg succeed');
+    assert(testResponseConfig.data.message === 'get config succeed');
 
     // check frameworke used as env
     const testResponseName2 = await axios.get(`http://127.0.0.1:${port}/get_name2`);
diff --git a/test/plugin.test.ts b/test/plugin.test.ts
index cbe9a41..6670da8 100644
--- a/test/plugin.test.ts
+++ b/test/plugin.test.ts
@@ -207,4 +207,39 @@ describe('test/app.test.ts', () => {
       console.warn = originWarn;
     });
   });
+ 
+  describe('format plugin config', () => { 
+    it('should format plugin with entry in package.json', async () => {
+      const mockPluginConfig = {
+        'plugin-with-entry-a': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_a`),
+        },
+        'plugin-with-entry-b': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_b`),
+        },
+        'plugin-with-entry-c': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_c`),
+        },
+      };
+      const formattedPluginConfigList = await PluginFactory.formatPluginConfig(mockPluginConfig);
+      const pluginList = await PluginFactory.createFromConfig(formattedPluginConfigList);
+      expect(pluginList.length).toBe(3);
+      for (const pluginItem of pluginList) {
+        expect(pluginItem.importPath).toBe(path.resolve(__dirname, pluginPrefix, pluginItem.name.replaceAll('-', '_') , 'mock_lib'));
+      }
+    });
+
+    it('should throw error if multi exports entry in package.json', async () => {
+      const mockPluginConfig = {
+        'plugin-with-entry-wrong': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_wrong`),
+        },
+      };
+      expect(() => PluginFactory.formatPluginConfig(mockPluginConfig)).rejects.toThrowError('inline package multi exports is not supported');
+    });
+  });
 });
diff --git a/test/scanner.test.ts b/test/scanner.test.ts
index 489f665..beaeb18 100644
--- a/test/scanner.test.ts
+++ b/test/scanner.test.ts
@@ -13,21 +13,19 @@ describe('test/scanner.test.ts', () => {
     expect(manifest).toBeDefined();
     expect(manifest.items).toBeDefined();
     // console.log('manifest', manifest);
-    expect(manifest.items.length).toBe(11);
+    expect(manifest.items.length).toBe(12);
 
     expect(manifest.items.find(item => item.filename === 'not_to_be_scanned_file.ts')).toBeFalsy();
-
-    expect(manifest.items.filter(item => item.loader === 'plugin-config').length).toBe(0);
     expect(manifest.items.filter(item => item.loader === 'plugin-meta').length).toBe(1);
     expect(manifest.items.filter(item => item.loader === 'exception').length).toBe(1);
     expect(manifest.items.filter(item => item.loader === 'exception-filter').length).toBe(1);
     expect(manifest.items.filter(item => item.loader === 'lifecycle-hook-unit').length).toBe(2);
-    expect(manifest.items.filter(item => item.loader === 'config').length).toBe(1);
+    expect(manifest.items.filter(item => item.loader === 'config').length).toBe(2);
     expect(manifest.items.filter(item => item.loader === 'module').length).toBe(4);
 
     expect(manifest.items.filter(item => item.unitName === 'redis').length).toBe(2);
     expect(manifest.items.filter(item => item.unitName === 'mysql').length).toBe(0);
-    expect(manifest.items.filter(item => item.source === 'app').length).toBe(9);
+    expect(manifest.items.filter(item => item.source === 'app').length).toBe(10);
     expect(manifest.pluginConfig).toStrictEqual({
       redis: { enable: true, path: path.join('src', 'redis_plugin') },
       mysql: { enable: false, path: path.join('src', 'mysql_plugin') },
@@ -38,8 +36,8 @@ describe('test/scanner.test.ts', () => {
     // console.log('devManifest', devManifest);
     expect(devManifest).toBeDefined();
     expect(devManifest.items).toBeDefined();
-    expect(devManifest.items.length).toBe(13);
-    expect(devManifest.items.filter(item => item.loader === 'config').length).toBe(2);
+    expect(devManifest.items.length).toBe(15);
+    expect(devManifest.items.filter(item => item.loader === 'config').length).toBe(4);
     expect(devManifest.items.filter(item => item.loader === 'plugin-meta').length).toBe(2);
     expect(devManifest.items.find(item => item.unitName === 'testDuplicate')).toBeDefined();
     expect(devManifest.pluginConfig).toStrictEqual({
diff --git a/test/specific.test.ts b/test/specific.test.ts
index 68b85f8..df76a60 100644
--- a/test/specific.test.ts
+++ b/test/specific.test.ts
@@ -45,7 +45,7 @@ describe('test/specific.test.ts', () => {
       //  check config loaded succeed
       const testResponseConfig = await axios.get(`http://127.0.0.1:${port}/config`);
       assert(testResponseConfig.status === 200);
-      assert(testResponseConfig.data.message === 'get conifg succeed');
+      assert(testResponseConfig.data.message === 'get config succeed');
 
       // check frameworke used as env
       const testResponseName2 = await axios.get(`http://127.0.0.1:${port}/get_name2`);
@@ -111,7 +111,7 @@ describe('test/specific.test.ts', () => {
       //  check config loaded succeed
       const testResponseConfig = await axios.get(`http://127.0.0.1:${port}/config`);
       assert(testResponseConfig.status === 200);
-      assert(testResponseConfig.data.message === 'get conifg succeed');
+      assert(testResponseConfig.data.message === 'get config succeed');
 
       // check frameworke used as env
       const testResponseName2 = await axios.get(`http://127.0.0.1:${port}/get_name2`);