Skip to content

Commit 14c8af7

Browse files
acdliteacdlite
committed
Fix: Resolve entangled actions independently
When there are multiple async actions at the same time, we entangle them together because we can't be sure which action an update might be associated with. (For this, we'd need AsyncContext.) However, if one of the async actions fails with an error, it should only affect that action, not all the other actions it may be entangled with. Resolving each action independently also means they can have independent pending state types, rather than being limited to an `isPending` boolean. We'll use this to implement an upcoming form API.
1 parent ed545ae commit 14c8af7

File tree

5 files changed

+278
-108
lines changed

5 files changed

+278
-108
lines changed

‎packages/react-reconciler/src/ReactFiberAsyncAction.js‎

Lines changed: 136 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -7,44 +7,36 @@
77
* @flow
88
*/
99

10-
import type {Wakeable} from 'shared/ReactTypes';
10+
import type {
11+
Thenable,
12+
PendingThenable,
13+
FulfilledThenable,
14+
RejectedThenable,
15+
} from 'shared/ReactTypes';
1116
import type {Lane} from './ReactFiberLane';
12-
import {requestTransitionLane} from './ReactFiberRootScheduler';
13-
14-
interface AsyncActionImpl {
15-
lane: Lane;
16-
listeners: Array<(false) => mixed>;
17-
count: number;
18-
then(
19-
onFulfill: (value: boolean) => mixed,
20-
onReject: (error: mixed) => mixed,
21-
): void;
22-
}
23-
24-
interface PendingAsyncAction extends AsyncActionImpl {
25-
status: 'pending';
26-
}
27-
28-
interface FulfilledAsyncAction extends AsyncActionImpl {
29-
status: 'fulfilled';
30-
value: boolean;
31-
}
3217

33-
interface RejectedAsyncAction extends AsyncActionImpl {
34-
status: 'rejected';
35-
reason: mixed;
36-
}
18+
import {requestTransitionLane} from './ReactFiberRootScheduler';
19+
import {NoLane} from './ReactFiberLane';
3720

38-
type AsyncAction =
39-
| PendingAsyncAction
40-
| FulfilledAsyncAction
41-
| RejectedAsyncAction;
21+
// If there are multiple, concurrent async actions, they are entangled. All
22+
// transition updates that occur while the async action is still in progress
23+
// are treated as part of the action.
24+
//
25+
// The ideal behavior would be to treat each async function as an independent
26+
// action. However, without a mechanism like AsyncContext, we can't tell which
27+
// action an update corresponds to. So instead, we entangle them all into one.
4228

43-
let currentAsyncAction: AsyncAction | null = null;
29+
// The listeners to notify once the entangled scope completes.
30+
let currentEntangledListeners: Array<() => mixed> | null = null;
31+
// The number of pending async actions in the entangled scope.
32+
let currentEntangledPendingCount: number = 0;
33+
// The transition lane shared by all updates in the entangled scope.
34+
let currentEntangledLane: Lane = NoLane;
4435

45-
export function requestAsyncActionContext(
36+
export function requestAsyncActionContext<S>(
4637
actionReturnValue: mixed,
47-
): AsyncAction | false {
38+
finishedState: S,
39+
): Thenable<S> | S {
4840
if (
4941
actionReturnValue !== null &&
5042
typeof actionReturnValue === 'object' &&
@@ -53,78 +45,131 @@ export function requestAsyncActionContext(
5345
// This is an async action.
5446
//
5547
// Return a thenable that resolves once the action scope (i.e. the async
56-
// function passed to startTransition) has finished running. The fulfilled
57-
// value is `false` to represent that the action is not pending.
58-
const thenable: Wakeable = (actionReturnValue: any);
59-
if (currentAsyncAction === null) {
48+
// function passed to startTransition) has finished running.
49+
50+
const thenable: Thenable<mixed> = (actionReturnValue: any);
51+
let entangledListeners;
52+
if (currentEntangledListeners === null) {
6053
// There's no outer async action scope. Create a new one.
61-
const asyncAction: AsyncAction = {
62-
lane: requestTransitionLane(),
63-
listeners: [],
64-
count: 0,
65-
status: 'pending',
66-
value: false,
67-
reason: undefined,
68-
then(resolve: boolean => mixed) {
69-
asyncAction.listeners.push(resolve);
70-
},
71-
};
72-
attachPingListeners(thenable, asyncAction);
73-
currentAsyncAction = asyncAction;
74-
return asyncAction;
54+
entangledListeners = currentEntangledListeners = [];
55+
currentEntangledPendingCount = 0;
56+
currentEntangledLane = requestTransitionLane();
7557
} else {
76-
// Inherit the outer scope.
77-
const asyncAction: AsyncAction = (currentAsyncAction: any);
78-
attachPingListeners(thenable, asyncAction);
79-
return asyncAction;
58+
entangledListeners = currentEntangledListeners;
8059
}
60+
61+
currentEntangledPendingCount++;
62+
let resultStatus = 'pending';
63+
let rejectedReason;
64+
thenable.then(
65+
() => {
66+
resultStatus = 'fulfilled';
67+
pingEngtangledActionScope();
68+
},
69+
error => {
70+
resultStatus = 'rejected';
71+
rejectedReason = error;
72+
pingEngtangledActionScope();
73+
},
74+
);
75+
76+
// Create a thenable that represents the result of this action, but doesn't
77+
// resolve until the entire entangled scope has finished.
78+
//
79+
// Expressed using promises:
80+
// const [thisResult] = await Promise.all([thisAction, entangledAction]);
81+
// return thisResult;
82+
const resultThenable = createResultThenable<S>(entangledListeners);
83+
84+
// Attach a listener to fill in the result.
85+
entangledListeners.push(() => {
86+
switch (resultStatus) {
87+
case 'fulfilled': {
88+
const fulfilledThenable: FulfilledThenable<S> = (resultThenable: any);
89+
fulfilledThenable.status = 'fulfilled';
90+
fulfilledThenable.value = finishedState;
91+
break;
92+
}
93+
case 'rejected': {
94+
const rejectedThenable: RejectedThenable<S> = (resultThenable: any);
95+
rejectedThenable.status = 'rejected';
96+
rejectedThenable.reason = rejectedReason;
97+
break;
98+
}
99+
case 'pending':
100+
default: {
101+
// The listener above should have been called first, so `resultStatus`
102+
// should already be set to the correct value.
103+
throw new Error(
104+
'Thenable should have already resolved. This ' +
105+
'is a bug in React.',
106+
);
107+
}
108+
}
109+
});
110+
111+
return resultThenable;
81112
} else {
82113
// This is not an async action, but it may be part of an outer async action.
83-
if (currentAsyncAction === null) {
84-
// There's no outer async action scope.
85-
return false;
114+
if (currentEntangledListeners === null) {
115+
return finishedState;
86116
} else {
87-
// Inherit the outer scope.
88-
return currentAsyncAction;
117+
// Return a thenable that does not resolve until the entangled actions
118+
// have finished.
119+
const entangledListeners = currentEntangledListeners;
120+
const resultThenable = createResultThenable<S>(entangledListeners);
121+
entangledListeners.push(() => {
122+
const fulfilledThenable: FulfilledThenable<S> = (resultThenable: any);
123+
fulfilledThenable.status = 'fulfilled';
124+
fulfilledThenable.value = finishedState;
125+
});
126+
return resultThenable;
89127
}
90128
}
91129
}
92130

93-
export function peekAsyncActionContext(): AsyncAction | null {
94-
return currentAsyncAction;
131+
function pingEngtangledActionScope() {
132+
if (
133+
currentEntangledListeners !== null &&
134+
--currentEntangledPendingCount === 0
135+
) {
136+
// All the actions have finished. Close the entangled async action scope
137+
// and notify all the listeners.
138+
const listeners = currentEntangledListeners;
139+
currentEntangledListeners = null;
140+
currentEntangledLane = NoLane;
141+
for (let i = 0; i < listeners.length; i++) {
142+
const listener = listeners[i];
143+
listener();
144+
}
145+
}
95146
}
96147

97-
function attachPingListeners(thenable: Wakeable, asyncAction: AsyncAction) {
98-
asyncAction.count++;
99-
thenable.then(
100-
() => {
101-
if (--asyncAction.count === 0) {
102-
const fulfilledAsyncAction: FulfilledAsyncAction = (asyncAction: any);
103-
fulfilledAsyncAction.status = 'fulfilled';
104-
completeAsyncActionScope(asyncAction);
105-
}
106-
},
107-
(error: mixed) => {
108-
if (--asyncAction.count === 0) {
109-
const rejectedAsyncAction: RejectedAsyncAction = (asyncAction: any);
110-
rejectedAsyncAction.status = 'rejected';
111-
rejectedAsyncAction.reason = error;
112-
completeAsyncActionScope(asyncAction);
113-
}
148+
function createResultThenable<S>(
149+
entangledListeners: Array<() => mixed>,
150+
): Thenable<S> {
151+
// Waits for the entangled async action to complete, then resolves to the
152+
// result of an individual action.
153+
const resultThenable: PendingThenable<S> = {
154+
status: 'pending',
155+
value: null,
156+
reason: null,
157+
then(resolve: S => mixed) {
158+
// This is a bit of a cheat. `resolve` expects a value of type `S` to be
159+
// passed, but because we're instrumenting the `status` field ourselves,
160+
// and we know this thenable will only be used by React, we also know
161+
// the value isn't actually needed. So we add the resolve function
162+
// directly to the entangled listeners.
163+
//
164+
// This is also why we don't need to check if the thenable is still
165+
// pending; the Suspense implementation already performs that check.
166+
const ping: () => mixed = (resolve: any);
167+
entangledListeners.push(ping);
114168
},
115-
);
116-
return asyncAction;
169+
};
170+
return resultThenable;
117171
}
118172

119-
function completeAsyncActionScope(action: AsyncAction) {
120-
if (currentAsyncAction === action) {
121-
currentAsyncAction = null;
122-
}
123-
124-
const listeners = action.listeners;
125-
action.listeners = [];
126-
for (let i = 0; i < listeners.length; i++) {
127-
const listener = listeners[i];
128-
listener(false);
129-
}
173+
export function peekEntangledActionLane(): Lane {
174+
return currentEntangledLane;
130175
}

‎packages/react-reconciler/src/ReactFiberHooks.js‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2431,8 +2431,10 @@ function updateDeferredValueImpl<T>(hook: Hook, prevValue: T, value: T): T {
24312431
}
24322432
}
24332433

2434-
function startTransition(
2435-
setPending: (Thenable<boolean> | boolean) => void,
2434+
function startTransition<S>(
2435+
pendingState: S,
2436+
finishedState: S,
2437+
setPending: (Thenable<S> | S) => void,
24362438
callback: () => mixed,
24372439
options?: StartTransitionOptions,
24382440
): void {
@@ -2443,7 +2445,7 @@ function startTransition(
24432445

24442446
const prevTransition = ReactCurrentBatchConfig.transition;
24452447
ReactCurrentBatchConfig.transition = null;
2446-
setPending(true);
2448+
setPending(pendingState);
24472449
const currentTransition = (ReactCurrentBatchConfig.transition =
24482450
({}: BatchConfigTransition));
24492451

@@ -2462,23 +2464,26 @@ function startTransition(
24622464
if (enableAsyncActions) {
24632465
const returnValue = callback();
24642466

2465-
// `isPending` is either `false` or a thenable that resolves to `false`,
2466-
// depending on whether the action scope is an async function. In the
2467-
// async case, the resulting render will suspend until the async action
2468-
// scope has finished.
2469-
const isPending = requestAsyncActionContext(returnValue);
2470-
setPending(isPending);
2467+
// This is either `finishedState` or a thenable that resolves to
2468+
// `finishedState`, depending on whether the action scope is an async
2469+
// function. In the async case, the resulting render will suspend until
2470+
// the async action scope has finished.
2471+
const maybeThenable = requestAsyncActionContext(
2472+
returnValue,
2473+
finishedState,
2474+
);
2475+
setPending(maybeThenable);
24712476
} else {
24722477
// Async actions are not enabled.
2473-
setPending(false);
2478+
setPending(finishedState);
24742479
callback();
24752480
}
24762481
} catch (error) {
24772482
if (enableAsyncActions) {
24782483
// This is a trick to get the `useTransition` hook to rethrow the error.
24792484
// When it unwraps the thenable with the `use` algorithm, the error
24802485
// will be thrown.
2481-
const rejectedThenable: RejectedThenable<boolean> = {
2486+
const rejectedThenable: RejectedThenable<S> = {
24822487
then() {},
24832488
status: 'rejected',
24842489
reason: error,
@@ -2594,6 +2599,8 @@ export function startHostTransition<F>(
25942599
}
25952600

25962601
startTransition(
2602+
true,
2603+
false,
25972604
setPending,
25982605
// TODO: We can avoid this extra wrapper, somehow. Figure out layering
25992606
// once more of this function is implemented.
@@ -2607,7 +2614,7 @@ function mountTransition(): [
26072614
] {
26082615
const [, setPending] = mountState((false: Thenable<boolean> | boolean));
26092616
// The `start` method never changes.
2610-
const start = startTransition.bind(null, setPending);
2617+
const start = startTransition.bind(null, true, false, setPending);
26112618
const hook = mountWorkInProgressHook();
26122619
hook.memoizedState = start;
26132620
return [false, start];

‎packages/react-reconciler/src/ReactFiberWorkLoop.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ import {
279279
requestTransitionLane,
280280
} from './ReactFiberRootScheduler';
281281
import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext';
282-
import {peekAsyncActionContext} from './ReactFiberAsyncAction';
282+
import {peekEntangledActionLane} from './ReactFiberAsyncAction';
283283

284284
const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
285285

@@ -632,10 +632,10 @@ export function requestUpdateLane(fiber: Fiber): Lane {
632632
transition._updatedFibers.add(fiber);
633633
}
634634

635-
const asyncAction = peekAsyncActionContext();
636-
return asyncAction !== null
635+
const actionScopeLane = peekEntangledActionLane();
636+
return actionScopeLane !== NoLane
637637
? // We're inside an async action scope. Reuse the same lane.
638-
asyncAction.lane
638+
actionScopeLane
639639
: // We may or may not be inside an async action scope. If we are, this
640640
// is the first update in that scope. Either way, we need to get a
641641
// fresh transition lane.

0 commit comments

Comments
 (0)