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

RFC: React Server Module Conventions v2 #227

Merged
merged 9 commits into from
Oct 25, 2022
Merged

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Oct 11, 2022

This is a new take on the original v1 of this proposal. See the discussion there for deeper context on the reason for the changes made.

Please watch the React Server Components intro talk and high level React Server Components RFC for some background context on this RFC.

View the rendered text

@sebmarkbage sebmarkbage changed the title React Module Conventions RFC v2 RFC: React Module Conventions v2 Oct 11, 2022
@sebmarkbage sebmarkbage changed the title RFC: React Module Conventions v2 RFC: React Server Module Conventions v2 Oct 11, 2022
Copy link
Contributor

@hamlim hamlim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed a few small typos/carryovers from the previous iteration of the RFC

text/0000-server-module-conventions.md Outdated Show resolved Hide resolved
text/0000-server-module-conventions.md Outdated Show resolved Hide resolved
text/0000-server-module-conventions.md Outdated Show resolved Hide resolved
text/0000-server-module-conventions.md Outdated Show resolved Hide resolved
text/0000-server-module-conventions.md Outdated Show resolved Hide resolved
sebmarkbage and others added 4 commits October 11, 2022 16:53
fix file import in rfc example
Co-authored-by: Matt Hamlin <matthewjameshamlin@gmail.com>
Co-authored-by: Matt Hamlin <matthewjameshamlin@gmail.com>
Co-authored-by: Matt Hamlin <matthewjameshamlin@gmail.com>
@sebmarkbage
Copy link
Collaborator Author

Thank you! ☺️

@redbar0n
Copy link

The implications of this is also that you shouldn't add the "use client" directive to all Client Components. Only the ones intended to be used directly by the Server.

This is confusing. Isn’t it effectively a «use server» directive?

Rather than telling the server to use a client component, the perspective would be to denote a client component for use on the server. The result is the same. But when looking at a client component it makes sense that it is the point of focus (not the server).

@redbar0n
Copy link

redbar0n commented Oct 12, 2022

ah, I see that my confusion was merely with the wording of the aforementioned paragraph. Since:

The real file never actually gets imported on the server.

So the «use client» directive is only for the server for the purpose of passing the component reference to the client where the component is executed. So «use client» kinda makes sense then.

@kylemh
Copy link

kylemh commented Oct 12, 2022

Interestingly, none of these use file extensions to separate the environments. Why is that? It would be equally useful to determine whether some code is intended to be used in Node.js, browsers, react-native or either by purely the file extension. For most npm packages there's not really any formal indication either way unless there's specific fallback implementations.

You mentioned this in the other thread, but - in my experience - in those scenarios either the project is all one environment or a folder separates the different environments allowing Webpack to bundle each file differently. For example:

some_monorepo
  --> react-app
  --> backend
  --> react-native-app

I know you're correct that the folders don't matter to Webpack - it just follows the trail of modules. That being said, this i think this structuring happens a lot and it happens for good reasons.

@bobaekang
Copy link

Instead of "use client" directive, how about a wrapper for client components that cross boundaries in the manner of React.memo() or React.lazy()? Maybe React.serializable()? Whatever it's called, a wrapper would look more familiar as API than a directive yet be no less viable for statically enforcing the rule.

@jplhomer
Copy link

Following up on my comments from the previous thread:

  • I really like this approach, because it's dramatically simpler and does not involve file name conventions.
  • This will make it easier for developers to migrate to RSC — most components will "just work."
  • The "client" wording issue is still unsolved, but I think the community might just get used to it. We haven't been able to come up with a better name.
  • The only remaining concern is with knowing where you are in the module graph while editing a single component. But with poison pills and good compiler errors, the feedback to developers should be really fast when they introduce a hook they cannot use. Plus, if server components are largely async, that's yet another indicator about where you are in the tree.

Great work getting this pushed through!

@gaearon gaearon merged commit d633174 into main Oct 25, 2022
@gaearon
Copy link
Member

gaearon commented Oct 25, 2022

We're going to go ahead with this. Very thankful to everyone for the discussion on the original issue.

@oljimenez
Copy link

oljimenez commented Oct 27, 2022

Following up on my comments from the previous thread:

  • I really like this approach, because it's dramatically simpler and does not involve file name conventions.
  • This will make it easier for developers to migrate to RSC — most components will "just work."
  • The "client" wording issue is still unsolved, but I think the community might just get used to it. We haven't been able to come up with a better name.
  • The only remaining concern is with knowing where you are in the module graph while editing a single component. But with poison pills and good compiler errors, the feedback to developers should be really fast when they introduce a hook they cannot use. Plus, if server components are largely async, that's yet another indicator about where you are in the tree.

Great work getting this pushed through!

The problem is that now i usually creating my components with .client.tsx or .server.tsx because I need a ez way of difference them. Maybe this not need to be a core feature of React, but the "use client" is a little annoying. Have to be a better way. But after all is a amazing work 20/10.

@zigang93
Copy link

zigang93 commented Oct 27, 2022

I am agreed that "use client" quite annoying..
what if I need the same components use in client and server side? I need write everything twice ?
prefer something like <SomeComponents client /> ..

this open out flexibility that I can decide use same components to render under client or server side without WET.
btw, this is just my own opinion.

@redbar0n
Copy link

@zigang93

what if I need the same components use in client and server side? I need write everything twice ?

No, you won't need to write everything twice. Components are treated as Shared Components by default. From the referenced documentation:

Server and Client Components no longer have to explicitly state so. Everything is implicitly Shared until proven otherwise, and then the build errors.

The implications of this is also that you shouldn't add the "use client" directive to all Client Components. Only the ones intended to be used directly by the Server.

@devongovett
Copy link

Could a compiler automatically do this without needing a "use client" directive at all? For example, it could statically analyze the module and if there are any hook calls, automatically mark it as a client component. Seems like this could provide some benefits, such as automatically moving things to the server if the code is refactored without manually adding/removing a directive.

@sebmarkbage
Copy link
Collaborator Author

The key is that when you move it, the props have to be serializable. This doesn't just mean that they use only the subset of types that are supported but also that those props aren't going to be an explosion in data transfer. There's benefit sometimes to move something to the client to make it a reusable template. Dev tooling will highlight this more.

It's also important that you teach patterns around what goes where for predictability.

I think some kind of "auto-fix" would be nicer to start.

That yea, it's totally possible. In fact, this idea spawned from work on Prepack to do exactly that. It just didn't work well for existing code that wasn't written with this code flow in mind.

nagrawal3 added a commit to nagrawal3/rainbowkit-use-siwe-auth that referenced this pull request Jan 26, 2023
- adding use-client allow us to add hooks on server side components
- adding use-client based on reactjs/rfcs#227
nagrawal3 added a commit to nagrawal3/rainbowkit-use-siwe-auth that referenced this pull request Jan 26, 2023
- adding use-client allow us to add hooks on server side components
- adding use-client based on reactjs/rfcs#227
- updated readme to include example on adding nextjs 13 with app folder
@HenriqueLimas
Copy link
Contributor

Hi team! Really wanted to thank you for the new proposal from file extension to the use ... directive, it is clear and easier to understand! Learned a lot by reading the discussion, I really appreciate that you make this public.

I think that the name React Server Components and React Client component are confusing though. As my understanding RSC in the future can also run as part of build time not only on server. And also that client actually runs on the Server when doing SSR. I think that developers might start using window and any browser API on the render function when they see use client causing confusion and errors.

My initial though (wrong one) of RSC was that they would render on the server and the hydrate/render would load it and put in the right place of the React application. But then after looking on the initial demo I noticed that it make use of a Protocol defined to serialize a component.

A different name could help preparing everyone for a different mental model of the current standard React components:

  • React Protocol Components -> 'use protocol'
  • React Standard Component-> 'use standard'
  • React Shared Component -> 'use shared'
  • "exports": { "react-protocol": "./..." }

The reason to use Protocol (inspired by protobuf) is to have something different than what we have today (SSR) to not cause confusion, so it force people having a different mental model.

Again, thank you for the huge work! RSC is really exciting technology!

@HenriqueLimas
Copy link
Contributor

But researching more, it seems that the community is more lean toward Server and Client components anyway where other frameworks also use Server/Client separation and introducing a new name might bring even more confusion to everyone.

@jamiter
Copy link

jamiter commented Mar 10, 2023

I like the idea of simply adding "use client" to a file, but currently bundlers seem to strip away the "use client" directive automatically, causing issues in monorepos like mentioned here: nrwl/nx#13194

If someone has an idea on how to work around this, please point me in the right direction.

Thanks for all the hard work put into this!

@gaearon
Copy link
Member

gaearon commented Mar 10, 2023

I messaged an Nx maintainer so they might be able to get a look at this.

@gaearon
Copy link
Member

gaearon commented Mar 10, 2023

But researching more, it seems that the community is more lean toward Server and Client components anyway where other frameworks also use Server/Client separation and introducing a new name might bring even more confusion to everyone.

Yeah. The names aren't perfect (sometimes "Data Components" vs "Interactive Components" seems to make more sense) but in practice I think they have stuck.

@juristr
Copy link

juristr commented Mar 18, 2023

Hey, @jamiter a fix is released in Nx v15.8.7. Thanks for reaching out 🙏

@thebuilder
Copy link

React libraries are starting to add use client to the bundled code they publish to NPM. This week react-query added it - This is nice if you are running a Next.js application with support for RSC, but it impacts the output from all other bundlers like Rollup/Vite.
Since RSC is not supported in Vite, logging that as a warning during build is not helpful. The user can't do anything to fix it.

packages/react-query/src/useMutation.ts (1:0)
1: 'use client'
   ^
2: import * as React from 'react'
3: import { useSyncExternalStore } from './useSyncExternalStore'
(!) Module level directives cause errors when bundled, 'use client' was ignored.
packages/react-query/src/useInfiniteQuery.ts (1:0)

I feel it would be a really good practice from your side, to ensure the ecosystem can handle use client properly by default, before it starts being rolled out further. You are going to see a continuous stream of questions/issues from users, getting the warnings from bundlers - and not knowing how to deal with it.

@mrazauskas
Copy link

Hope this is the right place to leave feedback.

In Jest repo few users reported an issue related with the 'server-only' package jestjs/jest#13967

Seems like users must pass 'react-server' to customExportConditions of test environment to be able to run tests in Jest.

How do you think, would it make sense to add "node": "./empty.js" export to the 'server-only' package? That would allow testing server only functions in Jest’s default Node environment.

If an extra export condition is not a good idea, I think it would be useful to document that 'react-server' export condition should be passed to Jest. Not sure whether that should do mentioned in React’s or Jest’s docs.

@sebmarkbage
Copy link
Collaborator Author

@mrazauskas It wouldn't be correct to add it to "node": "./empty.js" as that would by-pass the main purpose of that module. The goal is for it to throw by default to indicate that it doesn't work by default in existing environments that might be SSR hybrid.

Basically 'react-server' is an opt-in that it's ok to run server-only code in this environment (and that it's not ok to run client code)`.

Configuring jest seems the correct solution but it's tricky in jest to test multiple environments at once. E.g. even if you do that, how do you ensure that you can test client components in the same test?

@mrazauskas
Copy link

Ah right.. I didn't take into account SSR.

By the way, one user came to an idea (jestjs/jest#13967 (comment)) to use manual mock:

// __mocks__/server-only.js
module.exports = {};

In this case all checks of server-only get erased. This could be another solution.

@sebmarkbage
Copy link
Collaborator Author

The issue is that server-only is only one of possibly many different libraries that can fork. It's likely only to cover up other issues. E.g. "react" itself has a forked version of itself.

@tomconroy
Copy link

Hello, I hope this the right place for this question. I have a React component library that uses Relay for data fetching for the components. I am trying to accommodate RSCs by preloading data based on this pattern.

Is it sensible for libraries other than server-only and client-only to use these conditional exports, e.g. to export different components for server context vs client context? In the server context, we would fetch and serialize the relay data before rendering the client component. Then the client component can use the preloaded serialized data, or lazy load if omitted.

@aralroca
Copy link

Is there a way to specify in the same file which components are "client" and which are "server"? To use my libraries I use microbundle and it puts everything in the same file, I like microbundle but as a library maintainer should I change the bundler to create it in different files or is there a better way to do it? 🤔 Thanks

Note: This condition can only be used for conditional `imports` within the project itself.

Possible addition: A possible addition to this can be to also support the `"server-only"` condition as a less React specific alternative that could be adopted by other frameworks that also choose this strategy.

This comment was marked as spam.

Jaxensdad2012

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.