-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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: useId()
not working inside <Suspense>
#24669
Comments
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 |
@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 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 For me, it's actually the number 1 use case for 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 |
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"
The use case is spelled out in the documentation:
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. |
Yes, precisely, and being able to use I believe using
Doesn't React preserve siblings when a React component does a If yes, then React must have an internal bookkeeping to know which And, in that case, it makes sense to say "this is the same React element". |
It can be idiomatic at the very least because it doesn't work and is based on a property of |
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 |
I don't see a problem with that, except:
|
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. |
I guess arguing whether it is or isn't idiomatic is beside the point. The actual point is: |
The way I read the documentation for 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 |
@brillout It's unclear why you would want to call the |
@Inwerpsel I want to make the key optional. So that, by default, the data has the same lifespan than 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 |
It's basically the same point than @unframework made:
(@unframework I like your username 😀 I'm also the author of vite-plugin-ssr which allows you to "unframework" React 😀.) |
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 I think 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 |
The use case is this:
In particular the former (https://github.com/brillout/react-streaming#useasync) which is representative for why I want to use
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 |
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 What could you do with 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.
I didn't mean cache at all. My point was in general being able to use |
From https://react.dev/reference/react/useId#why-is-useid-better-than-an-incrementing-counter
This implies that as long as the parent path is the same, the first use of |
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". |
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 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. |
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 |
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 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 |
Yea, this is still happening, and is something I wanted to use for react native as well but it is always changing |
Trying to do somethink with this |
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 If that is the case, then |
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! |
It's still very much relevant, as it would significantly simplify the libraries I maintain (and I guess many other libraries). |
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! |
Still relevant. |
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! |
Bump
…On Wed 1. Jan 2025 at 00:04, github-actions[bot] ***@***.***> wrote:
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!
—
Reply to this email directly, view it on GitHub
<#24669 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHVQRVNWPNZWVXLEPF5WJ32IMPJRAVCNFSM5XYJUN3KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TENJWGY3TINRTGY2Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
useId()
doesn't return a stable ID when used inside<Suspense>
.Expected behavior: The logged ID
id: :r0:
to stay:r0:
(i.e. the ID is stable).Current behavior: The logged ID changes:
id: :r0:
, thenid: :r1:
, thenid: :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.
The text was updated successfully, but these errors were encountered: