Skip to content

Commit

Permalink
Warn if uncached promise suspends during render
Browse files Browse the repository at this point in the history
We do not yet support async/await on the client. You can only unwrap a
promise that was passed from a Server Component, or a
Suspense-compatible framework like Relay.
  • Loading branch information
acdlite committed Jan 30, 2024
1 parent fd66aa6 commit 54b1617
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 13 deletions.
63 changes: 58 additions & 5 deletions packages/react-reconciler/src/ReactFiberThenable.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,24 @@ import {getWorkInProgressRoot} from './ReactFiberWorkLoop';
import ReactSharedInternals from 'shared/ReactSharedInternals';
const {ReactCurrentActQueue} = ReactSharedInternals;

export opaque type ThenableState = Array<Thenable<any>>;
opaque type ThenableStateDev = {
didWarnAboutUncachedPromise: boolean,
thenables: Array<Thenable<any>>,
};

opaque type ThenableStateProd = Array<Thenable<any>>;

export opaque type ThenableState = ThenableStateDev | ThenableStateProd;

function getThenablesFromState(state: ThenableState): Array<Thenable<any>> {
if (__DEV__) {
const devState: ThenableStateDev = (state: any);
return devState.thenables;
} else {
const prodState = (state: any);
return prodState;
}
}

// An error that is thrown (e.g. by `use`) to trigger Suspense. If we
// detect this is caught by userspace, we'll log a warning in development.
Expand Down Expand Up @@ -56,7 +73,14 @@ export const noopSuspenseyCommitThenable = {
export function createThenableState(): ThenableState {
// The ThenableState is created the first time a component suspends. If it
// suspends again, we'll reuse the same state.
return [];
if (__DEV__) {
return {
didWarnAboutUncachedPromise: false,
thenables: [],
};
} else {
return [];
}
}

export function isThenableResolved(thenable: Thenable<mixed>): boolean {
Expand All @@ -74,15 +98,44 @@ export function trackUsedThenable<T>(
if (__DEV__ && ReactCurrentActQueue.current !== null) {
ReactCurrentActQueue.didUsePromise = true;
}

const previous = thenableState[index];
const trackedThenables = getThenablesFromState(thenableState);
const previous = trackedThenables[index];
if (previous === undefined) {
thenableState.push(thenable);
trackedThenables.push(thenable);
} else {
if (previous !== thenable) {
// Reuse the previous thenable, and drop the new one. We can assume
// they represent the same value, because components are idempotent.

if (__DEV__) {
const thenableStateDev: ThenableStateDev = (thenableState: any);
if (!thenableStateDev.didWarnAboutUncachedPromise) {
// We should only warn the first time an uncached thenable is
// discovered per component, because if there are multiple, the
// subsequent ones are likely derived from the first.
//
// We track this on the thenableState instead of deduping using the
// component name like we usually do, because in the case of a
// promise-as-React-node, the owner component is likely different from
// the parent that's currently being reconciled. We'd have to track
// the owner using state, which we're trying to move away from. Though
// since this is dev-only, maybe that'd be OK.
//
// However, another benefit of doing it this way is we might
// eventually have a thenableState per memo/Forget boundary instead
// of per component, so this would allow us to have more
// granular warnings.
thenableStateDev.didWarnAboutUncachedPromise = true;

// TODO: This warning should link to a corresponding docs page.
console.error(
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
);
}
}

// Avoid an unhandled rejection errors for the Promises that we'll
// intentionally ignore.
thenable.then(noop, noop);
Expand Down
40 changes: 32 additions & 8 deletions packages/react-reconciler/src/__tests__/ReactUse-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,17 @@ describe('ReactUse', () => {
}

const root = ReactNoop.createRoot();
await act(() => {
startTransition(() => {
root.render(<App />);
await expect(async () => {
await act(() => {
startTransition(() => {
root.render(<App />);
});
});
});
}).toErrorDev([
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
]);
assertLog(['ABC']);
expect(root).toMatchRenderedOutput('ABC');
});
Expand Down Expand Up @@ -394,11 +400,20 @@ describe('ReactUse', () => {
}

const root = ReactNoop.createRoot();
await act(() => {
startTransition(() => {
root.render(<App />);
await expect(async () => {
await act(() => {
startTransition(() => {
root.render(<App />);
});
});
});
}).toErrorDev([
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
]);
assertLog([
// First attempt. The uncached promise suspends.
'Suspend! [Async]',
Expand Down Expand Up @@ -1759,6 +1774,9 @@ describe('ReactUse', () => {
'Components. This error is often caused by accidentally adding ' +
"`'use client'` to a module that was originally written for " +
'the server.',
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
]);
},
);
Expand Down Expand Up @@ -1788,6 +1806,12 @@ describe('ReactUse', () => {
'Components. This error is often caused by accidentally adding ' +
"`'use client'` to a module that was originally written for " +
'the server.',
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
'A component was suspended by an uncached promise. Creating ' +
'promises inside a Client Component or hook is not yet ' +
'supported, except via a Suspense-compatible library or framework.',
]);
});
});

0 comments on commit 54b1617

Please sign in to comment.