Update domain-task package to version 2.0.1 (major bump because breaking change) and modify 'fetch' behaviour so it no longer tries to register the task with domain-task automatically. See code comments for reasons.

This commit is contained in:
SteveSandersonMS
2016-07-11 11:42:24 +01:00
parent c1a1bdf373
commit fc897475f3
4 changed files with 34 additions and 8 deletions

View File

@@ -1,6 +1,6 @@
{ {
"name": "aspnet-prerendering", "name": "aspnet-prerendering",
"version": "1.0.2", "version": "1.0.4",
"description": "Helpers for server-side rendering of JavaScript applications in ASP.NET Core projects. Works in conjunction with the Microsoft.AspNetCore.SpaServices NuGet package.", "description": "Helpers for server-side rendering of JavaScript applications in ASP.NET Core projects. Works in conjunction with the Microsoft.AspNetCore.SpaServices NuGet package.",
"main": "index.js", "main": "index.js",
"scripts": { "scripts": {
@@ -10,7 +10,7 @@
"author": "Microsoft", "author": "Microsoft",
"license": "Apache-2.0", "license": "Apache-2.0",
"dependencies": { "dependencies": {
"domain-task": "^1.0.1", "domain-task": "^2.0.1",
"es6-promise": "^3.1.2" "es6-promise": "^3.1.2"
}, },
"devDependencies": { "devDependencies": {

View File

@@ -1,8 +1,8 @@
{ {
"name": "domain-task", "name": "domain-task",
"version": "1.0.1", "version": "2.0.1",
"description": "Tracks outstanding operations for a logical thread of execution", "description": "Tracks outstanding operations for a logical thread of execution",
"main": "main.js", "main": "index.js",
"scripts": { "scripts": {
"prepublish": "tsd update && tsc && echo 'Finished building NPM package \"domain-task\"'", "prepublish": "tsd update && tsc && echo 'Finished building NPM package \"domain-task\"'",
"test": "echo \"Error: no test specified\" && exit 1" "test": "echo \"Error: no test specified\" && exit 1"

View File

@@ -1,7 +1,6 @@
import * as url from 'url'; import * as url from 'url';
import * as domain from 'domain'; import * as domain from 'domain';
import * as domainContext from 'domain-context'; import * as domainContext from 'domain-context';
import { addTask } from './main';
const isomorphicFetch = require('isomorphic-fetch'); const isomorphicFetch = require('isomorphic-fetch');
const isBrowser: boolean = (new Function('try { return this === window; } catch (e) { return false; }'))(); const isBrowser: boolean = (new Function('try { return this === window; } catch (e) { return false; }'))();
@@ -42,9 +41,33 @@ function issueRequest(baseUrl: string, req: string | Request, init?: RequestInit
} }
export function fetch(url: string | Request, init?: RequestInit): Promise<any> { export function fetch(url: string | Request, init?: RequestInit): Promise<any> {
const promise = issueRequest(baseUrl(), url, init); // As of domain-task 2.0.0, we no longer auto-add the 'fetch' promise to the current domain task list.
addTask(promise); // This is because it's misleading to do so, and can result in race-condition bugs, e.g.,
return promise; // https://github.com/aspnet/JavaScriptServices/issues/166
//
// Consider this usage:
//
// import { fetch } from 'domain-task/fetch';
// fetch(something).then(callback1).then(callback2) ...etc... .then(data => updateCriticalAppState);
//
// If we auto-add the very first 'fetch' promise to the domain task list, then the domain task completion
// callback might fire at any point among all the chained callbacks. If there are enough chained callbacks,
// it's likely to occur before the final 'updateCriticalAppState' one. Previously we thought it was enough
// for domain-task to use setTimeout(..., 0) so that its action occurred after all synchronously-scheduled
// chained promise callbacks, but this turns out not to be the case. Current versions of Node will run
// setTimeout-scheduled callbacks *before* setImmediate ones, if their timeout has elapsed. So even if you
// use setTimeout(..., 10), then this callback will run before setImmediate(...) if there were 10ms or more
// of CPU-blocking activity. In other words, a race condition.
//
// The correct design is for the final chained promise to be the thing added to the domain task list, but
// this can only be done by the developer and not baked into the 'fetch' API. The developer needs to write
// something like:
//
// var myTask = fetch(something).then(callback1).then(callback2) ...etc... .then(data => updateCriticalAppState);
// addDomainTask(myTask);
//
// ... so that the domain-tasks-completed callback never fires until after 'updateCriticalAppState'.
return issueRequest(baseUrl(), url, init);
} }
export function baseUrl(url?: string): string { export function baseUrl(url?: string): string {

View File

@@ -0,0 +1,3 @@
// This file determines the top-level package exports
export { addTask, run } from './main';
export { fetch } from './fetch';