Skip to content

Commit 1136e41

Browse files
rubennortemeta-codesync[bot]
authored andcommitted
Make AbortSignal.any spec-compliant with a multi-pass implementation (#57381)
Summary: Pull Request resolved: #57381 `AbortSignal.any` previously processed the input signals in a single pass, attaching an abort listener to each signal as it iterated and unwinding them with a `cleanup()` call whenever it later encountered an already-aborted (or invalid) signal. This deviates from the specification, which validates the whole list, then scans it for an already-aborted signal (returning an aborted signal immediately), and only then subscribes to the remaining signals. Process the list in three passes instead: - The first pass validates that every entry is an `AbortSignal`, throwing before any other work so an invalid entry never short-circuits ahead of an earlier already-aborted one. - The second pass returns an already-aborted signal (via `AbortSignal.abort(reason)`) if any input is already aborted. No listeners are registered yet, so there is nothing to unwind on an early return. - The third pass subscribes to every signal, since all of them are valid and none is aborted at that point. This removes the need to clean up partially-registered listeners mid-setup and matches the behavior described in the DOM specification. Also remove a dead branch in `AbortController`'s `getSignal` helper. The `controller === null ? 'null' : typeof controller` expression was unreachable for `null` input because the preceding `controller[SIGNAL_KEY]` access already throws a `TypeError` on `null`/`undefined`, and it only existed by suppressing a Flow `invalid-compare` error. Use `typeof controller` directly so the suppression is no longer needed. Changelog: [General][Fixed] - Make `AbortSignal.any()` process its input signals in multiple passes to match the DOM specification Reviewed By: javache Differential Revision: D110185361 fbshipit-source-id: 633a8ce4f8a2b2a892eccffb644d94ce705a3449
1 parent 4aef2b0 commit 1136e41

3 files changed

Lines changed: 38 additions & 15 deletions

File tree

packages/react-native/src/private/webapis/dom/abort-api/AbortController.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,7 @@ function getSignal(controller: AbortController): AbortSignal {
7272
const signal = controller[SIGNAL_KEY];
7373
if (signal == null) {
7474
throw new TypeError(
75-
`Expected 'this' to be an 'AbortController' object, but got ${
76-
// $FlowExpectedError[invalid-compare]
77-
controller === null ? 'null' : typeof controller
78-
}`,
75+
`Expected 'this' to be an 'AbortController' object, but got ${typeof controller}`,
7976
);
8077
}
8178
return signal;

packages/react-native/src/private/webapis/dom/abort-api/AbortSignal.js

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,30 +91,36 @@ export class AbortSignal extends EventTarget {
9191
throw new TypeError('The signals value must be an instance of Array');
9292
}
9393

94-
const controller = new AbortController();
95-
const listeners = [];
96-
const cleanup = () => listeners.forEach(unsubscribe => unsubscribe());
97-
94+
// First pass: validate that every item is an AbortSignal.
9895
for (let i = 0; i < signals.length; i++) {
9996
const signal = signals[i];
100-
101-
// Validate that each item is an AbortSignal
10297
if (!(signal instanceof AbortSignal)) {
103-
cleanup(); // Remove all listeners added so far
10498
throw new TypeError(
10599
'The "signals[' +
106100
i +
107101
']" argument must be an instance of AbortSignal',
108102
);
109103
}
104+
}
110105

111-
// Abort immediately if one of the signals is already aborted
106+
// Second pass: short-circuit if any of the signals is already aborted. No
107+
// listeners have been registered yet, so there is nothing to clean up if
108+
// we return early.
109+
for (let i = 0; i < signals.length; i++) {
110+
const signal = signals[i];
112111
if (signal.aborted) {
113-
cleanup(); // Remove all listeners added so far
114-
controller.abort(signal.reason);
115-
break;
112+
return AbortSignal.abort(signal.reason);
116113
}
114+
}
115+
116+
// Third pass: none of the signals is aborted, so subscribe to all of them
117+
// and abort the resulting signal when any of them aborts.
118+
const controller = new AbortController();
119+
const listeners = [];
120+
const cleanup = () => listeners.forEach(unsubscribe => unsubscribe());
117121

122+
for (let i = 0; i < signals.length; i++) {
123+
const signal = signals[i];
118124
const onAbort = () => {
119125
controller.abort(signal.reason);
120126
cleanup();

packages/react-native/src/private/webapis/dom/abort-api/__tests__/AbortController-itest.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,26 @@ describe('AbortController', () => {
373373
'The "signals[1]" argument must be an instance of AbortSignal',
374374
);
375375
});
376+
377+
it('should validate every element before short-circuiting on an already-aborted signal', () => {
378+
const aborted = new AbortController();
379+
aborted.abort(new Error('already aborted'));
380+
381+
let thrown;
382+
try {
383+
// $FlowExpectedError[incompatible-type]
384+
AbortSignal.any([aborted.signal, {}]);
385+
} catch (error) {
386+
thrown = error;
387+
}
388+
389+
expect(thrown).toBeInstanceOf(TypeError);
390+
// $FlowExpectedError[incompatible-type]
391+
const typeError: TypeError = thrown;
392+
expect(typeError.message).toBe(
393+
'The "signals[1]" argument must be an instance of AbortSignal',
394+
);
395+
});
376396
});
377397

378398
describe("'timeout' static method", () => {

0 commit comments

Comments
 (0)