vercel / next.js

The React Framework
https://nextjs.org
MIT License
127.41k stars 27.03k forks source link

Using `use` in a Server Component freezes #42469

Open arackaf opened 2 years ago

arackaf commented 2 years ago

Verify canary release

Provide environment information

---->npx --no-install next info

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.6.0: Mon Aug 22 20:17:10 PDT 2022; root:xnu-8020.140.49~2/RELEASE_X86_64
Binaries:
  Node: 16.11.1
  npm: 8.18.0
  Yarn: 1.22.17
  pnpm: N/A
Relevant packages:
  next: 13.0.2
  eslint-config-next: 13.0.0
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

Chrome

How are you deploying your application? (if relevant)

n/a

Describe the Bug

I have a pretty rudimentary RSC demo coded up here:

https://github.com/arackaf/next-13-data-fetching-blog-post/blob/feature/use-bug-repro/app/Todos.tsx

When I unwrap the promise in the component using the data using use then Node freezes up. I see about 15+ Node tasks in activity monitor, with one of them at about 150%. Using await works fine.

Eventually, after killing the Next watch, I get this

<--- Last few GCs --->

[5793:0x7f8b54a00000] 256154 ms: Mark-sweep 4018.6 (4130.7) -> 4003.6 (4126.9) MB, 5916.0 / 0.8 ms (average mu = 0.093, current mu = 0.015) allocation failure scavenge might not succeed [5793:0x7f8b54a00000] 256564 ms: Scavenge 4019.9 (4127.2) -> 4005.8 (4127.2) MB, 359.2 / 0.5 ms (average mu = 0.093, current mu = 0.015) allocation failure

<--- JS stacktrace --->

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory 1: 0x101978545 node::Abort() (.cold.1) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 2: 0x100679a49 node::Abort() [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 3: 0x100679bbf node::OnFatalError(char const, char const) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 4: 0x1007f9387 v8::Utils::ReportOOMFailure(v8::internal::Isolate, char const, bool) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 5: 0x1007f9323 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate, char const, bool) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 6: 0x10099aa35 v8::internal::Heap::FatalProcessOutOfMemory(char const) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 7: 0x10099ea7d v8::internal::Heap::RecomputeLimits(v8::internal::GarbageCollector) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 8: 0x10099b35d v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 9: 0x10099887d v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 10: 0x1009a5bb0 v8::internal::Heap::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 11: 0x1009a5c31 v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 12: 0x100972d87 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 13: 0x100d2855e v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long, v8::internal::Isolate*) [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node] 14: 0x1010d1e99 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/usr/local/Cellar/nvm/0.38.0/versions/node/v16.11.1/bin/node]

Expected Behavior

Use should work the same as await

Link to reproduction

https://github.com/arackaf/next-13-data-fetching-blog-post/blob/feature/use-bug-repro/app/Todos.tsx

To Reproduce

Clone that repo at that branch, and run. Swap the use calls out for the commented out await calls (after clearing the rogue Node processes) to see it work.

tumetus commented 2 years ago

I am experiencing this too.

pix303 commented 2 years ago

From beta docs I remember use is reserved for client only. So maybe you have to add 'use client' on top. However in my experience it doesn't work: it goes in infinite loop

apostolos commented 2 years ago

@arackaf use() is reserved for client components. You need to await the promises and it should just work.

@pix303 probably because the awaited promise is not stable (referentially equal) between renders. A good explanation of what's happening can found in the RFC: https://github.com/acdlite/rfcs/blob/first-class-promises/text/0000-first-class-support-for-promises.md#usepromise On @arackaf's example it should work fine since the promises are passed as props (unless the parent component re-renders the Promises will be stable).

timneutkens commented 2 years ago

Just checked in on this with @sebmarkbage. The component you currently have is an async function server component that is using use(), this is supposed to be a hooks lint warning which is on the list of things to implement. For async function you have to use await. If it's not an async server component use works either should work:

// Async function server component
export const Todos: TodosType = async (props: Props) => {
  const todos = await props.todos;
  const colors = await props.colors;

  return (
    <section>
      <TodoListFiltersHeader />
      <ul>
        {(todos?.data ?? []).map((todo, idx) => (
          <li key={idx} style={{ color: colors.data[todo.color] }}>
            {todo.name} - {todo.priority}
          </li>
        ))}
      </ul>
    </section>
  );
};

// Server Component
export const Todos: TodosType = (props: Props) => {
  const todos = use(props.todos);
  const colors = use(props.colors);

  return (
    <section>
      <TodoListFiltersHeader />
      <ul>
        {(todos?.data ?? []).map((todo, idx) => (
          <li key={idx} style={{ color: colors.data[todo.color] }}>
            {todo.name} - {todo.priority}
          </li>
        ))}
      </ul>
    </section>
  );
};
arackaf commented 2 years ago

@timneutkens thanks a ton! fwiw, my feedback would be that if it's not possible to get use to be functional in a non-async RSC, I hope ya'll will consider making this a client-only feature; that's a pretty massive footgun.

Am I correct in assuming this breaks since, under the covers, use works by throwing a promise, which, from an async function, gets turned into the return value from the async function, which is not what the calling RSC was expecting?

acdlite commented 2 years ago

my feedback would be that if it's not possible to get use to be functional in a non-async RSC, I hope ya'll will consider making this a client-only feature; that's a pretty massive footgun.

To be clear: you can use use in non-async components. You can't use it in async components. That's what the lint rule will enforce.

However there's no reason for it to crash with an out-of-memory exception. React should throw an error with a helpful message. We'll fix that.

timneutkens commented 2 years ago

Is it okay to leave this open so that we have something to track implementing the lint rule and can get back to you when it is implemented?

arackaf commented 2 years ago

@timneutkens done! Was just trying to lessen your backlog by if just by a tiny amount :)

arackaf commented 2 years ago

@acdlite sorry dumb typo on my part. I of course meant to say

my feedback would be that if it's not possible to get use to be functional in async RSCs, I hope ya'll will consider...

so wait - you said you're working on a fix for this? So, pending your fix, will use be usable in all async RSCs? Or just some of them?

arackaf commented 2 years ago

@acdlite ohhh - re-reading your comment for the third time, it looks like you want Promises to be valid React nodes, so my particular use of use in an async component should not crash after your fix, but it also won't do what I want. Is that about right?

KSubedi commented 2 years ago

From beta docs I remember use is reserved for client only. So maybe you have to add 'use client' on top. However in my experience it doesn't work: it goes in infinite loop

I thought it was something with my setup at first and spent a lot of time debugging it. Have you found a way around this? Seems like the promise gets resolved, network inspector shows that the results come through and the fetch call goes through, but the component just gets stuck in an infinite loop. Any guidance would be appreciated.

arackaf commented 2 years ago

@KSubedi did you read the thread? use cannot be used in an async RSC. It can only be used in a non-async RSC.

KSubedi commented 2 years ago

@KSubedi did you read the thread? use cannot be used in an async RSC. It can only be used in a non-async RSC.

I did read the thread. I apologize if it was not clear on my comment but I am having that infinite loop issue on a non-async client component just like @pix303 mentioned.

apostolos commented 2 years ago

Hi @KSubedi regarding use() in client components, you can read why you get infinite loops here: https://github.com/vercel/next.js/issues/42180#issuecomment-1303553483 and you can read this for the fix: https://github.com/vercel/next.js/issues/42180#issuecomment-1303676756

EDIT: also make sure you are on v13.0.2 or higher

acdlite commented 2 years ago

@arackaf https://github.com/vercel/next.js/issues/42469#issuecomment-1303958773

ohhh - re-reading your comment for the third time, it looks like you want Promises to be valid React nodes, so my particular use of use in an async component should not crash after your fix, but it also won't do what I want. Is that about right?

Yeah. I think we'll still throw an error when we detect this (with a helpful message) but there's no reason it should cause an out-of-memory exception.

I'll update the original comment to clarify.

arackaf commented 2 years ago

@acdlite thanks for clarifying!

KSubedi commented 2 years ago

Hi @KSubedi regarding use() in client components, you can read why you get infinite loops here: #42180 (comment) and you can read this for the fix: #42180 (comment)

EDIT: also make sure you are on v13.0.2 or higher

Thank you so much, wrapping it in cache worked and its good to know why the behavior is like that.