fix: fix ready callback id (#214)

killagu · web-flow · commit 77e11f5fb38b · 2019-06-21T10:32:55.000+08:00
diff --git a/.travis.yml b/.travis.yml
@@ -2,7 +2,6 @@ sudo: false
 os:
   - linux
   - osx
-  - windows
 language: node_js
 node_js:
   - '8'
@@ -13,4 +12,4 @@ install:
 script:
   - npm run ci
 after_success:
-  - npminstall codecov && codecov --disable=gcov -f .nyc_output/*.json
+  - npminstall codecov && codecov --disable=gcov
diff --git a/lib/egg.js b/lib/egg.js
@@ -21,7 +21,7 @@ const CLOSE_PROMISE = Symbol('EggCore#closePromise');
 class EggCore extends KoaApplication {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options - options
    * @param {String} [options.baseDir=process.cwd()] - the directory of application
    * @param {String} [options.type=application|agent] - whether it's running in app worker or agent worker
@@ -221,7 +221,7 @@ class EggCore extends KoaApplication {
    * register an callback function that will be invoked when application is ready.
    * @see https://github.com/node-modules/ready
    * @since 1.0.0
-   * @param {boolean|Error|Function} flagOrFunction -
+   * @param {boolean|Error|Function} [flagOrFunction] -
    * @return {Promise|null} return promise when argument is undefined
    * @example
    * const app = new Application(...);
diff --git a/lib/lifecycle.js b/lib/lifecycle.js
@@ -105,7 +105,11 @@ class Lifecycle extends EventEmitter {
   }
 
   registerBeforeStart(scope) {
-    this[REGISTER_READY_CALLBACK](scope, this.loadReady, 'Before Start');
+    this[REGISTER_READY_CALLBACK]({
+      scope,
+      ready: this.loadReady,
+      timingKeyPrefix: 'Before Start',
+    });
   }
 
   registerBeforeClose(fn) {
@@ -151,7 +155,12 @@ class Lifecycle extends EventEmitter {
     for (const boot of this[BOOTS]) {
       const didLoad = boot.didLoad && boot.didLoad.bind(boot);
       if (didLoad) {
-        this[REGISTER_READY_CALLBACK](didLoad, this.loadReady, 'Did Load');
+        this[REGISTER_READY_CALLBACK]({
+          scope: didLoad,
+          ready: this.loadReady,
+          timingKeyPrefix: 'Did Load',
+          scopeFullName: boot.fullPath + ':didLoad',
+        });
       }
     }
   }
@@ -162,7 +171,12 @@ class Lifecycle extends EventEmitter {
     for (const boot of this[BOOTS]) {
       const willReady = boot.willReady && boot.willReady.bind(boot);
       if (willReady) {
-        this[REGISTER_READY_CALLBACK](willReady, this.bootReady, 'Will Ready');
+        this[REGISTER_READY_CALLBACK]({
+          scope: willReady,
+          ready: this.bootReady,
+          timingKeyPrefix: 'Will Ready',
+          scopeFullName: boot.fullPath + ':willReady',
+        });
       }
     }
   }
@@ -230,13 +244,13 @@ class Lifecycle extends EventEmitter {
     }
   }
 
-  [REGISTER_READY_CALLBACK](scope, ready, timingKeyPrefix) {
+  [REGISTER_READY_CALLBACK]({ scope, ready, timingKeyPrefix, scopeFullName }) {
     if (!is.function(scope)) {
       throw new Error('boot only support function');
     }
 
-    // get filename from stack
-    const name = utils.getCalleeFromStack(true, 4);
+    // get filename from stack if scopeFullName is undefined
+    const name = scopeFullName || utils.getCalleeFromStack(true, 4);
     const timingkey = `${timingKeyPrefix} in ` + utils.getResolvedFilename(name, this.app.baseDir);
 
     this.timing.start(timingkey);
diff --git a/lib/loader/context_loader.js b/lib/loader/context_loader.js
@@ -41,7 +41,7 @@ class ClassLoader {
 class ContextLoader extends FileLoader {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options - options same as {@link FileLoader}
    * @param {String} options.fieldClass - determine the field name of inject object.
    */
diff --git a/lib/loader/egg_loader.js b/lib/loader/egg_loader.js
@@ -18,7 +18,7 @@ const REQUIRE_COUNT = Symbol('EggLoader#requireCount');
 class EggLoader {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options - options
    * @param {String} options.baseDir - the directory of application
    * @param {EggCore} options.app - Application instance
diff --git a/lib/loader/file_loader.js b/lib/loader/file_loader.js
@@ -32,7 +32,7 @@ const defaults = {
 class FileLoader {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options - options
    * @param {String|Array} options.directory - directories to be loaded
    * @param {Object} options.target - attach the target object from loaded files
diff --git a/lib/loader/mixin/config.js b/lib/loader/mixin/config.js
@@ -15,7 +15,7 @@ module.exports = {
    *
    * Will merge config.default.js 和 config.${env}.js
    *
-   * @method EggLoader#loadConfig
+   * @function EggLoader#loadConfig
    * @since 1.0.0
    */
   loadConfig() {
diff --git a/lib/loader/mixin/custom.js b/lib/loader/mixin/custom.js
@@ -59,6 +59,7 @@ module.exports = {
       }
       const bootHook = this.requireFile(bootFilePath);
       if (is.class(bootHook)) {
+        bootHook.prototype.fullPath = bootFilePath;
         // if is boot class, add to lifecycle
         this.lifecycle.addBootHook(bootHook);
       } else if (is.function(bootHook)) {
diff --git a/lib/loader/mixin/extend.js b/lib/loader/mixin/extend.js
@@ -15,7 +15,7 @@ module.exports = {
 
   /**
    * mixin Agent.prototype
-   * @method EggLoader#loadAgentExtend
+   * @function EggLoader#loadAgentExtend
    * @since 1.0.0
    */
   loadAgentExtend() {
@@ -24,7 +24,7 @@ module.exports = {
 
   /**
    * mixin Application.prototype
-   * @method EggLoader#loadApplicationExtend
+   * @function EggLoader#loadApplicationExtend
    * @since 1.0.0
    */
   loadApplicationExtend() {
@@ -33,7 +33,7 @@ module.exports = {
 
   /**
    * mixin Request.prototype
-   * @method EggLoader#loadRequestExtend
+   * @function EggLoader#loadRequestExtend
    * @since 1.0.0
    */
   loadRequestExtend() {
@@ -42,7 +42,7 @@ module.exports = {
 
   /**
    * mixin Response.prototype
-   * @method EggLoader#loadResponseExtend
+   * @function EggLoader#loadResponseExtend
    * @since 1.0.0
    */
   loadResponseExtend() {
@@ -51,7 +51,7 @@ module.exports = {
 
   /**
    * mixin Context.prototype
-   * @method EggLoader#loadContextExtend
+   * @function EggLoader#loadContextExtend
    * @since 1.0.0
    */
   loadContextExtend() {
@@ -60,7 +60,7 @@ module.exports = {
 
   /**
    * mixin app.Helper.prototype
-   * @method EggLoader#loadHelperExtend
+   * @function EggLoader#loadHelperExtend
    * @since 1.0.0
    */
   loadHelperExtend() {
@@ -83,7 +83,7 @@ module.exports = {
 
   /**
    * Loader app/extend/xx.js to `prototype`,
-   * @method loadExtend
+   * @function loadExtend
    * @param {String} name - filename which may be `app/extend/{name}.js`
    * @param {Object} proto - prototype that mixed
    * @since 1.0.0
diff --git a/lib/loader/mixin/middleware.js b/lib/loader/mixin/middleware.js
@@ -16,7 +16,7 @@ module.exports = {
    *
    * app.config.xx is the options of the middleware xx that has same name as config
    *
-   * @method EggLoader#loadMiddleware
+   * @function EggLoader#loadMiddleware
    * @param {Object} opt - LoaderOptions
    * @example
    * ```js
diff --git a/lib/loader/mixin/plugin.js b/lib/loader/mixin/plugin.js
@@ -52,7 +52,7 @@ module.exports = {
    * ```
    *
    * `loader.allPlugins` can be used when retrieve all plugins.
-   * @method EggLoader#loadPlugin
+   * @function EggLoader#loadPlugin
    * @since 1.0.0
    */
   loadPlugin() {
diff --git a/lib/loader/mixin/router.js b/lib/loader/mixin/router.js
@@ -7,7 +7,7 @@ module.exports = {
 
   /**
    * Load app/router.js
-   * @method EggLoader#loadRouter
+   * @function EggLoader#loadRouter
    * @param {Object} opt - LoaderOptions
    * @since 1.0.0
    */
diff --git a/lib/loader/mixin/service.js b/lib/loader/mixin/service.js
@@ -7,7 +7,7 @@ module.exports = {
 
   /**
    * Load app/service
-   * @method EggLoader#loadService
+   * @function EggLoader#loadService
    * @param {Object} opt - LoaderOptions
    * @since 1.0.0
    */
diff --git a/lib/utils/base_context_class.js b/lib/utils/base_context_class.js
@@ -8,7 +8,7 @@
 class BaseContextClass {
 
   /**
-   * @constructor
+   * @class
    * @param {Context} ctx - context instance
    * @since 1.0.0
    */
diff --git a/package.json b/package.json
@@ -33,10 +33,10 @@
   "ci": {
     "type": "travis",
     "os": {
-      "travis": "linux, osx, windows"
+      "travis": "linux, osx"
     },
     "version": "8, 10, 12",
-    "afterScript": "after_success:\n  - npminstall codecov && codecov --disable=gcov -f .nyc_output/*.json",
+    "afterScript": "after_success:\n  - npminstall codecov && codecov --disable=gcov",
     "license": {
       "year": 2016
     }
diff --git a/test/egg.test.js b/test/egg.test.js
@@ -855,5 +855,24 @@ describe('test/egg.test.js', () => {
         app.close();
       });
     });
+
+    describe('boot timeout', () => {
+      beforeEach(() => {
+        mm(process.env, 'EGG_READY_TIMEOUT_ENV', 1);
+      });
+
+      it('should warn write filename and function', async () => {
+        let timeoutId;
+        const app = utils.createApp('boot-timeout');
+        app.once('ready_timeout', id => {
+          timeoutId = id;
+        });
+        app.loader.loadAll();
+        await app.ready();
+        assert(timeoutId);
+        const suffix = path.normalize('test/fixtures/boot-timeout/app.js');
+        assert(timeoutId.endsWith(suffix + ':didLoad'));
+      });
+    });
   });
 });
diff --git a/test/fixtures/boot-timeout/app.js b/test/fixtures/boot-timeout/app.js
@@ -0,0 +1,9 @@
+'use strict';
+
+const sleep = require('mz-modules/sleep');
+
+module.exports = class TimeoutHook {
+  async didLoad() {
+    await sleep(10);
+  }
+};
diff --git a/test/fixtures/boot-timeout/package.json b/test/fixtures/boot-timeout/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "boot-timeout"
+}

Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,11 @@ class Lifecycle extends EventEmitter {`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`registerBeforeStart(scope) {`
`108`		`- this[REGISTER_READY_CALLBACK](scope, this.loadReady, 'Before Start');`
	`108`	`+ this[REGISTER_READY_CALLBACK]({`
	`109`	`+ scope,`
	`110`	`+ ready: this.loadReady,`
	`111`	`+ timingKeyPrefix: 'Before Start',`
	`112`	`+ });`
`109`	`113`	`}`
`110`	`114`
`111`	`115`	`registerBeforeClose(fn) {`
`@@ -151,7 +155,12 @@ class Lifecycle extends EventEmitter {`
`151`	`155`	`for (const boot of this[BOOTS]) {`
`152`	`156`	`const didLoad = boot.didLoad && boot.didLoad.bind(boot);`
`153`	`157`	`if (didLoad) {`
`154`		`- this[REGISTER_READY_CALLBACK](didLoad, this.loadReady, 'Did Load');`
	`158`	`+ this[REGISTER_READY_CALLBACK]({`
	`159`	`+ scope: didLoad,`
	`160`	`+ ready: this.loadReady,`
	`161`	`+ timingKeyPrefix: 'Did Load',`
	`162`	`+ scopeFullName: boot.fullPath + ':didLoad',`
	`163`	`+ });`
`155`	`164`	`}`
`156`	`165`	`}`
`157`	`166`	`}`
`@@ -162,7 +171,12 @@ class Lifecycle extends EventEmitter {`
`162`	`171`	`for (const boot of this[BOOTS]) {`
`163`	`172`	`const willReady = boot.willReady && boot.willReady.bind(boot);`
`164`	`173`	`if (willReady) {`
`165`		`- this[REGISTER_READY_CALLBACK](willReady, this.bootReady, 'Will Ready');`
	`174`	`+ this[REGISTER_READY_CALLBACK]({`
	`175`	`+ scope: willReady,`
	`176`	`+ ready: this.bootReady,`
	`177`	`+ timingKeyPrefix: 'Will Ready',`
	`178`	`+ scopeFullName: boot.fullPath + ':willReady',`
	`179`	`+ });`
`166`	`180`	`}`
`167`	`181`	`}`
`168`	`182`	`}`
`@@ -230,13 +244,13 @@ class Lifecycle extends EventEmitter {`
`230`	`244`	`}`
`231`	`245`	`}`
`232`	`246`
`233`		`- [REGISTER_READY_CALLBACK](scope, ready, timingKeyPrefix) {`
	`247`	`+ [REGISTER_READY_CALLBACK]({ scope, ready, timingKeyPrefix, scopeFullName }) {`
`234`	`248`	`if (!is.function(scope)) {`
`235`	`249`	`throw new Error('boot only support function');`
`236`	`250`	`}`
`237`	`251`
`238`		`- // get filename from stack`
`239`		`- const name = utils.getCalleeFromStack(true, 4);`
	`252`	`+ // get filename from stack if scopeFullName is undefined`
	`253`	`+ const name = scopeFullName \|\| utils.getCalleeFromStack(true, 4);`
`240`	`254`	const timingkey = `${timingKeyPrefix} in ` + utils.getResolvedFilename(name, this.app.baseDir);
`241`	`255`
`242`	`256`	`this.timing.start(timingkey);`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ module.exports = {`
`15`	`15`	`*`
`16`	`16`	`* Will merge config.default.js 和 config.${env}.js`
`17`	`17`	`*`
`18`		`- * @method EggLoader#loadConfig`
	`18`	`+ * @function EggLoader#loadConfig`
`19`	`19`	`* @since 1.0.0`
`20`	`20`	`*/`
`21`	`21`	`loadConfig() {`
Original file line number	Diff line number	Diff line change
`@@ -59,6 +59,7 @@ module.exports = {`
`59`	`59`	`}`
`60`	`60`	`const bootHook = this.requireFile(bootFilePath);`
`61`	`61`	`if (is.class(bootHook)) {`
	`62`	`+ bootHook.prototype.fullPath = bootFilePath;`
`62`	`63`	`// if is boot class, add to lifecycle`
`63`	`64`	`this.lifecycle.addBootHook(bootHook);`
`64`	`65`	`} else if (is.function(bootHook)) {`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ module.exports = {`
`16`	`16`	`*`
`17`	`17`	`* app.config.xx is the options of the middleware xx that has same name as config`
`18`	`18`	`*`
`19`		`- * @method EggLoader#loadMiddleware`
	`19`	`+ * @function EggLoader#loadMiddleware`
`20`	`20`	`* @param {Object} opt - LoaderOptions`
`21`	`21`	`* @example`
`22`	`22`	* ```js