-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[refactor] Common function for Ember-App instance destruction #93
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,6 +209,34 @@ class EmberApp { | |
}); | ||
} | ||
|
||
/** | ||
* @private | ||
* | ||
* Creates a destroy function to destroy the created `ApplicationInstance`. | ||
* | ||
* @param {Object} The Result object that contains the application instance to be destroyed. | ||
* @param {Integer} How long to wait before manually destroying the application instance. Ignored if its value is 0. | ||
* | ||
* @returns {Function} The destructor function. | ||
*/ | ||
destroyAppInstance(result, timeout) { | ||
const instance = result.instance; | ||
|
||
if (!timeout) return () => instance.destroy(); | ||
|
||
// start a timer to destroy the appInstance forcefully in the given ms. | ||
// This is a failure mechanism so that node process doesn't get wedged if the `visit` never completes. | ||
const destructionTimer = setTimeout(function() { | ||
instance.destroy(); | ||
result.error = new Error(`App instance was forcefully destroyed in ${timeout}ms`); | ||
}, timeout); | ||
|
||
return () => { | ||
clearTimeout(destructionTimer); | ||
instance.destroy(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to make sure we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohoo sorry this will work. I had only added tests for this. You can drop this. |
||
}; | ||
} | ||
|
||
/** | ||
* @private | ||
* | ||
|
@@ -221,25 +249,22 @@ class EmberApp { | |
* @param {Object} bootOptions An object containing the boot options that are used by | ||
* by ember to decide whether it needs to do rendering or not. | ||
* @param {Object} result | ||
* @param {Integer} How long to wait before manually destroying the application instance. Ignored if its value is 0. | ||
* | ||
* @return {Promise<instance>} instance | ||
*/ | ||
visitRoute(path, fastbootInfo, bootOptions, result) { | ||
let instance; | ||
|
||
visitRoute(path, fastbootInfo, bootOptions, result, destroyAppInstanceInMs) { | ||
return this.buildAppInstance() | ||
.then(appInstance => { | ||
instance = appInstance; | ||
result.instance = instance; | ||
registerFastBootInfo(fastbootInfo, instance); | ||
|
||
return instance.boot(bootOptions); | ||
result.instance = appInstance; | ||
registerFastBootInfo(fastbootInfo, appInstance); | ||
result.destroyInstance = this.destroyAppInstance(result, destroyAppInstanceInMs); | ||
return appInstance.boot(bootOptions); | ||
}) | ||
.then(() => result.instanceBooted = true) | ||
.then(() => instance.visit(path, bootOptions)) | ||
.then(() => waitForApp(instance)) | ||
.then(() => { | ||
return instance; | ||
}); | ||
.then(() => result.instance.visit(path, bootOptions)) | ||
.then(() => waitForApp(result.instance)) | ||
.then(() => result.instance); | ||
} | ||
|
||
/** | ||
|
@@ -269,7 +294,7 @@ class EmberApp { | |
let res = options.response; | ||
let html = options.html || this.html; | ||
let disableShoebox = options.disableShoebox || false; | ||
let destroyAppInstanceInMs = options.destroyAppInstanceInMs; | ||
let destroyAppInstanceInMs = parseInt(options.destroyAppInstanceInMs, 10) || 0; | ||
|
||
let shouldRender = (options.shouldRender !== undefined) ? options.shouldRender : true; | ||
let bootOptions = buildBootOptions(shouldRender); | ||
|
@@ -287,42 +312,20 @@ class EmberApp { | |
fastbootInfo: fastbootInfo | ||
}); | ||
|
||
let destroyAppInstanceTimer; | ||
if (parseInt(destroyAppInstanceInMs, 10) > 0) { | ||
// start a timer to destroy the appInstance forcefully in the given ms. | ||
// This is a failure mechanism so that node process doesn't get wedged if the `visit` never completes. | ||
destroyAppInstanceTimer = setTimeout(function() { | ||
if (instance && !result.instanceDestroyed) { | ||
result.instanceDestroyed = true; | ||
result.error = new Error('App instance was forcefully destroyed in ' + destroyAppInstanceInMs + 'ms'); | ||
instance.destroy(); | ||
} | ||
}, destroyAppInstanceInMs); | ||
} | ||
|
||
let instance; | ||
return this.visitRoute(path, fastbootInfo, bootOptions, result) | ||
.then(appInstance => { | ||
instance = appInstance; | ||
}) | ||
return this.visitRoute(path, fastbootInfo, bootOptions, result, destroyAppInstanceInMs) | ||
.then(() => result.instanceBooted = true) | ||
.then(() => result.instance.visit(path, bootOptions)) | ||
.then(() => waitForApp(result.instance)) | ||
.then(() => { | ||
if (!disableShoebox) { | ||
// if shoebox is not disabled, then create the shoebox and send API data | ||
createShoebox(doc, fastbootInfo); | ||
} | ||
}) | ||
.catch(error => result.error = error) | ||
.catch(error => result.error = result.error || error) | ||
.then(() => result._finalize()) | ||
.finally(() => { | ||
if (instance && !result.instanceDestroyed) { | ||
result.instanceDestroyed = true; | ||
instance.destroy(); | ||
|
||
if (destroyAppInstanceTimer) { | ||
clearTimeout(destroyAppInstanceTimer); | ||
} | ||
} | ||
}); | ||
.finally(() => result.destroyInstance()); | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add some comments to this function just like other functions have. Makes it easier to generate an API at a later point.