Conversation
Size changesDetails📦 Next.js Bundle Analysis for react-devThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
| <button onClick={handleRefresh} disabled={isPending}> | ||
| {isPending ? 'Refreshing...' : 'Refresh'} | ||
| </button> | ||
| <ErrorBoundary fallback={<p>⚠️Something went wrong</p>}> |
| async function getAlbums(artistId) { | ||
| // Add a fake delay to make waiting noticeable. | ||
| await new Promise(resolve => { | ||
| setTimeout(resolve, 80); |
There was a problem hiding this comment.
Is this delay a bit too short? The loading state is very brief in the sandbox. I keep trying to see the loading state in the other two tabs, but the promise resolves too quickly for the difference between hovering/not hovering to be clear.
Trying it in a forked sandbox, 1second seems better.
|
|
||
| function handleRefresh() { | ||
| startTransition(() => { | ||
| setAlbumsPromise(fetchData('/the-beatles/albums')); |
There was a problem hiding this comment.
Adding some artificial delay in fetchData would make the isPending part of the code and the <Suspense fallback={<p>Loading...</p>}> more useful.
However, without async/await, any potential delay on the refresh will not actually disable the button. I added some delay in a forked sandbox, and had to add async await in front of fetchData for it to correctly disable the refresh button. But I guess that would make the error boundary useless?
As Maxwell said, seems like there's a few issues with this example.
| ```js | ||
| function Albums() { | ||
| // 🔴 fetch creates a new Promise on every render. | ||
| const albums = use(fetch('/albums')); |
There was a problem hiding this comment.
Adding more examples of ways to recreate promises between renders would be helpful, such as some ways you can create a new promise between renders
- calling a function that creates a new uncached promise. ie fetch
- adding a
.then,.catch,.finallyon a cached promise - calling an async function
- calling functions from the Promise object i.e. Promise.resolve
| } | ||
| ``` | ||
| But using `await` in a [Server Component](/reference/rsc/server-components) will block its rendering until the `await` statement is finished. Passing a Promise from a Server Component to a Client Component prevents the Promise from blocking the rendering of the Server Component. |
There was a problem hiding this comment.
This framing reads a bit like we should always prefer use() over awaiting in a Server Component. It might help to clarify that use() doesn’t inherently avoid blocking and that both approaches still depend on Suspense, and the reason we prefer use() is because we need to use a client component.
There was a problem hiding this comment.
Agree! To add to this, the "RSC blocks rendering" framing can read as if it blocks the entire app, when that's not really the case. Maybe we can mention that if we move the async work to a child async Server Component wrapped in <Suspense>, it enables SSR streaming and avoids blocking.
I think this Deep Dive (or somewhere else in these use() docs) would benefit from mentioning the async SC + Suspense pattern and clarifying when each approach is preferable. Also, it's worth noting that neither the Suspense docs nor the RSC docs currently cover this "stremaing SSR" pattern either. Could be a good follow-up to add streaming examples there as well :)
There was a problem hiding this comment.
One thing I'm not fully clear on: beyond the bundle size tradeoff, are there other meaningful tradeoffs between the two approaches? Might be worth clarifying that in the Deep Dive as welll
| #### Fetching data with `useEffect` {/*fetching-data-with-useeffect*/} | ||
| Without `use`, a common approach is to fetch data in an Effect and update state when the data arrives. This requires managing loading and error states manually, and the component renders empty on first paint before the Effect fires. |
There was a problem hiding this comment.
would it make sense to mention this isn't recommended and you should use use. Maybe thats obvious
| <Pitfall> | ||
| ##### Promises passed to `use` must be cached {/*promises-must-cached*/} |
There was a problem hiding this comment.
I really like this section as I think its a common pitfall
| // Set status fields so React can read the value | ||
| // synchronously if the Promise resolves before | ||
| // `use` is called (e.g. when preloading on hover). | ||
| promise.status = 'pending'; |
There was a problem hiding this comment.
interesting hadn't known it was preferred too set the status fields like this
| ```jsx | ||
| function Albums({ albumsPromise }) { | ||
| try { | ||
| // ❌ Don't wrap `use` in try-catch |
There was a problem hiding this comment.
does this trip quite a few people up? Its interesting because its pretty obvious when you know use throws internally. Good callout forsure
|
Overall looks great. Nice work! |
| * `promise`: A [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) whose resolved value you want to read. The Promise must be [cached](#caching-promises-for-client-components) so that the same instance is reused across re-renders. | ||
| * The `use` API must be called inside a Component or a Hook. | ||
| * When fetching data in a [Server Component](/reference/rsc/server-components), prefer `async` and `await` over `use`. `async` and `await` pick up rendering from the point where `await` was invoked, whereas `use` re-renders the component after the data is resolved. |
There was a problem hiding this comment.
I'm curious why this caveat was removed? Is it no longer the preferred way?
I know it could depend of the use case, but I did follow this suggestion as it helps shipping smaller initial bundles, instead of use(dataPromise) where the component where it was called from is still part of the initial bundle.
| * `use` must be called inside a Component or a Hook. | ||
| * `use` cannot be called inside a try-catch block. Instead, wrap your component in an [Error Boundary](#displaying-an-error-with-an-error-boundary) to catch the error and display a fallback. | ||
| * Promises passed to `use` must be cached so the same Promise instance is reused across re-renders. [See caching Promises below.](#caching-promises-for-client-components) | ||
| * When passing a Promise from a Server Component to a Client Component, its resolved value must be serializable to pass between server and client. Data types like functions aren't serializable and cannot be the resolved value of such a Promise. |
There was a problem hiding this comment.
the 'use client' docs has a section with a list of serializable props, we could link to this page
| * When passing a Promise from a Server Component to a Client Component, its resolved value must be serializable to pass between server and client. Data types like functions aren't serializable and cannot be the resolved value of such a Promise. | |
| * When passing a Promise from a Server Component to a Client Component, its resolved value must be [serializable](https://react.dev/reference/rsc/use-client#serializable-types) to pass between server and client. Data types like functions aren't serializable and cannot be the resolved value of such a Promise. |
There was a problem hiding this comment.
[...] to pass between server and client.
nit: for me this phrase feels redundant here, and it reads a bit weird when I first read it. It also subtly shifts from React terminology to infra terminology, which could be confusing since RSC don't necessarily imply a physical server. I would just drop that phrase:
| * When passing a Promise from a Server Component to a Client Component, its resolved value must be serializable to pass between server and client. Data types like functions aren't serializable and cannot be the resolved value of such a Promise. | |
| * When passing a Promise from a Server Component to a Client Component, its resolved value must be serializable. Data types like functions aren't serializable and cannot be the resolved value of such a Promise. |

Preview
cc @samselikoff @gaearon @stephan-noel @aurorascharff @brenelz @MaxwellCohen @hernan-yadiel