Skip to content

Commit

Permalink
Unwrap sync resolved thenables without suspending (#25615)
Browse files Browse the repository at this point in the history 
If a thenable resolves synchronously, `use` should unwrap its result
without suspending or interrupting the component's execution.
  • Loading branch information
@acdlite
acdlite authored Nov 2, 2022
1 parent 4ea063b commit 1a90262
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 64 deletions.
27 changes: 27 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5300,6 +5300,33 @@ describe('ReactDOMFizzServer', () => {
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual('Hi');
});

// @gate enableUseHook
it('unwraps thenable that fulfills synchronously without suspending', async () => {
function App() {
const thenable = {
then(resolve) {
// This thenable immediately resolves, synchronously, without waiting
// a microtask.
resolve('Hi');
},
};
try {
return <Text text={use(thenable)} />;
} catch {
throw new Error(
'`use` should not suspend because the thenable resolved synchronously.',
);
}
}
// Because the thenable resolves synchronously, we should be able to finish
// rendering synchronously, with no fallback.
await act(async () => {
const {pipe} = renderToPipeableStream(<App />);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual('Hi');
});
});

describe('useEvent', () => {
Expand Down
18 changes: 15 additions & 3 deletions packages/react-reconciler/src/ReactFiberThenable.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,24 +110,36 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
// it's defined, but an unknown value, assume it's been instrumented by
// some custom userspace implementation. We treat it as "pending".
} else {
const pendingThenable: PendingThenable<mixed> = (thenable: any);
const pendingThenable: PendingThenable<T> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
const rejectedThenable: RejectedThenable<T> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);

// Check one more time in case the thenable resolved synchronously
switch (thenable.status) {
case 'fulfilled': {
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
return fulfilledThenable.value;
}
case 'rejected': {
const rejectedThenable: RejectedThenable<T> = (thenable: any);
throw rejectedThenable.reason;
}
}
}

// Suspend.
Expand Down
18 changes: 15 additions & 3 deletions packages/react-reconciler/src/ReactFiberThenable.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,24 +110,36 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
// it's defined, but an unknown value, assume it's been instrumented by
// some custom userspace implementation. We treat it as "pending".
} else {
const pendingThenable: PendingThenable<mixed> = (thenable: any);
const pendingThenable: PendingThenable<T> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
const rejectedThenable: RejectedThenable<T> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);

// Check one more time in case the thenable resolved synchronously
switch (thenable.status) {
case 'fulfilled': {
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
return fulfilledThenable.value;
}
case 'rejected': {
const rejectedThenable: RejectedThenable<T> = (thenable: any);
throw rejectedThenable.reason;
}
}
}

// Suspend.
Expand Down
36 changes: 10 additions & 26 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2016,37 +2016,21 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
workInProgressSuspendedReason !== NotSuspended &&
workInProgress !== null
) {
// The work loop is suspended. We need to either unwind the stack or
// replay the suspended component.
// The work loop is suspended. During a synchronous render, we don't
// yield to the main thread. Immediately unwind the stack. This will
// trigger either a fallback or an error boundary.
// TODO: For discrete and "default" updates (anything that's not
// flushSync), we want to wait for the microtasks the flush before
// unwinding. Will probably implement this using renderRootConcurrent,
// or merge renderRootSync and renderRootConcurrent into the same
// function and fork the behavior some other way.
const unitOfWork = workInProgress;
const thrownValue = workInProgressThrownValue;
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);

// TODO: This check is only here to account for thenables that
// synchronously resolve. Otherwise we would always unwind when
// rendering with renderRootSync. (In the future, discrete updates will
// use renderRootConcurrent instead.) We should account for
// synchronously resolved thenables before hitting this path.
switch (workInProgressSuspendedReason) {
case SuspendedOnError: {
// Unwind then continue with the normal work loop.
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
break;
}
default: {
const wasPinged =
workInProgressSuspendedThenableState !== null &&
isThenableStateResolved(workInProgressSuspendedThenableState);
if (wasPinged) {
replaySuspendedUnitOfWork(unitOfWork, thrownValue);
} else {
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
}
// Continue with the normal work loop.
break;
}
}
// Continue with the normal work loop.
}
workLoopSync();
break;
Expand Down
36 changes: 10 additions & 26 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2016,37 +2016,21 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
workInProgressSuspendedReason !== NotSuspended &&
workInProgress !== null
) {
// The work loop is suspended. We need to either unwind the stack or
// replay the suspended component.
// The work loop is suspended. During a synchronous render, we don't
// yield to the main thread. Immediately unwind the stack. This will
// trigger either a fallback or an error boundary.
// TODO: For discrete and "default" updates (anything that's not
// flushSync), we want to wait for the microtasks the flush before
// unwinding. Will probably implement this using renderRootConcurrent,
// or merge renderRootSync and renderRootConcurrent into the same
// function and fork the behavior some other way.
const unitOfWork = workInProgress;
const thrownValue = workInProgressThrownValue;
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);

// TODO: This check is only here to account for thenables that
// synchronously resolve. Otherwise we would always unwind when
// rendering with renderRootSync. (In the future, discrete updates will
// use renderRootConcurrent instead.) We should account for
// synchronously resolved thenables before hitting this path.
switch (workInProgressSuspendedReason) {
case SuspendedOnError: {
// Unwind then continue with the normal work loop.
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
break;
}
default: {
const wasPinged =
workInProgressSuspendedThenableState !== null &&
isThenableStateResolved(workInProgressSuspendedThenableState);
if (wasPinged) {
replaySuspendedUnitOfWork(unitOfWork, thrownValue);
} else {
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
}
// Continue with the normal work loop.
break;
}
}
// Continue with the normal work loop.
}
workLoopSync();
break;
Expand Down
28 changes: 28 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactThenable-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,4 +640,32 @@ describe('ReactThenable', () => {
});
expect(Scheduler).toHaveYielded(['Something different']);
});

// @gate enableUseHook
test('unwraps thenable that fulfills synchronously without suspending', async () => {
function App() {
const thenable = {
then(resolve) {
// This thenable immediately resolves, synchronously, without waiting
// a microtask.
resolve('Hi');
},
};
try {
return <Text text={use(thenable)} />;
} catch {
throw new Error(
'`use` should not suspend because the thenable resolved synchronously.',
);
}
}
// Because the thenable resolves synchronously, we should be able to finish
// rendering synchronously, with no fallback.
const root = ReactNoop.createRoot();
ReactNoop.flushSync(() => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['Hi']);
expect(root).toMatchRenderedOutput('Hi');
});
});
36 changes: 36 additions & 0 deletions packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,4 +778,40 @@ describe('ReactFlightDOMBrowser', () => {
});
expect(container.innerHTML).toBe('Hi');
});

// @gate enableUseHook
it('unwraps thenable that fulfills synchronously without suspending', async () => {
function Server() {
const thenable = {
then(resolve) {
// This thenable immediately resolves, synchronously, without waiting
// a microtask.
resolve('Hi');
},
};
try {
return use(thenable);
} catch {
throw new Error(
'`use` should not suspend because the thenable resolved synchronously.',
);
}
}

// Because the thenable resolves synchronously, we should be able to finish
// rendering synchronously, with no fallback.
const stream = ReactServerDOMWriter.renderToReadableStream(<Server />);
const response = ReactServerDOMReader.createFromReadableStream(stream);

function Client() {
return use(response);
}

const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(<Client />);
});
expect(container.innerHTML).toBe('Hi');
});
});
18 changes: 15 additions & 3 deletions packages/react-server/src/ReactFizzThenable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,36 @@ export function trackUsedThenable<T>(
// it's defined, but an unknown value, assume it's been instrumented by
// some custom userspace implementation. We treat it as "pending".
} else {
const pendingThenable: PendingThenable<mixed> = (thenable: any);
const pendingThenable: PendingThenable<T> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
const rejectedThenable: RejectedThenable<T> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);

// Check one more time in case the thenable resolved synchronously
switch (thenable.status) {
case 'fulfilled': {
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
return fulfilledThenable.value;
}
case 'rejected': {
const rejectedThenable: RejectedThenable<T> = (thenable: any);
throw rejectedThenable.reason;
}
}
}

// Suspend.
Expand Down
18 changes: 15 additions & 3 deletions packages/react-server/src/ReactFlightThenable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,36 @@ export function trackUsedThenable<T>(
// it's defined, but an unknown value, assume it's been instrumented by
// some custom userspace implementation. We treat it as "pending".
} else {
const pendingThenable: PendingThenable<mixed> = (thenable: any);
const pendingThenable: PendingThenable<T> = (thenable: any);
pendingThenable.status = 'pending';
pendingThenable.then(
fulfilledValue => {
if (thenable.status === 'pending') {
const fulfilledThenable: FulfilledThenable<mixed> = (thenable: any);
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
fulfilledThenable.status = 'fulfilled';
fulfilledThenable.value = fulfilledValue;
}
},
(error: mixed) => {
if (thenable.status === 'pending') {
const rejectedThenable: RejectedThenable<mixed> = (thenable: any);
const rejectedThenable: RejectedThenable<T> = (thenable: any);
rejectedThenable.status = 'rejected';
rejectedThenable.reason = error;
}
},
);

// Check one more time in case the thenable resolved synchronously
switch (thenable.status) {
case 'fulfilled': {
const fulfilledThenable: FulfilledThenable<T> = (thenable: any);
return fulfilledThenable.value;
}
case 'rejected': {
const rejectedThenable: RejectedThenable<T> = (thenable: any);
throw rejectedThenable.reason;
}
}
}

// Suspend.
Expand Down

0 comments on commit 1a90262

Please sign in to comment.