[DO NOT MERGE] code review

README.md

@@ -88,6 +88,7 @@ Load config/plugin.js
 
 #### loadConfig
 
+// config.default.js
 Load config/config.js and config/{serverEnv}.js
 
 #### loadController

lib/egg.js

@@ -33,6 +33,7 @@ class EggCore extends KoaApplication {
      * @member {Object} EggCore#options
      * @since 1.0.0
      */
+    // 跟 jsdoc 不符, 如果暴露给外部的, 那是不是直接挂在 this.options 就好了?
     this._options = options;
 
     /**

lib/loader/egg_loader.js

@@ -130,6 +130,7 @@ class EggLoader {
    *
    * ```
    * // lib/xx.js
+   * // 这里需要修改示例, egg-core ?
    * const egg = require('egg');
    * class XxApplication extends egg.Application {
    *   constructor(options) {
@@ -190,6 +191,7 @@ class EggLoader {
       return null;
     }
 
+    // 这里调用的是 utils.loadFile, 但同名了, 会让人以为是递归
     const ret = loadFile(filepath);
     // function(arg1, args, ...) {}
     let inject = Array.prototype.slice.call(arguments, 1);
@@ -221,6 +223,7 @@ class EggLoader {
 
     if (this.orderPlugins) {
       for (const plugin of this.orderPlugins) {
+        // 插件是在哪里定义的, 这样要不要暴露出去?
         dirs.push({
           path: plugin.path,
           type: 'plugin',

lib/loader/file_loader.js

@@ -7,6 +7,7 @@ const path = require('path');
 const globby = require('globby');
 const is = require('is-type-of');
 const loadFile = require('../utils').loadFile;
+// Symbol.for ?
 const FULLPATH = Symbol('EGG_LOADER_ITEM_FULLPATH');
 const EXPORTS = Symbol('EGG_LOADER_ITEM_EXPORTS');
 

lib/loader/mixin/config.js

@@ -54,6 +54,7 @@ module.exports = {
     this.config = target;
   },
 
+  // config 文件会加载两次, 这个到时候文档里面最好注明下
   _preloadAppConfig() {
     const names = [
       'config.default.js',

lib/loader/mixin/extend.js

@@ -76,6 +76,7 @@ module.exports = {
     const filepaths = this.getLoadUnits()
       .map(unit => {
         let pluginExtendsPath = path.join(unit.path, 'app/extend');
+        // 还需要兼容 app 目录?
         if (!fs.existsSync(pluginExtendsPath)) {
           pluginExtendsPath = path.join(unit.path, 'app');
         }

lib/loader/mixin/middleware.js

@@ -35,6 +35,7 @@ module.exports = {
       override: true,
       lowercaseFirst: true,
     }, opt);
+    // 感觉很多地方都会执行 map, 是不是加个辅助方法? getLoadDirs('app/middleware')
     const middlewarePaths = this.getLoadUnits().map(unit => join(unit.path, 'app/middleware'));
     this.loadToApp(middlewarePaths, 'middlewares', opt);
 

lib/loader/mixin/plugin.js

@@ -237,6 +237,8 @@ module.exports = {
     debug('Got plugins %j after sequencify', result);
 
     // catch error when result.sequence is empty
+    // 场景: plugin 是在应用层引入的, 开发期的 plugin 期望配置为应用的 devDeps, 当 npm install --prod 时不会被安装, 导致报错
+    // 建议: 当 plugin 是在应用层 plugin.js 中定义的, 且当前 env 下无需用到该插件, 则 skip 掉.
     if (!result.sequence.length) {
       const err = new Error(`sequencify plugins has problem, missing: [${result.missingTasks}], recursive: [${result.recursiveDependencies}]`);
       // find plugins which is required by the missing plugin

lib/loader/mixin/router.js

@@ -19,6 +19,7 @@ module.exports = {
     app.use(router.middleware());
 
     // 加载 router.js
+    // 增加对 app/router/index.js 的支持? 当 router 比较多的时候有分文件的需求
     this.loadFile(path.join(this.options.baseDir, 'app/router.js'));
   },
 };