Skip to content

Commit 260d3ed

Browse files
arturovtthePunderWoman
arturovt
authored and
thePunderWoman
committed
fix(zone.js): patch Response methods returned by fetch (#50653)
This commit updates the implementation of the `fetch` patch and additionally patches `Response` methods which return promises. These are `arrayBuffer`, `blob`, `formData`, `json` and `text`. This fixes the issue when zone becomes stable too early before all of the `fetch` tasks complete. Given the following code: ```ts appRef.isStable.subscribe(console.log); fetch(...).then(response => response.json()).then(console.log); ``` The `isStable` observer would log `false, true, false, true`. This was happening because `json()` was returning a native promise (and not a `ZoneAwarePromise`). But calling `then` on the native promise returns a `ZoneAwarePromise` which notifies Angular about the task being scheduled and forces to re-calculate the `isStable` state. Issue: #50327 PR Close #50653
1 parent f87f058 commit 260d3ed

2 files changed

Lines changed: 102 additions & 41 deletions

File tree

‎packages/zone.js/lib/common/fetch.ts‎

Lines changed: 69 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -26,61 +26,89 @@ Zone.__load_patch('fetch', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
2626
const ZoneAwarePromise = global.Promise;
2727
const symbolThenPatched = api.symbol('thenPatched');
2828
const fetchTaskScheduling = api.symbol('fetchTaskScheduling');
29+
const OriginalResponse = global.Response;
2930
const placeholder = function() {};
31+
32+
const createFetchTask =
33+
(source: string, data: TaskData|undefined, originalImpl: any, self: any, args: any[],
34+
ac?: AbortController) => new Promise((resolve, reject) => {
35+
const task = Zone.current.scheduleMacroTask(
36+
source, placeholder, data,
37+
() => {
38+
// The promise object returned by the original implementation passed into the
39+
// function. This might be a `fetch` promise, `Response.prototype.json` promise,
40+
// etc.
41+
let implPromise;
42+
let zone = Zone.current;
43+
44+
try {
45+
(zone as any)[fetchTaskScheduling] = true;
46+
implPromise = originalImpl.apply(self, args);
47+
} catch (error) {
48+
reject(error);
49+
return;
50+
} finally {
51+
(zone as any)[fetchTaskScheduling] = false;
52+
}
53+
54+
if (!(implPromise instanceof ZoneAwarePromise)) {
55+
let ctor = implPromise.constructor;
56+
if (!ctor[symbolThenPatched]) {
57+
api.patchThen(ctor);
58+
}
59+
}
60+
61+
implPromise.then(
62+
(resource: any) => {
63+
if (task.state !== 'notScheduled') {
64+
task.invoke();
65+
}
66+
resolve(resource);
67+
},
68+
(error: any) => {
69+
if (task.state !== 'notScheduled') {
70+
task.invoke();
71+
}
72+
reject(error);
73+
});
74+
},
75+
() => {
76+
ac?.abort();
77+
});
78+
});
79+
3080
global['fetch'] = function() {
3181
const args = Array.prototype.slice.call(arguments);
3282
const options = args.length > 1 ? args[1] : {};
33-
const signal = options && options.signal;
83+
const signal: AbortSignal|undefined = options?.signal;
3484
const ac = new AbortController();
3585
const fetchSignal = ac.signal;
3686
options.signal = fetchSignal;
3787
args[1] = options;
88+
3889
if (signal) {
3990
const nativeAddEventListener =
40-
signal[Zone.__symbol__('addEventListener')] || signal.addEventListener;
91+
signal[Zone.__symbol__('addEventListener') as 'addEventListener'] ||
92+
signal.addEventListener;
93+
4194
nativeAddEventListener.call(signal, 'abort', function() {
4295
ac!.abort();
4396
}, {once: true});
4497
}
45-
return new Promise((res, rej) => {
46-
const task = Zone.current.scheduleMacroTask(
47-
'fetch', placeholder, {fetchArgs: args} as FetchTaskData,
48-
() => {
49-
let fetchPromise;
50-
let zone = Zone.current;
51-
try {
52-
(zone as any)[fetchTaskScheduling] = true;
53-
fetchPromise = fetch.apply(this, args);
54-
} catch (error) {
55-
rej(error);
56-
return;
57-
} finally {
58-
(zone as any)[fetchTaskScheduling] = false;
59-
}
6098

61-
if (!(fetchPromise instanceof ZoneAwarePromise)) {
62-
let ctor = fetchPromise.constructor;
63-
if (!ctor[symbolThenPatched]) {
64-
api.patchThen(ctor);
65-
}
66-
}
67-
fetchPromise.then(
68-
(resource: any) => {
69-
if (task.state !== 'notScheduled') {
70-
task.invoke();
71-
}
72-
res(resource);
73-
},
74-
(error: any) => {
75-
if (task.state !== 'notScheduled') {
76-
task.invoke();
77-
}
78-
rej(error);
79-
});
80-
},
81-
() => {
82-
ac.abort();
83-
});
84-
});
99+
return createFetchTask('fetch', {fetchArgs: args} as FetchTaskData, fetch, this, args, ac);
85100
};
101+
102+
if (OriginalResponse?.prototype) {
103+
// https://fetch.spec.whatwg.org/#body-mixin
104+
['arrayBuffer', 'blob', 'formData', 'json', 'text']
105+
// Safely check whether the method exists on the `Response` prototype before patching.
106+
.filter(method => typeof OriginalResponse.prototype[method] === 'function')
107+
.forEach(method => {
108+
api.patchMethod(
109+
OriginalResponse.prototype, method,
110+
(delegate: Function) => (self, args) => createFetchTask(
111+
`Response.${method}`, undefined, delegate, self, args, undefined));
112+
});
113+
}
86114
});

‎packages/zone.js/test/common/fetch.spec.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,39 @@ describe(
156156
});
157157
});
158158

159+
// https://github.com/angular/angular/issues/50327
160+
it('Response.json() should be considered as macroTask', done => {
161+
fetchZone.run(() => {
162+
global['fetch']('/base/angular/packages/zone.js/test/assets/sample.json')
163+
.then((response: any) => {
164+
const promise = response.json();
165+
// Ensure it's a `ZoneAwarePromise`.
166+
expect(promise).toBeInstanceOf(global.Promise);
167+
return promise;
168+
})
169+
.then(() => {
170+
expect(logs).toEqual([
171+
'scheduleTask:fetch:macroTask', 'scheduleTask:Promise.then:microTask',
172+
'invokeTask:Promise.then:microTask', 'invokeTask:fetch:macroTask',
173+
'scheduleTask:Promise.then:microTask', 'invokeTask:Promise.then:microTask',
174+
// Please refer to the issue link above. Previously, `Response` methods were not
175+
// patched by zone.js, and their return values were considered only as
176+
// microtasks (not macrotasks). The Angular zone stabilized prematurely,
177+
// occurring before the resolution of the `response.json()` promise due to the
178+
// falsy value of `zone.hasPendingMacrotasks`. We are now ensuring that
179+
// `Response` methods are treated as macrotasks, similar to the behavior of
180+
// `fetch`.
181+
'scheduleTask:Response.json:macroTask', 'scheduleTask:Promise.then:microTask',
182+
'invokeTask:Promise.then:microTask', 'invokeTask:Response.json:macroTask',
183+
'scheduleTask:Promise.then:microTask', 'invokeTask:Promise.then:microTask',
184+
'scheduleTask:Promise.then:microTask', 'invokeTask:Promise.then:microTask'
185+
]);
186+
187+
done();
188+
});
189+
});
190+
});
191+
159192
it('cancel fetch should invoke onCancelTask',
160193
ifEnvSupportsWithDone('AbortController', (done: DoneFn) => {
161194
if (isSafari()) {

0 commit comments

Comments
 (0)