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>}> |
There was a problem hiding this comment.
This ErrorBoundary does not seem to be working
| 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.
|
|
||
| #### 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! |
4 participants
Preview
cc @samselikoff @gaearon @stephan-noel @aurorascharff @brenelz @MaxwellCohen @hernan-yadiel