Bug: `useId()` not working inside `<Suspense>`

Question

Bug: `useId()` not working inside `<Suspense>`

brillout opened this issue 3 years ago · 28 comments

useId() doesn't return a stable ID when used inside <Suspense>.

function App() {
  return (
    <React.Suspense fallback={<p>I'm lazy loaded...</p>}>
      <LazyComponent />
    </React.Suspense>
  )
}

function LazyComponent() {
  const id = useId()
  console.log('id:', id)

  // The usual `throw promise` technique
  // ...
}

Expected behavior: The logged ID id: :r0: to stay :r0: (i.e. the ID is stable).

Current behavior: The logged ID changes: id: :r0:, then id: :r1:, then id: :r2:, etc.

React version: 18.1.0.

Reproduction

See github.com/brillout/react-bug-useId-suspense.

CodeSandbox: https://codesandbox.io/s/github/brillout/react-bug-useId-suspense?file=/index.tsx

Additional Context

This is a blocker for useAsync, Telefunc and probably many other React tools.

Answer 1 · 2022-06-03T13:45:36.000Z

This seems to be completely expected. When component suspends during initial render it is essentially not even mounted so it can't store any associated state, that is useState, useRef, etc. useId is not an exception

Answer 2 · 2022-06-09T10:20:39.000Z

@vkurchatkin That's besides the point: React does internally know that it's the same React element and, as a user, I'd expect React to provide the same const id = useId() since it's the same React element.

I believe the mental model here to be "same id" <=> "same React element".

That's why I'd say it's a bug and not a feature request.

Besides terminology, being able to use useId() with <Suspense> would be very convenient.

For me, it's actually the number 1 use case for useId(). (But I'm probably missing other use cases for useId().)

And, concretely, it's a blocker for the libraries I'm developing. For example:

import { useAsync } from 'react-streaming'

function StarWarsMovies() {
  return (
    <div>
      <p>List of Star Wars movies:</p>
      <Suspense fallback={<p>Loading...</p>}>
        <MovieList />
      </Suspense>
    </div>
  )
}

// This component is isomorphic: it works on both the client-side and server-side.
// The data fetched while SSR is automatically passed and re-used on the client for hydration.
function MovieList() {
  const movies = useAsync(async () => {
    const response = await fetch('https://star-wars.brillout.com/api/films.json')
    return response.json()
  })
  return (
    <ul>
      {movies.forEach((movie) => (
        <li>
          {movie.title} ({movie.release_date})
        </li>
      ))}
    </ul>
  )
}

But this is currently not possible because useId() doesn't work with <Suspsense>.

Answer 3 · 2022-06-09T12:07:40.000Z

That's besides the point: React does internally know that it's the same React element

I don't think that it actually the case when a component suspends on initial render. There is simple no instance of the component in this case, so it can't be "the same"

For me, it's actually the number 1 use case for useId(). (But I'm probably missing other use cases for useId().)

The use case is spelled out in the documentation:

useId is a hook for generating unique IDs that are stable across the server and client,
while avoiding hydration mismatches.

No indication that it should work as you expect or support any use cases beyond hydration.

The reality is that you need for data fetching with Suspense you need to provide an explicit key. Every library in existance (e.g. React-Query) does just that.

Answer 4 · 2022-06-09T13:57:34.000Z

The reality is that you need for data fetching with Suspense you need to provide an explicit key. Every library in existance (e.g. React-Query) does just that.

Yes, precisely, and being able to use useId() with <Suspense> would free users and library authors from the need for a cache key.

I believe using useId() is more React idiomatic than a cache key. For example useAsync() fits the React mental model like a glove.

I don't think that it actually the case when a component suspends on initial render. There is simple no instance of the component in this case, so it can't be "the same"

Doesn't React preserve siblings when a React component does a throw promise? That's what I would expect.

If yes, then React must have an internal bookkeeping to know which throw promise corresponds to which React element.

And, in that case, it makes sense to say "this is the same React element".

Answer 5 · 2022-06-09T18:14:35.000Z

I believe using useId() is more React idiomatic than a cache key. For example useAsync() fits the React mental model like a glove.

It can be idiomatic at the very least because it doesn't work and is based on a property of useId that never existed. Moreover it works strictly in "Fetch-on-Render" paradigm and for Suspense "Render-as-You-Fetch" is considered preferred and idiomatic. " Render-as-You-Fetch" requires explicit keys to populate the cache before you even start rendering and ids from useId are even known.

Answer 6 · 2022-06-09T19:41:43.000Z

I'm not sure I understand the problem.

For example:

function TodoList() {
  const todoList = useAsync(async () => {
    const response = await fetch('https://api.todo-list.com')
    return response.json()
  })

  return (
    <ul>
      {todoList.forEach((todoItem) => (
        <li>
          {todoItem.text}
        </li>
      ))}
    </ul>
  )
}

function App() {
  <>
    <SideBar>
      <Navigation />
    </SideBar>
    <MainView>
      <TodoList />
    </MainView>
  </>
}

Where do you see a problem with that?

This seems React idiomatic to me: the fetched data todoList of the component <TodoList/> is refetched only if useId() changes, that is when <TodoList/> is unmounted and mounted again. As a user, this is what I would expect.

Answer 7 · 2022-06-09T20:32:57.000Z

I don't see a problem with that, except:

it is not really possible implement (if you don't consider using fn.toString() as key)
it is definitely not idiomatic use of Suspense, since Suspense is design with Render-as-You-Fetch strategy in mind, which means that fetching has to be triggered outside of the component

Answer 8 · 2022-06-11T18:58:33.000Z

it is definitely not idiomatic use of Suspense

It's a matter of opinion, I don't see that to be a given fact.

Telefunc is designed around the idea of Fetch-on-Render while circumventing Waterfalls.

The central idea of Telefunc is to collocate RPC with Components. And Render-as-You-Fetch doesn't fit Telefunc's world.

Answer 9 · 2022-06-12T20:28:54.000Z

I guess arguing whether it is or isn't idiomatic is beside the point. The actual point is: useId never worked the way you want it work nor it says anywhere in the documentation that it should work like this.

Answer 10 · 2023-01-12T16:52:20.000Z

The way I read the documentation for useId is that the ID should stay stable for the same position in the parent component tree. If a component suspends and has to be re-rendered later, that does not change its position in the parent tree and hence the useId output should stay the same as well. Deviating from that does sound like a bug.

And yes, this would be an incredibly useful behaviour for libraries that need ad-hoc cache keys for fetching, processing, etc. For example, the @pmndrs/react-three-lightmap library computes the lightmap based on passed-in scene content which is a ReactNode. Right now to suspend while computation happens I will need to ask the developer for an arbitrary cache key string. Relying on useId would conveniently produce an implicit key for me instead.

Answer 11 · 2023-01-21T10:58:42.000Z

@brillout It's unclear why you would want to call the useId hook, instead of using some kind of natural key, which should be there in all cases if the server knows what it's processing. Does your use case not have a natural key? Or is there a reason not to use it?

Answer 12 · 2023-01-21T11:03:05.000Z

@Inwerpsel I want to make the key optional. So that, by default, the data has the same lifespan than useState().

Because of this issue I cannot do that and I've to force my users to use a key. I'd rather offer a sensible default that works most of the time.

Imagine forcing users to define a key everytime they use useState(): that'd be silly. That's how I see it.

Answer 13 · 2023-01-21T11:09:00.000Z

It's basically the same point than @unframework made:

I will need to ask the developer for an arbitrary cache key string. Relying on useId would conveniently produce an implicit key for me instead.

(@unframework I like your username 😀 I'm also the author of vite-plugin-ssr which allows you to "unframework" React 😀.)

Answer 14 · 2023-01-21T16:27:01.000Z

I rather was asking for more details of the use case. If you're running React on the server, you're loading things from some sort of database, or static objects from code. If a task has to be suspended, it's doing something with some data. It needs some keys in order to find what to do, and the result set can also contain data with keys. Hence, most of the time, something that would qualify as a key is very likely to be present in scope already. Especially on the server.

You maybe misunderstood what I meant with "natural key". I don't mean to require a key as part of your API, but that the code that would use useId might be able to derive a key from something else. Could be using a prop or function argument, or a combination of them.

I think useId was mostly built as an alternative to existing implementations which did this less efficiently in user land and were unreliable on the server. But if you already have something that qualifies as a key, that's always better than calling an additional hook.

Now maybe your use case is entirely different and this doesn't apply. In fact I feel that might be the case. But if you don't provide more than the minimum detail, then people will just assume the most general use case and won't see the relevancy. Can you maybe point to the relevant lines of code where you need this behavior? Or some basic explanation of what purpose these IDs have in the application, across the line. How many components would be using the thing that uses useId under the hood, what does it mean for a component to use that thing?

Answer 15 · 2023-01-21T17:00:16.000Z

The use case is this:

This is a blocker for useAsync, Telefunc and probably many other React tools.

In particular the former (https://github.com/brillout/react-streaming#useasync) which is representative for why I want to use useId().

derive a key from [...] function argument

While it would work, the problem is that it'd effectively become an infinite cache. When do you invalidate the cache?

The motivation to use useId() is essentially to have a cache the same way useState() has a cache (same lifespan).

Answer 16 · 2023-01-21T18:20:59.000Z

Ok, that's which libraries you want to use it in. Now, again, what will you do with those IDs? It doesn't matter you have multiple use cases in mind. Just pick the most convincing one and provide some detail to make a solid case for why useId has to be made more complex for everyone that uses React.

What could you do with useId inside Suspense that you couldn't do as well with your own internal ID generation logic? Is this something that many people will be doing? Or is it fulfilling some very niche piece of functionality in a library that could just as well include this logic itself?

It's definitely not a "bug" (as in an oversight) that requires a quick look or quick fix. It's clear from the implementation that it's not trivial to make work with suspense. useId generates the ID from the position in the tree. Addressing the limitation would add complexity that may not be worth it if the use case is not general enough.

When do you invalidate the cache?

I didn't mean cache at all. My point was in general being able to use useId in that way is not very useful, because in such contexts something to use as a key would likely be present anyway. That key would in almost all cases be easier to work with and more semantic than the keys React generates per rendering.

Answer 17 · 2023-04-04T20:33:15.000Z

@vkurchatkin

useId is explicitly documented to work as others are expecting it to:

From https://react.dev/reference/react/useId#why-is-useid-better-than-an-incrementing-counter

Inside React, useId is generated from the “parent path” of the calling component.

This implies that as long as the parent path is the same, the first use of useId in a render function should always generate the same ID.

Answer 18 · 2023-04-04T22:18:14.000Z

I haven't checked with the team, but I doubt that the intention of the API is for it to be reliable as a cache key. I think the expectation is more "it's guaranteed to not clash" and "it's guaranteed to remain stable while a component is mounted", not "it's guaranteed to stay stable between attempts to mount".

Answer 19 · 2023-04-17T11:41:37.000Z

I suffered from this today and arrived at this issue from a different misunderstanding.

React doesn't behave the same if the component suspends in the initial render vs. in a later render. I wasn't expecting this. Once I discovered it, I thought "oh, then if I use useId, I won't get a stable one either if I suspend during the first render, but I will if I get to suspend later", and indeed that was the case. The same applies to useCallback, useRef...

I think this is confusing because IMO, we shouldn't have to make the distinction between while a component is mounted vs. between attempts to mount.

Answer 20 · 2023-04-17T11:50:35.000Z

I haven't checked with the team, but I doubt that the intention of the API is for it to be reliable as a cache key. I think the expectation is more "it's guaranteed to not clash" and "it's guaranteed to remain stable while a component is mounted", not "it's guaranteed to stay stable between attempts to mount".

My thinking is that React must internally know that it's the same React "element", since otherwise React wouldn't be able to resolve the <Suspense> boundary. So I'm inclined to think that, in principle, it should be possible to make useId() also work for <Suspense>.

Answer 21 · 2023-06-08T14:32:41.000Z

I think this issue goes beyond stabilizing the id across suspension on client-side updates. This also affects SSR and client hydration. I recently ran into an issue where the id generated on the server was different than the id generated on the client which ultimately caused hydration issues.

Let's say we have an SSR application that fetches data using some modern "fetch-as-you-render" library to load data from an external service. Now when rendering this on the server, the upper suspense boundary is going to suspend on the server and once the request resolved react fizz streams the html to the client. Now for the client we don't want to repeat the same request so instead, the server is injecting a small <script>...</script> snippet that will add the services response to some global "cache". Rendering now on the client means that we'd simply not suspend and just read from that global "cache". All good but this means now that our server is suspending once while the client is not, which (I think) then causes the same problem.

I've created a small reproducible example: https://codesandbox.io/s/react-use-id-unstable-37kndi?file=/src/App.js

Running this will cause some hydration error because of the id, and in this case it is showing the use case for accessibility attributes as advertised in react docs and not "just" cache keys

Answer 22 · 2023-12-30T21:04:46.000Z

Yea, this is still happening, and is something I wanted to use for react native as well but it is always changing

Answer 23 · 2023-12-30T22:10:32.000Z

Trying to do somethink with this

Answer 24 · 2024-04-05T20:01:12.000Z

My thinking is that React must internally know that it's the same React "element", since otherwise React wouldn't be able to resolve the <Suspense> boundary.

Could someone confirm if this is actually the case? Given that Suspense-aware hooks need an external cache (not to be confused with the cache keys mentioned here; alluded to in #28588), it was my understanding that when a component suspends by throwing a Promise, that actually causes the tree up to the nearest <Suspense> boundary to unmount, losing the states of all hooks in scope. React then decides (sometime later) to attempt to re-render the component, which may succeed if no Promise/Error is thrown during that attempt.

If that is the case, then useId() losing its state would make sense in the context of Suspense, even if that isn't the ideal behavior. I'd appreciate if the Suspense docs were at least updated to clarify this, because if my understanding is correct, this is not intuitive behavior.

Answer 25 · 2024-07-04T20:04:36.000Z

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

Answer 26 · 2024-07-04T20:07:22.000Z

It's still very much relevant, as it would significantly simplify the libraries I maintain (and I guess many other libraries).

Answer 27 · 2024-10-02T21:05:26.000Z

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

Answer 28 · 2024-10-02T21:06:39.000Z

Still relevant.