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.

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

Bug: Don't crash the app if an async component is accidentally used on the client #26801

Open
2 tasks
acdlite opened this issue May 10, 2023 · 3 comments
Open
2 tasks
Labels
Component: Suspense React Core Team Opened by a member of the React Core Team

Comments

@acdlite
Copy link
Collaborator

acdlite commented May 10, 2023

We don't yet officially support async/await in client components. (Refer to the RFC for more details: https://github.com/acdlite/rfcs/blob/first-class-promises/text/0000-first-class-support-for-promises.md#why-cant-client-components-be-async-functions)

Confusingly, though, it does sometimes work, because the primitives are there to support it in the future.

However, if you accidentally render an async component on the client (for example, when refactoring a Server Component to a client one), React should fail gracefully. Right now, in some cases, it will instead crash the app by falling into an infinite loop.

Let's use this issue to track how we'll improve the DX for preventing async components (and async/await data fetching more generally) on the client.

This is not about implementing support for async/await on the client. Although we will probably do that eventually, for now we're just focused on making it fail more gracefully.

In the meantime, if you're using a Server Components enabled framework like Next.js, you should prefer to do as much data fetching as possible in Server Components. For the remaining client cases, use an established framework like useSWR or React Query.

  • Detect and prevent an infinite loop caused by when uncached promise is rendered outside of a Suspense boundary.
  • Add a warning if an uncached promise is detected outside of a transition.
@acdlite acdlite added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label May 10, 2023
@acdlite acdlite added Component: Suspense React Core Team Opened by a member of the React Core Team and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels May 10, 2023
@KaramveerSidhu
Copy link

@acdlite Thank you for opening this issue and having a look in this direction. This is something that's really required.

@saudpunjwani101
Copy link

saudpunjwani101 commented Jun 6, 2023

I'm still "Error: Not implemented. at Object.getCacheForType" on the server when cache the fetch with "use".

const fetchCities = cache(() => fetch(${getAnalyticsUrl()}/rest/cities))

const CitiesPage = () => {
let cities = use(fetchCities())
...

It works on the client though.

acdlite added a commit to acdlite/react that referenced this issue Jun 27, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 27, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 27, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 27, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 27, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit to acdlite/react that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any
loading UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
acdlite added a commit that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any loading
UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See #26801 for more details.
github-actions bot pushed a commit that referenced this issue Jun 29, 2023
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any loading
UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See #26801 for more details.

DiffTrain build for [fc80111](fc80111)
@natsandbox
Copy link

I am using Next.js and I accidentally put 'use client' and async together and the page was rending well.

However when I made a change with the page open in the browser and recompiled the browser completely froze up.

If I made changes and recompiled without having the page in question open, it would recompile without issues.

jerrydev0927 added a commit to jerrydev0927/react that referenced this issue Jan 5, 2024
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any loading
UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook/react#26801 for more details.

DiffTrain build for [fc801116c80b68f7ebdaf66ac77d5f2dcd9e50eb](facebook/react@fc80111)
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any loading
UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See facebook#26801 for more details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Suspense React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

No branches or pull requests

4 participants