Skip to content

Commit

Permalink
Detect and warn about native async function components in development (
Browse files Browse the repository at this point in the history
…#27031)

Adds a development warning to complement the error introduced by
#27019.

We can detect and warn about async client components by checking the
prototype of the function. This won't work for environments where async
functions are transpiled, but for native async functions, it allows us
to log an earlier warning during development, including in cases that
don't trigger the infinite loop guard added in
#27019. It does not supersede the
infinite loop guard, though, because that mechanism also prevents the
app from crashing.

I also added a warning for calling a hook inside an async function. This
one fires even during a transition. We could add a corresponding warning
to Flight, since hooks are not allowed in async Server Components,
either. (Though in both environments, this is better handled by a lint
rule.)

DiffTrain build for [5c8dabf](5c8dabf)
  • Loading branch information
acdlite committed Jul 2, 2023
1 parent 023fffc commit d8c7f59
Show file tree
Hide file tree
Showing 26 changed files with 2,715 additions and 1,904 deletions.
9 changes: 8 additions & 1 deletion compiled/facebook-www/JSXDEVRuntime-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,14 @@ function describeNativeComponentFrame(fn, construct) {
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too

fn();
var 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(function () {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
Expand Down
9 changes: 8 additions & 1 deletion compiled/facebook-www/JSXDEVRuntime-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,14 @@ function describeNativeComponentFrame(fn, construct) {
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too

fn();
var 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(function () {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7f362de1588d98438787d652941533e21f2f332d
5c8dabf8864e1d826c831d6096b2dfa66309961a
11 changes: 9 additions & 2 deletions compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-60aa7d44";
var ReactVersion = "18.3.0-www-classic-cc85cd34";

// ATTENTION
// When adding new symbols to this file,
Expand Down Expand Up @@ -2293,7 +2293,14 @@ function describeNativeComponentFrame(fn, construct) {
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too

fn();
var 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(function () {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
Expand Down
11 changes: 9 additions & 2 deletions compiled/facebook-www/React-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-modern-fec65964";
var ReactVersion = "18.3.0-www-modern-01767f27";

// ATTENTION
// When adding new symbols to this file,
Expand Down Expand Up @@ -2293,7 +2293,14 @@ function describeNativeComponentFrame(fn, construct) {
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too

fn();
var 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(function () {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -622,4 +622,4 @@ exports.useSyncExternalStore = function (
);
};
exports.useTransition = useTransition;
exports.version = "18.3.0-www-modern-e397df4a";
exports.version = "18.3.0-www-modern-4c04bf5d";
128 changes: 107 additions & 21 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-classic-9da53968";
var ReactVersion = "18.3.0-www-classic-4bc5632c";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -3013,7 +3013,14 @@ function describeNativeComponentFrame(fn, construct) {
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too

fn();
var 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(function () {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
Expand Down Expand Up @@ -5401,6 +5408,7 @@ function trackUsedThenable(thenableState, thenable, index) {

case "rejected": {
var rejectedError = thenable.reason;
checkIfUseWrappedInAsyncCatch(rejectedError);
throw rejectedError;
}

Expand Down Expand Up @@ -5456,18 +5464,20 @@ function trackUsedThenable(thenableState, thenable, index) {
rejectedThenable.reason = error;
}
}
);
} // Check one more time in case the thenable resolved synchronously.
); // Check one more time in case the thenable resolved synchronously.

switch (thenable.status) {
case "fulfilled": {
var fulfilledThenable = thenable;
return fulfilledThenable.value;
}
switch (thenable.status) {
case "fulfilled": {
var fulfilledThenable = thenable;
return fulfilledThenable.value;
}

case "rejected": {
var rejectedThenable = thenable;
throw rejectedThenable.reason;
case "rejected": {
var rejectedThenable = thenable;
var _rejectedError = rejectedThenable.reason;
checkIfUseWrappedInAsyncCatch(_rejectedError);
throw _rejectedError;
}
}
} // Suspend.
//
Expand Down Expand Up @@ -5526,6 +5536,22 @@ function checkIfUseWrappedInTryCatch() {

return false;
}
function checkIfUseWrappedInAsyncCatch(rejectedReason) {
// 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."
);
}
}

var thenableState$1 = null;
var thenableIndexCounter$1 = 0;
Expand Down Expand Up @@ -7843,10 +7869,12 @@ var ReactCurrentDispatcher$1 = ReactSharedInternals.ReactCurrentDispatcher,
var didWarnAboutMismatchedHooksForComponent;
var didWarnUncachedGetSnapshot;
var didWarnAboutUseWrappedInTryCatch;
var didWarnAboutAsyncClientComponent;

{
didWarnAboutMismatchedHooksForComponent = new Set();
didWarnAboutUseWrappedInTryCatch = new Set();
didWarnAboutAsyncClientComponent = new Set();
} // The effect "instance" is a shared object that remains the same for the entire
// lifetime of an effect. In Rust terms, a RefCell. We use it to store the
// "destroy" function that is returned from an effect, because that is stateful.
Expand Down Expand Up @@ -7987,6 +8015,57 @@ function warnOnHookMismatchInDev(currentHookName) {
}
}

function warnIfAsyncClientComponent(Component, componentDoesIncludeHooks) {
{
// 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.
var 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.
var componentName = getComponentNameFromFiber(currentlyRenderingFiber$1);

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.

var root = getWorkInProgressRoot();
var rootRenderLanes = getWorkInProgressRootRenderLanes();

if (root !== null && includesBlockingLane(root, rootRenderLanes)) {
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) {
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 @@ -8156,18 +8235,20 @@ function renderWithHooks(
}
}

finishRenderingHooks(current, workInProgress);
finishRenderingHooks(current, workInProgress, Component);
return children;
}

function finishRenderingHooks(current, workInProgress) {
// 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$1.current = ContextOnlyDispatcher;

function finishRenderingHooks(current, workInProgress, Component) {
{
workInProgress._debugHookTypes = hookTypesDev;
} // This check uses currentHook so that it works the same in DEV and prod bundles.
var 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$1.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.

var didRenderTooFewHooks = currentHook !== null && currentHook.next !== null;
Expand Down Expand Up @@ -8240,7 +8321,12 @@ function finishRenderingHooks(current, workInProgress) {
var 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);

error(
Expand Down Expand Up @@ -8280,7 +8366,7 @@ function replaySuspendedComponentWithHooks(
props,
secondArg
);
finishRenderingHooks(current, workInProgress);
finishRenderingHooks(current, workInProgress, Component);
return children;
}

Expand Down
Loading

0 comments on commit d8c7f59

Please sign in to comment.