diff --git a/src/Microsoft.AspNetCore.SpaServices/npm/aspnet-prerendering/package.json b/src/Microsoft.AspNetCore.SpaServices/npm/aspnet-prerendering/package.json index 2c0651b7..11bd4ecb 100644 --- a/src/Microsoft.AspNetCore.SpaServices/npm/aspnet-prerendering/package.json +++ b/src/Microsoft.AspNetCore.SpaServices/npm/aspnet-prerendering/package.json @@ -1,6 +1,6 @@ { "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.", "main": "index.js", "scripts": { @@ -10,7 +10,7 @@ "author": "Microsoft", "license": "Apache-2.0", "dependencies": { - "domain-task": "^1.0.1", + "domain-task": "^2.0.1", "es6-promise": "^3.1.2" }, "devDependencies": { diff --git a/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/package.json b/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/package.json index 02174e89..0bceb713 100644 --- a/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/package.json +++ b/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/package.json @@ -1,8 +1,8 @@ { "name": "domain-task", - "version": "1.0.1", + "version": "2.0.1", "description": "Tracks outstanding operations for a logical thread of execution", - "main": "main.js", + "main": "index.js", "scripts": { "prepublish": "tsd update && tsc && echo 'Finished building NPM package \"domain-task\"'", "test": "echo \"Error: no test specified\" && exit 1" diff --git a/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/fetch.ts b/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/fetch.ts index 9763a007..be081f04 100644 --- a/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/fetch.ts +++ b/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/fetch.ts @@ -1,7 +1,6 @@ import * as url from 'url'; import * as domain from 'domain'; import * as domainContext from 'domain-context'; -import { addTask } from './main'; const isomorphicFetch = require('isomorphic-fetch'); 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 { - const promise = issueRequest(baseUrl(), url, init); - addTask(promise); - return promise; + // As of domain-task 2.0.0, we no longer auto-add the 'fetch' promise to the current domain task list. + // This is because it's misleading to do so, and can result in race-condition bugs, e.g., + // 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 { diff --git a/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/index.ts b/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/index.ts new file mode 100644 index 00000000..78f0c17c --- /dev/null +++ b/src/Microsoft.AspNetCore.SpaServices/npm/domain-task/src/index.ts @@ -0,0 +1,3 @@ +// This file determines the top-level package exports +export { addTask, run } from './main'; +export { fetch } from './fetch';