Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect and warn about native async function components in development #27031

Merged
merged 1 commit into from
Jul 2, 2023
Merged

Detect and warn about native async function components in development #27031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 77 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,11 @@ export type UpdateQueue<S, A> = {
let didWarnAboutMismatchedHooksForComponent;
let didWarnUncachedGetSnapshot: void | true;
let didWarnAboutUseWrappedInTryCatch;
let didWarnAboutAsyncClientComponent;
if (__DEV__) {
didWarnAboutMismatchedHooksForComponent = new Set<string | null>();
didWarnAboutUseWrappedInTryCatch = new Set<string | null>();
didWarnAboutAsyncClientComponent = new Set<string | null>();
}

export type Hook = {
Expand Down Expand Up @@ -370,6 +372,59 @@ function warnOnHookMismatchInDev(currentHookName: HookType): void {
}
}

function warnIfAsyncClientComponent(
Component: Function,
componentDoesIncludeHooks: boolean,
) {
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
// async client components from crashing the app; the prod one works even
// for transpiled async functions. Neither mechanism is completely
// bulletproof but together they cover the most common cases.
const isAsyncFunction =
// $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.
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.',
);
}
}
}
}
}
}

function throwInvalidHookError() {
throw new Error(
'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
Expand Down Expand Up @@ -554,20 +609,28 @@ export function renderWithHooks<Props, SecondArg>(
}
}

finishRenderingHooks(current, workInProgress);
finishRenderingHooks(current, workInProgress, Component);

return children;
}

function finishRenderingHooks(current: Fiber | null, workInProgress: Fiber) {
// We can assume the previous dispatcher is always this one, since we set it
// at the beginning of the render phase and there's no re-entrance.
ReactCurrentDispatcher.current = ContextOnlyDispatcher;

function finishRenderingHooks<Props, SecondArg>(
current: Fiber | null,
workInProgress: Fiber,
Component: (p: Props, arg: SecondArg) => any,
): 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
// at the beginning of the render phase and there's no re-entrance.
ReactCurrentDispatcher.current = ContextOnlyDispatcher;

// This check uses currentHook so that it works the same in DEV and prod bundles.
// hookTypesDev could catch more cases (e.g. context) but only in DEV bundles.
const didRenderTooFewHooks =
Expand Down Expand Up @@ -645,7 +708,13 @@ function finishRenderingHooks(current: Fiber | null, workInProgress: Fiber) {
if (checkIfUseWrappedInTryCatch()) {
const componentName =
getComponentNameFromFiber(workInProgress) || 'Unknown';
if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) {
if (
!didWarnAboutUseWrappedInTryCatch.has(componentName) &&
// This warning also fires if you suspend with `use` inside an
// async component. Since we warn for that above, we'll silence this
// second warning by checking here.
!didWarnAboutAsyncClientComponent.has(componentName)
) {
didWarnAboutUseWrappedInTryCatch.add(componentName);
console.error(
'`use` was called from inside a try/catch block. This is not allowed ' +
Expand Down Expand Up @@ -683,7 +752,7 @@ export function replaySuspendedComponentWithHooks<Props, SecondArg>(
props,
secondArg,
);
finishRenderingHooks(current, workInProgress);
finishRenderingHooks(current, workInProgress, Component);
return children;
}

Expand Down
40 changes: 30 additions & 10 deletions packages/react-reconciler/src/ReactFiberThenable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export function trackUsedThenable<T>(
}
case 'rejected': {
const rejectedError = thenable.reason;
checkIfUseWrappedInAsyncCatch(rejectedError);
throw rejectedError;
}
default: {
Expand Down Expand Up @@ -149,17 +150,19 @@ export function trackUsedThenable<T>(
}
},
);
}

// 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;
// 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);
const rejectedError = rejectedThenable.reason;
checkIfUseWrappedInAsyncCatch(rejectedError);
throw rejectedError;
}
}
}

Expand Down Expand Up @@ -223,3 +226,20 @@ export function checkIfUseWrappedInTryCatch(): boolean {
}
return false;
}

export function checkIfUseWrappedInAsyncCatch(rejectedReason: any) {
// This check runs in prod, too, because it prevents a more confusing
// downstream error, where SuspenseException is caught by a promise and
// thrown asynchronously.
// TODO: Another way to prevent SuspenseException from leaking into an async
// execution context is to check the dispatcher every time `use` is called,
// or some equivalent. That might be preferable for other reasons, too, since
// it matches how we prevent similar mistakes for other hooks.
if (rejectedReason === SuspenseException) {
throw new 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.',
);
}
}
84 changes: 70 additions & 14 deletions packages/react-reconciler/src/__tests__/ReactUse-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1639,13 +1639,20 @@ describe('ReactUse', () => {
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(
<ErrorBoundary>
<AsyncClientComponent />
</ErrorBoundary>,
);
});
await expect(async () => {
await act(() => {
root.render(
<ErrorBoundary>
<AsyncClientComponent />
</ErrorBoundary>,
);
});
}).toErrorDev([
'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.',
]);
assertLog([
'async/await is not yet supported in Client Components, only Server ' +
'Components. This error is often caused by accidentally adding ' +
Expand Down Expand Up @@ -1685,13 +1692,20 @@ describe('ReactUse', () => {
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(
<ErrorBoundary>
<AsyncClientComponent />
</ErrorBoundary>,
);
});
await expect(async () => {
await act(() => {
root.render(
<ErrorBoundary>
<AsyncClientComponent />
</ErrorBoundary>,
);
});
}).toErrorDev([
'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.',
]);
assertLog([
'async/await is not yet supported in Client Components, only Server ' +
'Components. This error is often caused by accidentally adding ' +
Expand All @@ -1709,4 +1723,46 @@ describe('ReactUse', () => {
'the server.',
);
});

test('warn if async client component calls a hook (e.g. useState)', async () => {
async function AsyncClientComponent() {
useState();
return <Text text="Hi" />;
}

const root = ReactNoop.createRoot();
await expect(async () => {
await act(() => {
startTransition(() => {
root.render(<AsyncClientComponent />);
});
});
}).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.',
]);
});

test('warn if async client component calls a hook (e.g. use)', async () => {
const promise = Promise.resolve();

async function AsyncClientComponent() {
use(promise);
return <Text text="Hi" />;
}

const root = ReactNoop.createRoot();
await expect(async () => {
await act(() => {
startTransition(() => {
root.render(<AsyncClientComponent />);
});
});
}).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.',
]);
});
});
10 changes: 9 additions & 1 deletion packages/shared/ReactComponentStackFrame.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,15 @@ export function describeNativeComponentFrame(
// TODO(luna): This will currently only throw if the function component
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too
fn();
const maybePromise = fn();

// If the function component returns a promise, it's likely an async
// component, which we don't yet support. Attach a noop catch handler to
// silence the error.
// TODO: Implement component stacks for async client components?
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
Expand Down
3 changes: 2 additions & 1 deletion scripts/error-codes/codes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,5 +467,6 @@
"479": "Cannot update optimistic state while rendering.",
"480": "File/Blob fields are not yet supported in progressive forms. It probably means you are closing over binary data or FormData in a Server Action.",
"481": "Tried to encode a Server Action from a different instance than the encoder is from. This is a bug in React.",
"482": "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."
"482": "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.",
"483": "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."
}