diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 3cdfae52c25db..2ace73aa69ad6 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -388,10 +388,7 @@ function warnOnHookMismatchInDev(currentHookName: HookType): void { } } -function warnIfAsyncClientComponent( - Component: Function, - componentDoesIncludeHooks: boolean, -) { +function warnIfAsyncClientComponent(Component: Function) { if (__DEV__) { // This dev-only check only works for detecting native async functions, // not transpiled ones. There's also a prod check that we use to prevent @@ -402,40 +399,16 @@ function warnIfAsyncClientComponent( // $FlowIgnore[method-unbinding] Object.prototype.toString.call(Component) === '[object AsyncFunction]'; if (isAsyncFunction) { - // Encountered an async Client Component. This is not yet supported, - // except in certain constrained cases, like during a route navigation. + // Encountered an async Client Component. This is not yet supported. const componentName = getComponentNameFromFiber(currentlyRenderingFiber); if (!didWarnAboutAsyncClientComponent.has(componentName)) { didWarnAboutAsyncClientComponent.add(componentName); - - // Check if this is a sync update. We use the "root" render lanes here - // because the "subtree" render lanes may include additional entangled - // lanes related to revealing previously hidden content. - const root = getWorkInProgressRoot(); - const rootRenderLanes = getWorkInProgressRootRenderLanes(); - if (root !== null && includesBlockingLane(root, rootRenderLanes)) { - console.error( - 'async/await is not yet supported in Client Components, only ' + - 'Server Components. This error is often caused by accidentally ' + - "adding `'use client'` to a module that was originally written " + - 'for the server.', - ); - } else { - // This is a concurrent (Transition, Retry, etc) render. We don't - // warn in these cases. - // - // However, Async Components are forbidden to include hooks, even - // during a transition, so let's check for that here. - // - // TODO: Add a corresponding warning to Server Components runtime. - if (componentDoesIncludeHooks) { - console.error( - 'Hooks are not supported inside an async component. This ' + - "error is often caused by accidentally adding `'use client'` " + - 'to a module that was originally written for the server.', - ); - } - } + console.error( + 'async/await is not yet supported in Client Components, only ' + + 'Server Components. This error is often caused by accidentally ' + + "adding `'use client'` to a module that was originally written " + + 'for the server.', + ); } } } @@ -521,6 +494,8 @@ export function renderWithHooks( // Used for hot reloading: ignorePreviousDependencies = current !== null && current.type !== workInProgress.type; + + warnIfAsyncClientComponent(Component); } workInProgress.memoizedState = null; @@ -637,10 +612,6 @@ function finishRenderingHooks( ): void { if (__DEV__) { workInProgress._debugHookTypes = hookTypesDev; - - const componentDoesIncludeHooks = - workInProgressHook !== null || thenableIndexCounter !== 0; - warnIfAsyncClientComponent(Component, componentDoesIncludeHooks); } // We can assume the previous dispatcher is always this one, since we set it diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 5da965e41c4fb..956f81e89270a 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -19,7 +19,24 @@ import {getWorkInProgressRoot} from './ReactFiberWorkLoop'; import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; -export opaque type ThenableState = Array>; +opaque type ThenableStateDev = { + didWarnAboutUncachedPromise: boolean, + thenables: Array>, +}; + +opaque type ThenableStateProd = Array>; + +export opaque type ThenableState = ThenableStateDev | ThenableStateProd; + +function getThenablesFromState(state: ThenableState): Array> { + 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. @@ -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): boolean { @@ -74,15 +98,44 @@ export function trackUsedThenable( 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); diff --git a/packages/react-reconciler/src/__tests__/ReactUse-test.js b/packages/react-reconciler/src/__tests__/ReactUse-test.js index fd49f8c8d784f..12e93620b5ded 100644 --- a/packages/react-reconciler/src/__tests__/ReactUse-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUse-test.js @@ -230,11 +230,17 @@ describe('ReactUse', () => { } const root = ReactNoop.createRoot(); - await act(() => { - startTransition(() => { - root.render(); + await expect(async () => { + await act(() => { + startTransition(() => { + root.render(); + }); }); - }); + }).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'); }); @@ -394,11 +400,20 @@ describe('ReactUse', () => { } const root = ReactNoop.createRoot(); - await act(() => { - startTransition(() => { - root.render(); + await expect(async () => { + await act(() => { + startTransition(() => { + root.render(); + }); }); - }); + }).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]', @@ -1733,25 +1748,38 @@ describe('ReactUse', () => { ); }); - test('warn if async client component calls a hook (e.g. useState)', async () => { - async function AsyncClientComponent() { - useState(); - return ; - } + test( + 'warn if async client component calls a hook (e.g. useState) ' + + 'during a non-sync update', + async () => { + async function AsyncClientComponent() { + useState(); + return ; + } - const root = ReactNoop.createRoot(); - await expect(async () => { - await act(() => { - startTransition(() => { - root.render(); + const root = ReactNoop.createRoot(); + await expect(async () => { + await act(() => { + startTransition(() => { + root.render(); + }); }); - }); - }).toErrorDev([ - 'Hooks are not supported inside an async component. This ' + - "error is often caused by accidentally adding `'use client'` " + - 'to a module that was originally written for the server.', - ]); - }); + }).toErrorDev([ + // Note: This used to log a different warning about not using hooks + // inside async components, like we do on the server. Since then, we + // decided to warn for _any_ async client component regardless of + // whether the update is sync. But if we ever add back support for async + // client components, we should add back the hook warning. + 'async/await is not yet supported in Client Components, only Server ' + + '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.', + ]); + }, + ); test('warn if async client component calls a hook (e.g. use)', async () => { const promise = Promise.resolve(); @@ -1769,9 +1797,21 @@ describe('ReactUse', () => { }); }); }).toErrorDev([ - 'Hooks are not supported inside an async component. This ' + - "error is often caused by accidentally adding `'use client'` " + - 'to a module that was originally written for the server.', + // Note: This used to log a different warning about not using hooks + // inside async components, like we do on the server. Since then, we + // decided to warn for _any_ async client component regardless of + // whether the update is sync. But if we ever add back support for async + // client components, we should add back the hook warning. + 'async/await is not yet supported in Client Components, only Server ' + + '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.', ]); }); });