fix(plugin): optional plugin also need to calc order (#256)

* fix(plugin): optional plugin also need to calc order

* test: update snapshot

Author: noahziheng

src/plugin/impl.ts

@@ -51,21 +51,29 @@ export class Plugin implements PluginType {
   }
 
   public checkDepExisted(pluginMap: PluginMap) {
-    for (const { name: pluginName, optional } of this.metadata.dependencies ?? []) {
+    if (!this.metadata.dependencies) {
+      return;
+    }
+
+    for (let i = 0; i < this.metadata.dependencies.length; i++) {
+      const { name: pluginName, optional } = this.metadata.dependencies[i];
       const instance = pluginMap.get(pluginName);
       if (!instance || !instance.enable) {
         if (optional) {
           this.logger?.warn(`Plugin ${this.name} need have optional dependency: ${pluginName}.`);
         } else {
           throw new Error(`Plugin ${this.name} need have dependency: ${pluginName}.`);
         }
+      } else {
+        // Plugin exist and enabled, need calc edge
+        this.metadata.dependencies[i]._enabled = true;
       }
     }
   }
 
   public getDepEdgeList(): [string, string][] {
     return this.metadata.dependencies
-      ?.filter(({ optional }) => !optional)
+      ?.filter(({ optional, _enabled }) => !optional || _enabled)
       ?.map(({ name: depPluginName }) => [this.name, depPluginName]) ?? [];
   }
 

src/plugin/types.ts

@@ -15,6 +15,9 @@ export interface PluginMetadata {
 export interface PluginDependencyItem {
   name: string;
   optional?: boolean;
+
+  // Only exist on runtime, cannot config in meta.json
+  _enabled?: boolean;
 }
 
 export interface PluginConfigItem {

test/__snapshots__/scanner.test.ts.snap

@@ -278,7 +278,7 @@ Object {
       "pluginMetadata": Object {
         "dependencies": Array [
           Object {
-            "name": "plugin-e",
+            "name": "plugin-c",
             "optional": true,
           },
         ],
@@ -508,7 +508,7 @@ Object {
       "pluginMetadata": Object {
         "dependencies": Array [
           Object {
-            "name": "plugin-e",
+            "name": "plugin-c",
             "optional": true,
           },
         ],

test/fixtures/plugins/plugin_d/meta.json

@@ -2,7 +2,7 @@
   "name": "plugin-d",
   "dependencies": [
       {
-          "name": "plugin-e",
+          "name": "plugin-c",
           "optional": true
       }
   ]

test/plugin.test.ts

@@ -201,10 +201,91 @@ describe('test/app.test.ts', () => {
         expect(plugin).toBeInstanceOf(Plugin);
         expect(plugin.enable).toBeTruthy();
       });
-      expect(mockWarnFn).toBeCalledWith(`Plugin plugin-d need have optional dependency: plugin-e.`);
+      expect(mockWarnFn).toBeCalledWith(`Plugin plugin-d need have optional dependency: plugin-c.`);
 
       // restore warn
       console.warn = originWarn;
     });
+
+    it('should not throw if optional dependence disabled', async () => {
+      const mockPluginConfig = {
+        'plugin-c': {
+          enable: false,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
+          manifest: {
+            pluginMeta: {
+              path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
+              extname: '.js',
+              filename: 'meta.js',
+            },
+          },
+        },
+        'plugin-d': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
+          manifest: {
+            pluginMeta: {
+              path: path.resolve(__dirname, `${pluginPrefix}/plugin_d/meta.js`),
+              extname: '.js',
+              filename: 'meta.js',
+            },
+          },
+        },
+      };
+
+      // mock warn
+      const originWarn = console.warn;
+      const mockWarnFn = jest.fn();
+      console.warn = mockWarnFn;
+      const pluginList = await PluginFactory.createFromConfig(mockPluginConfig, {
+        logger: new Logger(),
+      });
+      expect(pluginList.length).toEqual(1);
+      pluginList.forEach(plugin => {
+        expect(plugin).toBeInstanceOf(Plugin);
+        expect(plugin.enable).toBeTruthy();
+      });
+      expect(mockWarnFn).toBeCalledWith(`Plugin plugin-d need have optional dependency: plugin-c.`);
+
+      // restore warn
+      console.warn = originWarn;
+    });
+
+    it('should calc order if optional dependence enabled', async () => {
+      const mockPluginConfig = {
+        'plugin-d': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
+          manifest: {
+            pluginMeta: {
+              path: path.resolve(__dirname, `${pluginPrefix}/plugin_d/meta.js`),
+              extname: '.js',
+              filename: 'meta.js',
+            },
+          },
+        },
+        'plugin-c': {
+          enable: true,
+          path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
+          manifest: {
+            pluginMeta: {
+              path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
+              extname: '.js',
+              filename: 'meta.js',
+            },
+          },
+        },
+      };
+
+      const pluginList = await PluginFactory.createFromConfig(mockPluginConfig, {
+        logger: new Logger(),
+      });
+      expect(pluginList.length).toEqual(2);
+      pluginList.forEach(plugin => {
+        expect(plugin).toBeInstanceOf(Plugin);
+        expect(plugin.enable).toBeTruthy();
+      });
+      expect(pluginList.map(plugin => plugin.name)).toStrictEqual(['plugin-c', 'plugin-d']);
+    });
   });
 });