Skip to content
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

Boundary Serialization - Props must be serializable for components in "use client" file warning #74343

Open
inducingchaos opened this issue Dec 28, 2024 · 8 comments
Labels
bug Issue was opened via the bug report template. Developer Experience Issues related to Next.js logs, Error overlay, etc. Linting Related to `next lint` or ESLint with Next.js. Partial Prerendering (PPR) Related to Partial Prerendering.

Comments

@inducingchaos
Copy link

inducingchaos commented Dec 28, 2024

Link to the code that reproduces this issue

https://github.com/inducingchaos/riley-barabash

To Reproduce

(Ignore code repro - SEE #46795)

In my experience: just create any component in a "use client" file that accepts a function, dispatch for state, or anything that is non-serializable as props.

Current vs. Expected behavior

Current: Typescript (Next.js version) will give a warning (the title) to either a) make props serializable, or b) to postfix the prop name with 'Action'. This has led to an unnecessary amount of renamings simply to avoid this warning - where the majority of cases don't have anything to do with Server Actions at all.

Expected: No warnings when passing props unrelated to SA's or the client-server boundary, especially in cases where client components are layered.

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 24.0.0: Mon Aug 12 20:54:30 PDT 2024; root:xnu-11215.1.10~2/RELEASE_X86_64
  Available memory (MB): 32768
  Available CPU cores: 16
Binaries:
  Node: 22.9.0
  npm: 10.8.3
  Yarn: N/A
  pnpm: 9.12.0
Relevant Packages:
  next: 15.1.1-canary.16 // There is a newer canary version (15.1.1-canary.22) available, please upgrade! 
  eslint-config-next: 14.2.11
  react: 19.0.0-rc-603e6108-20241029
  react-dom: 19.0.0-rc-603e6108-20241029
  typescript: 5.6.2
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Developer Experience, Linting, Partial Prerendering (PPR)

Which stage(s) are affected? (Select all that apply)

next dev (local)

Additional context

Didn't provide a repro because I'm lazy and I think there's enough context within the discussion thread I linked.

See: #46795

@inducingchaos inducingchaos added the bug Issue was opened via the bug report template. label Dec 28, 2024
@github-actions github-actions bot added Developer Experience Issues related to Next.js logs, Error overlay, etc. Linting Related to `next lint` or ESLint with Next.js. Partial Prerendering (PPR) Related to Partial Prerendering. labels Dec 28, 2024
@pawelblaszczyk5
Copy link

“use client” should be used only for marking the boundaries between client-server code - similarly to “use server”. Not every client component should be marked with it. Boundaries have additional restrictions, e.g. serializable props you mentioned

@inducingchaos
Copy link
Author

That's easy to say, and has been said - but it doesn't make a lot of sense in practice (or in this context), nor is it a viable solution to this problem.

First of all: I think that's a great model, and if this philosophy was more deeply & widely propagated (more importantly - the parts of the framework that actively oppose and prevent this are changed/fixed) it could work.

Model 1: What You're Suggesting

"use client" should be used only at the boundary where a client component is installed in a server-first architecture - almost like an "escape hatch" in a similar way to how Server Actions are used.

Benefits

  • Further framework/build optimizations??? Personally, I think this should be implemented on a compiler level, where Next.js just knows where the boundary is.
  • DX gain because the boundary is clearly marked? This seems minimal to me. Using unlabeled components (see below) throws all of these gains out the window.
  • A distinction to disallow non-serializable props? This additional "layer" seemingly creates more DX complications than UX errors. This can/should be solved in other ways.

Caveats

  • No clear distinction between server and client components IF it's not a boundary component. Requires more structural overhead and creates a "third" type of file (that which is a client component, but unlabeled). It's unintuitive.
  • Unlabeled Client Components behave like Server Components - try using Framer Motion, useEffect, or useState in one of these. In my experience, you can't.

Model 2: How 88% of Next.js devs use "use client"

"use client" (a client component) is antonymous to no declaration (a server component) and is used to send JS to the client, rather than pre-compiled HTML.

Benefits

  • Clear distinction between client/server: every component that needs this functionality is marked.
  • Easier to understand, already well understood.

Why This Warning Does NOT Make Sense

  • It can't be disabled on a by-line or by-file basis like other TS/ESLint errors.
  • Even following Model 1, littering the codebase with 'Action' postfixes on props doesn't make a lot of sense when 80% of error/warn cases are unrelated to Server Actions. The error is not intelligent enough to tell the difference between SA's and functions like state setters, which makes the 'Action' postfix more irrelevant.

Proposed Solution

We need to make a unanimous decision on which model to follow, and fix all parts of the framework, docs, and the understanding of the community accordingly.

There is clearly a misalignment here in both the judgement of internal contributors (hence the linting <> rule conflicts) and the way Next.js is being consumed.

Fix the thinking, then fix the details. This frustrating "warning" is clearly a manifestation of the conflict between these two different philosophies.

Conclusion

I see the need to prevent non-serializable props from crossing the client-server boundary — but this can't be the best solution. Using the model suggested here & in the discussion creates more warnings, errors, and headaches that will likely resurface in the future. This needs to be given a deeper consideration.

@pawelblaszczyk5
Copy link

“use server”, “use client” aren’t Next.js features. These are React feature that a few frameworks already implemented. Next.js merely warns you that you’re breaking React rules here. That’s the same as not marking every server code piece with “use server”

@inducingchaos
Copy link
Author

inducingchaos commented Dec 28, 2024

That's true, but your response isn't contributing to the solution... passing the blame to React doesn't solve problems that we're having in Next.js.

The Next.js team is known for overcoming issues, and taking responsibility for ones in the framework it is built on top of. There is a real issue here that needs to be given more effort than a 3-line copy-pasta from a different framework's docs - respectfully.

(and your last sentence didn't make sense to me - SC's aren't marked by design and SA's can be marked individually when a file or module marking isn't clear/sensical)

Edit:

The philosophies, errors, and the entire context in question are those of Next.js.

Using React as a scapegoat (instead of a consideration in a valid solution) is like if Resend shamelessly blamed AWS for email delivery problems.

When you consume a service as a brand, you become the layer that takes the hit for your users. Just wanted to drive this one home because Next & Vercel has such a great history of doing so — and your effort & stance on this antagonizes that.

@pawelblaszczyk5
Copy link

Okay - let’s assume for a moment that marking every client component with “use client” should be the way and this warning should be removed. Let’s say you’re working in a big code base where you aren’t the only developer. How would you know whether you can add a unserializable prop to component? Would you search for every single usage of the component and investigate it? How would you know whether you should optimize component props because they’ll be serialized in RSC payload or not? How would you prevent other developers from adding non-serializable props to your component that’s supposed to be used from server component? How would you prevent incorrect usages where someone tries to pass unserializable prop to component that acts as a boundary? The list could go on and on 😃

These directives and the model that React team has in mind aren’t perfect imho, but they have a huge advantage - they’re 1) statically analyzable 2) you see server-client boundary at a glance.

Both of which are huge when you are working with code which you didn’t author/at bigger scale/older code.

@inducingchaos
Copy link
Author

Okay - let’s assume for a moment that marking every client component with “use client” should be the way and this warning should be removed. Let’s say you’re working in a big code base where you aren’t the only developer. How would you know whether you can add a unserializable prop to component?

That's a really good question... something that I really haven't given much thought. Maybe another directive (lol) on top of "use client", so that you're not creating an "intermediary" version of a Client Component that creates the type of confusion I outlined above?

My main point so far is that when solving a (newer) problem like the serialization thing - breaking/changing what already works (the model/pattern) is usually not the best first step.

*OR* at the end of the day, keep pushing Model 1 but fix the broken parts... so that you can use useState and similar in an unmarked CC file. This just seems extremely unintuitive to me. Without "use client" or extreme file organization, this looks like a Server Component and currently behaves like one... BIG issue.

Would you search for every single usage of the component and investigate it?

I don't think this would be efficient, but it would be more reliable/predictable than the current behaviour until we come up with a better solution.

How would you know whether you should optimize component props because they’ll be serialized in RSC payload or not?

That's something that we need a (hopefully, different) solution for.

How would you prevent other developers from adding non-serializable props to your component that’s supposed to be used from server component?

We need a solution for that, I agree - but meddling with client/server directives is not it. Would love to hear some more opinions on both of our stances from people in the original discussion.

How would you prevent incorrect usages where someone tries to pass unserializable prop to component that acts as a boundary?

DX warning is probably the best preventative measure, but again, this needs to be a result of a different solution/approach, not the intermingling with "use client" directives that drastically modify the purpose and function of a file/component.

These directives and the model that React team has in mind aren’t perfect imho, but they have a huge advantage - they’re 1) statically analyzable 2) you see server-client boundary at a glance.

I do see your point. But instead of creating extra complexity on the (already confusing for new AppDir users) client/server dynamic — make the solution independent of this. Maybe this means a "boundary-only" directive (this is actually clever, as it's an edge-case like "server-only") or some kind of specifier/differentiator on the component itself, or how it's constructed.

I hope these answers make sense to you, and are productive to the goal.

@inducingchaos
Copy link
Author

On a better solution:

Thinking Process

  • What are the different ways we could statically mark a boundary for both the developer and the framework? (directives, function namings, variable exports, file structure...)
  • How can we best articulate/convey the meaning of the solution here? This is important, because our messaging will determine the level of clarity & understanding for users. (is it related closest to the Javascript/SSR boundary, serialization itself, Server Actions specifically...)
  • How can this actually be implemented in a non-obvious way (completely integrated/hidden, "use client" almost does this but not quite), or in a completely separate, independent way (another directive/philosophy/rule)? We can't have an in-between, because that creates the type of confusion that got us here.

I think issues like this need to be solved lowest-level. This is a matter of serialization of components in general (from my understanding), and should not be described using terms like "Server Actions" and "Client/Server".

Direction

I think we need to take a page out of Swift's playbook here. I'm not to informed on the relevance of protocols in Typescript, but following a similar concept:

Certain client components (boundaries, the ones installed in server components) need to be serializable.

That's it. Mark these "boundaries" as serializable — allow regular CC's to be marked at the component, file, or module level like SA's. This would create consistency in declaration patterns.

With this, we have created two separate concepts, two layers of thinking. Mitigated complexity through careful abstraction.

Using an additional directive, export, marking, or component type of some sort indicating:

  • "serializable" directive, or similar.
  • Serializable<JSX.Element> return type (although this would likely have to be the function type, rather than a return - and I'm not a fan of typing the component itself).
  • "boundary only" directive.

These are the most accurate solutions that I can think of - all somewhat more clear than the current implementation.

@inducingchaos
Copy link
Author

My favourite idea so far is typing the props as Serializable - this could be implemented similarly to Promise, which could be:

  • Statically analyzable, and able to statically check the params object it wraps
  • Related to the types of the props, which is directly conducive to the goal
  • Would allow client components to be client components
  • Clear that the component itself needs to accept serializable inputs - which is somewhat equivalent to arbitrarily naming a component, prop, or file
  • We wouldn't need to extend Typescript, instead just error on the type definition imported from Next

@inducingchaos inducingchaos changed the title Props must be serializable for components in "use client" file (for discussion #46795) Boundary Serialization - Props must be serializable for components in "use client" file warning Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Developer Experience Issues related to Next.js logs, Error overlay, etc. Linting Related to `next lint` or ESLint with Next.js. Partial Prerendering (PPR) Related to Partial Prerendering.
Projects
None yet
Development

No branches or pull requests

2 participants