[DO NOT MERGE] code review

eggjs · Aug 17, 2016 · c92ea85 · c92ea85
1 parent 35ac53d
commit c92ea85
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 0 deletions.
diff --git a/lib/egg.js b/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;
 
     /**

diff --git a/lib/loader/egg_loader.js b/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',

diff --git a/lib/loader/file_loader.js b/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');
 

diff --git a/lib/loader/mixin/plugin.js b/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

diff --git a/lib/loader/mixin/router.js b/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'));
   },
 };