fix: should not load optional plugin & their deps (#154)

lib/utils/sequencify.js

@@ -1,11 +1,11 @@
 'use strict';
 
-function sequence(tasks, names, results, missing, recursive, nest, optional) {
+const debug = require('debug')('egg-core#sequencify');
+
+function sequence(tasks, names, results, missing, recursive, nest, optional, parent) {
   names.forEach(function(name) {
-    if (results.sequence.includes(name)) {
-      results.requires[name] = results.requires[name] || !optional;
-      return; // de-dup results
-    }
+    if (results.requires[name]) return;
+
     const node = tasks[name];
 
     if (!node) {
@@ -18,15 +18,20 @@ function sequence(tasks, names, results, missing, recursive, nest, optional) {
     } else if (node.dependencies.length || node.optionalDependencies.length) {
       nest.push(name);
       if (node.dependencies.length) {
-        sequence(tasks, node.dependencies, results, missing, recursive, nest);
+        sequence(tasks, node.dependencies, results, missing, recursive, nest, optional, name);
       }
       if (node.optionalDependencies.length) {
-        sequence(tasks, node.optionalDependencies, results, missing, recursive, nest, true);
+        sequence(tasks, node.optionalDependencies, results, missing, recursive, nest, true, name);
       }
       nest.pop(name);
     }
-    results.requires[name] = results.requires[name] || !optional;
-    results.sequence.push(name);
+    if (!optional) {
+      results.requires[name] = true;
+      debug('task: %s is enabled by %s', name, parent);
+    }
+    if (!results.sequence.includes(name)) {
+      results.sequence.push(name);
+    }
   });
 }
 
@@ -40,7 +45,7 @@ module.exports = function(tasks, names) {
   const missing = []; // missing tasks
   const recursive = []; // recursive task dependencies
 
-  sequence(tasks, names, results, missing, recursive, []);
+  sequence(tasks, names, results, missing, recursive, [], false, 'app');
 
   if (missing.length || recursive.length) {
     results.sequence = []; // results are incomplete at best, completely wrong at worst, remove them to avoid confusion

package.json

@@ -35,13 +35,13 @@
   "devDependencies": {
     "autod": "^3.0.1",
     "coffee": "^4.1.0",
-    "egg-bin": "^4.3.6",
+    "egg-bin": "^4.3.7",
     "egg-ci": "^1.8.0",
-    "eslint": "^4.13.1",
-    "eslint-config-egg": "^5.1.1",
-    "js-yaml": "^3.10.0",
+    "eslint": "^4.18.2",
+    "eslint-config-egg": "^7.0.0",
+    "js-yaml": "^3.11.0",
     "mm": "^2.2.0",
-    "mz-modules": "^2.0.0",
+    "mz-modules": "^2.1.0",
     "pedding": "^1.1.0",
     "rimraf": "^2.6.2",
     "spy": "^1.0.0",
@@ -50,16 +50,16 @@
   "dependencies": {
     "co": "^4.6.0",
     "debug": "^3.1.0",
-    "depd": "^1.1.1",
-    "egg-logger": "^1.6.0",
+    "depd": "^1.1.2",
+    "egg-logger": "^1.6.1",
     "egg-path-matching": "^1.0.1",
     "extend2": "^1.0.0",
-    "globby": "^7.1.1",
+    "globby": "^8.0.1",
     "inflection": "^1.12.0",
     "is-type-of": "^1.2.0",
-    "koa": "^2.4.1",
+    "koa": "^2.5.0",
     "koa-convert": "^1.2.0",
-    "koa-router": "^7.3.0",
+    "koa-router": "^7.4.0",
     "node-homedir": "^1.1.0",
     "ready-callback": "^2.0.1",
     "utility": "^1.13.1"

test/fixtures/plugin-complex-deps/config/plugin.js

@@ -0,0 +1,18 @@
+'use strict';
+
+const path = require('path');
+
+exports.tracelog = {
+  enable: true,
+  path: path.join(__dirname, '../plugin/tracelog'),
+};
+
+exports.rpcServer = {
+  enable: false,
+  path: path.join(__dirname, '../plugin/rpc-server'),
+};
+
+exports.gw = {
+  enable: false,
+  path: path.join(__dirname, '../plugin/gw'),
+};

test/fixtures/plugin-complex-deps/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "demo"
+}

test/fixtures/plugin-complex-deps/plugin/gw/package.json

@@ -0,0 +1,8 @@
+{
+  "eggPlugin": {
+    "name": "gw",
+    "dependencies": [
+      "rpcServer"
+    ]
+  }
+}

test/fixtures/plugin-complex-deps/plugin/rpc-server/package.json

@@ -0,0 +1,5 @@
+{
+  "eggPlugin": {
+    "name": "rpcServer"
+  }
+}

test/fixtures/plugin-complex-deps/plugin/tracelog/package.json

@@ -0,0 +1,9 @@
+{
+  "eggPlugin": {
+    "name": "tracelog",
+    "optionalDependencies": [
+      "gw",
+      "rpcServer"
+    ]
+  }
+}

test/loader/file_loader.test.js

@@ -8,7 +8,7 @@ const yaml = require('js-yaml');
 const FileLoader = require('../../lib/loader/file_loader');
 const dirBase = path.join(__dirname, '../fixtures/load_dirs');
 
-describe('test/file_loader.test.js', () => {
+describe('test/loader/file_loader.test.js', () => {
   it('should load files', done => {
     const services = {};
     new FileLoader({
@@ -326,7 +326,7 @@ describe('test/file_loader.test.js', () => {
             return filepath;
           },
         }).load();
-      }, /caseStyle expect an array, but got foo_bar1.js/);
+      }, /caseStyle expect an array, but got/);
     });
 
     it('should be overridden by lowercaseFirst', () => {

test/loader/mixin/load_plugin.test.js

@@ -570,4 +570,14 @@ describe('test/load_plugin.test.js', function() {
     assert(loader.allPlugins.c.enable === false);
     assert(loader.allPlugins.d.enable === true);
   });
+
+  it('should not load optionalDependencies and their dependencies', () => {
+    mm(process.env, 'EGG_SERVER_ENV', 'default');
+    app = utils.createApp('plugin-complex-deps');
+    const loader = app.loader;
+    loader.loadPlugin();
+    assert(loader.allPlugins.tracelog.enable === true);
+    assert(loader.allPlugins.gw.enable === false);
+    assert(loader.allPlugins.rpcServer.enable === false);
+  });
 });