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 #189

Closed
wants to merge 1 commit into from

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Dec 21, 2020

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

React Server Components doesn't have much of an API other than integration into meta-frameworks. Instead, we propose that the main way you make choices about what runs on the server and client is defined by its file extension.

View the rendered text

@sebmarkbage sebmarkbage changed the title React Module Conventions RFC React Module Conventions Dec 21, 2020
@sebmarkbage sebmarkbage changed the title React Module Conventions RFC: React Module Conventions Dec 21, 2020
@sebmarkbage sebmarkbage changed the title RFC: React Module Conventions RFC: React Server Module Conventions Dec 21, 2020

The approach we're taking will need strong bundler integration to perform well. Therefore, we'll have specific bundler plugins provided by the runtime that they support. However, ideally you shouldn't need anything else. For example, existing TypeScript and Lint rules can still apply because we just expands on existing conventions.

There is some infrastructure that likely needs deeper support - such as CSS-in-JS libraries need to be designed to be able to ship CSS without the JS.

Choose a reason for hiding this comment

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

Hi! I'm the Emotion maintainer and I would be very keen to discuss how this can play out for CSS-in-JS.

It seems that CSS "references" with custom handlers would solve this use case partially. From what I understand this would require:

  • providing a handler to React (which somewhat breaks Emotion's 0 config premise but that's just what it is)
  • idempotently-generated class names - this is already covered by most CSS-in-JS, clients can rehydrate SSRed content
  • a way to deal with hydration. We need to provide styles to the application before React can even be used so before it kicks in it won't be able to use a custom CSS references handler. This is usually done by rendering <style> elements - we do that "inline" to avoid double extra work on the server and so the response can be streamed. This naturally leads to a server-client mismatch but we use a hack to move all our <style> elements to <head> (before the React client can see them). This probably doesn't play ideally with streaming though (our initial JS could be executed before the whole SSR content got streamed to the client so <style> elements that arrive later won't be handled correctly).

Drawbacks:

  • Server Components are stateless, we can't share a cache of already inserted IDs easily so each server request would return many CSS rules to the client that would just be skipped over by it

I understand that solving some of those might just be out of the scope of the whole design - it would still be great to establish what we can try to tackle and what is simply a no-go.

cc @kof

@mAAdhaTTah
Copy link
Contributor

If all components right now are "client components", and those generally use .js(x) extensions, would it be an easier migration path for .js to be client, and do .shared.js or .server.js for the new types being introduced by SSC? Even if most components will eventually be "shared", client components, relative to current React, don't have any restrictions, whereas shared & server components do. It might be an easier for devs to opt-in to those restrictions with an extension.


In theory we could use this instead of "importing" a `.client.js` file. However, since this functionality isn't widely available in tooling yet it's not practical. Additionally, we would still need different APIs to describe preloading and unwrapping. Since those are common operations, we'd need something more convenient than plain asset references anyway.

## TypeScript Support

Choose a reason for hiding this comment

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

Another related thing, maybe, to mention, is how typescript auto-imports could work.

For example, it'd be nice if IDEs didn't suggest importing a server.ts file when the developer has a client.ts file open (and vice-versa)


It's not best practice to fork client components based on if they use SSR or not because the hydration should use the same path. If there is a fork in behavior, then it should be forked on the client as well during hydration. That needs a separate dynamic flag that's different than this.

It might seem confusing when you look at it hard enough, but we've found that when you're working on a product you mostly spend your time thinking of .server or .client disinction and these provide the right intuition.

Choose a reason for hiding this comment

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

`

Suggested change
It might seem confusing when you look at it hard enough, but we've found that when you're working on a product you mostly spend your time thinking of .server or .client disinction and these provide the right intuition.
It might seem confusing when you look at it hard enough, but we've found that when you're working on a product you mostly spend your time thinking of .server or .client distinction and these provide the right intuition.


However, in all configurations the bundler needs to be able to know about which `.client.js` files to include in the bundle. The Server Environment and Client Environment is bundled separately, by default the client bundle wouldn't include any of these files since they're not reachable from any of the client components included in the shell of the app. We must therefore tell the bundler to include them.

We expect production builds to build the server first and then use this tree information as input to build the client. However, during development it is useful to be able to just rebuild the client separately. In some scalable configurations, it's also helpful to not use the reachability of the graph but instead every build every file on the file system in parallel. By using a file extension we can build the client bundle but simply just telling it to build all `.client.js` files it can find.

Choose a reason for hiding this comment

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

Suggested change
We expect production builds to build the server first and then use this tree information as input to build the client. However, during development it is useful to be able to just rebuild the client separately. In some scalable configurations, it's also helpful to not use the reachability of the graph but instead every build every file on the file system in parallel. By using a file extension we can build the client bundle but simply just telling it to build all `.client.js` files it can find.
We expect production builds to build the server first and then use this tree information as input to build the client. However, during development it is useful to be able to just rebuild the client separately. In some scalable configurations, it's also helpful to not use the reachability of the graph but instead build every file on the file system in parallel. By using a file extension we can build the client bundle but simply just telling it to build all `.client.js` files it can find.


Adding a file extension to files and to most imports adds noise to your code. If our thesis of a lot of components being expressible as Shared components is incorrect, we might end up with a lot of `.client.js` file extensions in the ecosystem.

# Alternatives

Choose a reason for hiding this comment

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

I'm curious if there was ever consideration around something like:

// A server component. Through the use of linting, we could prevent hooks being called. 
// When building in "client" mode, this should be DCE'd. 
// DCE may be expensive in development. Instead, it'd be a function that throws if called. 
const FooComponent = React.server(() => { 

})

// A client component. 
const FooClient = React.client(() => {

})

I'm not really sure if something like this would be feasible however.

The benefits I see of this is that it allows developers to change from server->shared-> client, without having to rename files.

Copy link

Choose a reason for hiding this comment

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

I'm guessing this strategy would make it difficult for a bundler to optimize a client or server module graph, because the distinction happens in the JS runtime rather that as a file name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could also just error if this module recursively imports a named export that isn't available for the environment you're compiling for.

@facebook-github-bot
Copy link
Collaborator

Hi @sebmarkbage!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

Copy link

@jplhomer jplhomer left a comment

Choose a reason for hiding this comment

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

Hi! Providing some feedback on the these conventions based on what we've discovered the past year building Hydrogen, a meta-framework that uses React Server Components.

Client components are a confusing concept to most developers. We propose eliminating the concept of client components altogether.

This RFC describes client modules as "modules that can only be imported on the client." This definition makes sense if you consider both the browser and SSR-emulator as a "client."

However, we've found that most developers think of client modules as "modules that render on the client." While this is a slightly different definition, we've found developers are taken by surprise when their client modules render during SSR — something that is always going to happen on some sort of server.

We've also found that most third-party React libraries and components in the ecosystem are going to be client components by default.

Take react-bootstrap for example: even a simple badge component, which you wouldn't anticipate having any client-only functionality, references useContext in order to apply a global theme set by a provider. This means that, in order to use a library like react-bootstrap in a server components application, you would need to re-export every single component from a *.client.js wrapper — or the maintainers of react-bootstrap would need to rewrite their library to implement server context.

Finally, we find that developers are rarely using shared components. This is likely due to the restrictions imposed by needing to run in both a react-server and client context. Their use will likely increase when server context is introduced.

We propose what @mAAdhaTTah has proposed above: all *.js modules are client components by default, and developers should opt-in to server and shared modules.

With this new proposal, a developer would migrate to RSC by:

  • Creating a root .server.js entrypoint
  • Rendering their app tree as normal
  • Gradually converting components to *.shared.js and *.server.js as explicit opt-in behaviors

Benefits to this modified approach include:

  • Reduced mental overhead of learning RSC: you can now opt-in to special behavior by setting a .server.js or .shared.js suffix. It's called "React server components" after all, and this places the emphasis back on those components
  • Most existing apps will "just work" right away.

Downsides to this modified approach:

  • Optimizes for the status quo instead of the future: We acknowledge that this doesn't represent a future where all modern apps and libraries in the ecosystem have adopted server components. However, we see this as a major unblocker for adoption of RSC today.
  • Makes client module discovery more cumbersome: The naive v1 way of finding client components during development will become very bloated, because every single JS module in your source code and potentially node_modules needs to be bundled and referenced in a manifest. One way to solve this would be to modify the discovery process by starting with server components, and using data from that module graph to inform which client modules need to be bundled. It sounds like this is the plan already.

The use of conditional exports to fork behavior is an unnecessary optimization which creates friction and confusion. We should instead rely on the React runtime(s) to perform the fork.

As described in current proposal, the react-server condition is used to fork implementations based on environment (react-server or normal). The intention is to be able to export "react-server-only" exports from a package, or provide different functionality based on runtime environment, like adding more client-only features to a data framework.

We've found it really difficult to build a meta-framework using this constraint. While Hydrogen is built with Vite, we anticipate other meta-frameworks using other bundlers will face difficulties as well. Specifically, "unbundlers" like Vite are meant to run in a single process. They don't emit to the local filesystem during development and instead reference raw modules in the filesystem, transforming them on the fly.

This makes orchestrating development and build processes more complicated because you need to build different bundles under different conditions. Relying on multiple node processes during development is not a new concept, but it also a has non-zero impact on developer experience from a tooling perspective.

As mentioned in this proposal, it also makes it difficult to perform SSR, because you have two separate server module graphs.

This places additional burden on third-party libraries in the React ecosystem. Their build processes already need to take into account browser vs node vs ESM vs CJS, and adding a react-server vs "normal" into the matrix makes things more complicated.

As an alternative, we propose that the React runtimes (fizz, flight, client) become responsible for making the fork based on their own runtime conditions.

We can do this by modifying the proposed client module reference and introducing a $$value (or similarly-named) property whose value is the original module export:

export const myString = {
  $$typeof: MODULE_REFERENCE, 
  filepath, 
  name: 'myString', 
  $$value: _myString // original value
}

This dramatically simplifies bundler integration and support for RSC:

  • Flight runtime continues to operate as normal
  • Fizz and Client runtimes learn how to unwrap a client module reference by using the $$value property
  • SSR and RSC share the same module graph and can run in a single process

We'd be super interested in the above proposals, as I've assumed you have considered them already before arriving at what exists today. Thank you!


We propose that whether you're using Node.js runtime or building a server bundle, that those module resolutions add the `"react-server"` condition.

This allows a package to fork its implementation based on whether it's used on the server or the client. For example, a data framework might support simple data fetching on the server but on the client it can support optimistic updates or fine grained invalidation in a client environment. Even React itself uses this technique to exclude stateful Hook exports on the server.
Copy link

Choose a reason for hiding this comment

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

I'd love to learn more about this, as it seems to be the primary motivator for being able to fork based on react-server environment. For example, if I'm writing a data framework: why wouldn't I just include all the fancy features intended for the client on the server as well, especially if we're not concerned about bundle size (like we would be on the client)?

The only thing I can think of is maybe needing to use certain hooks (state, effect) to perform optimistic updates, and not being able to use them in a react-server environment. Is that accurate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bundle size is a huge issue on the server with edge workers too. They're also part of initialization cost to start those workers. If you run initialization code that isn't compatible with that environment, it might not even work.

I answered the rest below. State, effects, etc. is one consideration.


It might seem confusing when you look at it hard enough, but we've found that when you're working on a product you mostly spend your time thinking of .server or .client disinction and these provide the right intuition.

A possible downside of this approach is that you can't run SSR and a React Server environment in the same module graph. You can however build two separate bundles and run them in the same Node.js VM. However, we expect most people to want to use two separate Node.js instances for parallelization purposes anyway and possibly even putting the SSR instance close to the "edge" and the React Server instance close to the data.
Copy link

Choose a reason for hiding this comment

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

However, we expect most people to want to use two separate Node.js instances for parallelization

In production?

The split between React Server (close to DB) and SSR (close to Edge) is fascinating. I hadn't thought about that use case. Do you see this practice being adopted by most developers, or is this more of a large-scale app optimization? The orchestration of both of those bundles sounds tricky.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea I also see React Server with SSG and SSR. With systems like Vercel, my current employer 😅, that orchestration doesn't have to apply to only large-scale apps but everyone could. Same thing with Shopify.

However, using different runtimes for backwards compatibility is another angle to this.


Adding a file extension to files and to most imports adds noise to your code. If our thesis of a lot of components being expressible as Shared components is incorrect, we might end up with a lot of `.client.js` file extensions in the ecosystem.

# Alternatives
Copy link

Choose a reason for hiding this comment

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

I'm guessing this strategy would make it difficult for a bundler to optimize a client or server module graph, because the distinction happens in the JS runtime rather that as a file name.

@facebook-github-bot
Copy link
Collaborator

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@sebmarkbage
Copy link
Collaborator Author

all *.js modules are client components by default

Before going further, the main thing to consider here is that without any further changes this means that you can't import anything in the ecosystem today. Including helper utility modules or anything like node-fetch.

I believe that only works if you also assume that only "components" are special as they're the only thing that can be passed to the client and those are the ones that have module semantics. Alternatively with a Proxy where only some are rewritten.

modifying the proposed client module reference

This part doesn't seem to work together with assuming that all .js files are client component are default since simple helpers would be module references. It does work in the variant that you actually use with Proxies and assuming only components can be passed to the client, but that's not what you're proposing here.

So something is not consistent and I'm curious how you intend to solve that.

In your approach you wish to run both Server and Client components in the same runtime environment, which is reasonable. After all in the simple case you can at the very least end up loading duplicate modules otherwise. However, combined with the all *.js files are client components by default and are conditional at runtime means that you import those modules even if they're not used. Any check for runtime specific things inside of those modules, would be an error in an environment they're not used. And slower. I believe to make the all *.js files are client components by default work together with this, you'd have to not just make this an optional approach but you'd have to make it a requirement that all code can execute in either environment. E.g. it wouldn't work in a set up where your Server Components can run in a Node environment with Node dependencies and SSR in a Cloudflare Worker with only those dependencies. Because I believe you'd pull in the Node dependencies into the Cloudflare Worker too.

So I think roughly works in your current setup, but not with another setup.

@sebmarkbage
Copy link
Collaborator Author

I'm not super sure about the react-server condition being a requirement. There are a number of cases where this has popped up where you'd want different implementation details. For example, a useCookies implementation might use state etc. on the client and more complex subscriptions to the state of some cookies. Where as on the server it might just read some global. Even the implementation details of useServerContextsForRefetch could have forked implementation details.

So the use case is when you work a complex implementation that's prepared to handle updates and state, vs a lightweight implementation that only handles initial render. Since useState can't be conditional and useState errors on the server, there's no other way to implement this as one hook in both environments atm.

For some of the data fetching strategy, it would probably be better to add other conditions. For example currently a Service Worker, Web Worker and Cloudflare Worker are typically all built with the "worker" condition. So you can't fork between a proxy and direct DB access for example. However, for that strategy it might be sufficient for the ecosystem to adopt more granular conditions. I do the use of conditions is a requirement going forward regardless.

Unbundlers and things like ESM CDNs and Deno needs to properly support that all the way down without having to update intermediate URLs, for things like dev/prod mode. I think that's doable but a lot of those strategies are trailing and we can't really reasonably support an environment that don't allow such deep condition flipping. Vite does support it though afaik, so it's not an issue there per se. It's more about an issue whether you can also run them in the same instance runtime.

The way that ESM CDNs seem work with something like ?dev is that they recursively pass that down so the entry point decides what condition to load the rest of that tree in. In that world it would be trivial to run them in the same environment, they'd just be duplicate modules. So if anything, once they catch up to arbitrary conditions it seems trivial to adopt there.

I suspect we'll see actual bundlers add that capability too.

That said, this part is not set in stone. It's more important to iron out the .client.js or .js runtime part because that's still not clear to me how that would work out.

@sebmarkbage
Copy link
Collaborator Author

Another consideration that came up is that there is risk to implementing server components and SSR in the same environment since you can easily leak between them. E.g. if you assume the .server.js environment is safe and won't leak secrets (like API tokens or whatever) then you might put them on a shared module scope that then gets picked up by a SSR render. Accidentally through something convoluted.

This is analogous to running multiple SSR requests from different users in the same Node instance for example. Typically that's avoided for this reason that it's too accidentally to leak data.

It gets worse if you want to cache the Server Components part separately from the SSR part (since it's independently cacheable) because now data from the per-request SSR render might leak into the Server Components that then gets leaked to other users through the Cache.

So one benefit of running them independently is that stronger isolation.

Now, depending on your infra set up this might not matter so it doesn't negate the need for a lighter runtime in those cases. However, it might mean that it is less versatile in more infrastructures than might seem from the simple straight up RSC + SSR case.

@jplhomer
Copy link

jplhomer commented Mar 3, 2022

without any further changes this means that you can't import anything in the ecosystem today

Yeah, I totally spaced on that requirement. In that case, Proxies would be the way to go — but that seems like it would come with its own set of challenges.

I still think the lack of a client suffix would help us in a number of ways, but I understand the reasoning behind keeping it 👍 You're much more familiar with this space, so I'd defer to your judgement.

Regarding shared SSR & RSC bundles: one thing that has become clear to me based on your feedback is that the split between bundles is intentional rather than an unfortunate side effect, as I had initially interpreted it:

  • Smaller individual bundle size
  • Prevents leaking secrets from Server > SSR
  • Ability to run server component and SSR in two separate processes, if not two separate environments altogether

None of these things had occurred to me as benefits before, but that's good to know, and helps Shopify inform the direction we should be heading.

I'm curious how running two separate server bundles in two different environments (e.g. CFW for SSR, Node.js for RSC) has on developer experience. Today, most SSR React developers are familiar with the concept of writing isomorphic JavaScript that is able to run in a server and in the client.

With the introduction of v8 runtimes like CFW, developers have been required to adapt a slightly new mindset and be aware of their production target. For example, if I know I'm going to be deploying to a v8 runtime in production, I'm not going to rely on fs in my user code. I know this mindset shift has been difficult for a lot of developers, and we've been trying to figure out how to teach Hydrogen developers that "Hey, you might be deploying to a Workers target if you're going to use Oxygen... here's what that means."

My question: do you see this proposal affecting this developer experience at all? Will developers care less about their intended deployment target, and instead rely on the meta-framework or the deployment platform (like Vercel) to run their code isomorphically across environments?

Or will developer need to care even more about this split behavior?

@sebmarkbage
Copy link
Collaborator Author

There are benefits to the split.

There is a downside of more difficult infrastructure to build but I’m actually not too concerned about that. Once some techniques are out there they’ll be duplicated. The rest of the system is hard to build anyway but people will figure it out or use off-the-shelf solutions like now. Also build tooling will want more control and therefore adopt more streamlined implementation that allows to that. We’re just in an equilibrium around old tech right now.

The thing that keeps me up at night is that there is also a perf downside of initializing duplicate shared modules. Module initialization is the thing about JS that scares me the most. That’s why I’m still thinking about other ways to solve this.

I also agree that it would be nice if .js could be reused more easily.

As for environments, I see people thinking less about Node vs Web in this model as they’re getting closer to each other and alternatives are more Web-like.

That will instead thinking about where the data lives and how much data to pass to the client. So you end up thinking more about server vs client components.

@sebmarkbage
Copy link
Collaborator Author

One interesting case is that in either environment code like this sometimes work and sometimes doesn't:

// shared.js
import Foo from "foo.client.js";
const Bar = Foo.Bar;
...

I think this goes in the scenario where shared code should ideally have some restrictions - like it also shouldn't useState. This could be a lint but it could also be part of compiling the client bundle to error early.

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Mar 15, 2022

I think there is a possible way forward to a solution that can work somewhat equivalently both in the dual environment and in the single environment.

  1. Drop the react-server condition. This would mean that you can't fork to custom stateless implementations of the same Hook that works in different ways whether it's on the client or the server but maybe that's ok.

  2. In the world where you compile proxies instead of module references, the Proxies should cover all named exports. They work like their underlying object/function when used outside of a Server Components render function. When accessed inside a Server Component function while rendering you can't access their properties and if called (if it proxies a function) it errors. When serialized, it gets replaced by a module reference.

  3. If the thing being Proxied is a string it can't be wrapped in a Proxy. Instead, the raw string is returned. If it's a short string, it ends there. If it's a long string, it can be added to a global Map. If Server Components runtime sees one of the strings in that map, it instead serializes a module reference. So that you can send large strings that are defined elsewhere instead of in each response such as for i18n.

Note that in this proposal we still assume .client.js files and not .js. I don't think that part of the proposal quite works.

@jplhomer
Copy link

Yeah this sounds like a valid option.

Note that in this proposal we still assume .client.js files and not .js. I don't think that part of the proposal quite works.

Given the Proxy approach, wouldn't dropping .client.js work out of the box? Proxy values work like their underlying object or function when used outside of a render function (e.g. node-fetch), and we could support strings as you describe as well. Or am I missing something here?

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Mar 16, 2022

Given the Proxy approach, wouldn't dropping .client.js work out of the box?

Yes but it only works with the Proxy approach. The goal is to get as close as possible to interoperable code. So I'd say it shouldn't work with the Proxy approach so that we preserve interoperability. In other words, the Proxy should only be applied to .client.js files.

@jplhomer
Copy link

The goal is to get as close as possible to interoperable code.

Can you clarify what interoperable means in this sense? Do you mean normal JS modules working in both server and client components?

@sebmarkbage
Copy link
Collaborator Author

Like some environments like Shopify might use the Proxy approach and some environments like Vercel might use the multiple worker approach.

If we have two different rules then the React ecosystem would fork.

@sebmarkbage
Copy link
Collaborator Author

Relevant this this thread:

microsoft/TypeScript#48189

That system uses file extensions to fork an implementation instead of package.json export conditions.

@jplhomer
Copy link

Circling back:

We see dropping the client.js suffix as critical to adoption. The most common feedback we've received from developers building on RSC in Hydrogen is that they are confused by client components rendering during SSR, and they feel that (re)naming most of their components with the client suffix is tedious. We're working on moving Hydrogen to suffix-less client components, and we'd like build consensus around this method.

A couple paths forward here:

Option 1: Leverage the jsx extension

If we want to drop the client suffix, we need a way to differentiate JS modules which should be wrapped in module references from JS modules which should be executed as normal.

If we treat modules that end in .jsx as module references in server components, we can effectively drop client.jsx as a required suffix. We can also support shared components by introducing a new shared.jsx suffix. We see shared components being used significantly less anyway — mainly for the purpose of server context.

Here is a working prototype in Hydrogen: Shopify/hydrogen#1090

The client.js suffix can still optionally be used for runtimes which drop the jsx suffix (like Next.js) or for third-party NPM modules. One option to simplify adoption of 3P modules is to add a code-scanning heuristic, like the presence of certain imports. Another option is to introduce a pseudo import decorator as an explicit asset reference or an explicit regular module:

import SharedComponent from '3p/lib?as:shared';

Option 2: Proxies everywhere, and smarter React runtime

This option is similar to our initial approach at the Vite RSC plugin.

All JS components which don't end in a server or shared suffix will be modified to wrap their exports in Proxy.

This is outlined here: https://codesandbox.io/s/server-components-no-client-vionz6?file=/src/App.js

In this version, all JS code is executed as normal until it is passed to React.createElement. React's runtime checks for the presence of a module reference property and creates the module reference at that point instead of the bundler.

More burden is placed on the bundler in the form of AST parsing and determining which modules actually need to be bundled as references on the client. But we see this work happening in some form regardless in order to make an optimized client bundle.


What are your thoughts about these options? Have you explored any additional approaches that don't require a client suffix?

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Apr 18, 2022

One thing that I'm not sure is in your implementation but one of the concerns I have is that it's actually also really important to use .server.js extensions appropriately. E.g. we think that the data fetching strategy might be limited to files ending in .server.js. I'm not sure you're fully enforcing this as otherwise you'd likely have complaints there too.

As we add more server-only features, we'd need to be able to enforce this statically and perhaps syntactically.

@sebmarkbage
Copy link
Collaborator Author

I'm seeing more and more that the react-server ability to fork implementations for client(able) and server usage becoming more important. So we might want to still keep that. That requires a module graph implementation that's capable of forking them.

I'm also seeing that it's more and more important to be able to run small slices of server components in independent workers.

So to me it seems like the Proxy approach is DOA. It's not going to work as a required implementation detail.

@sebmarkbage
Copy link
Collaborator Author

Unnecessary client opt-ins isn't bad because technically you can import any reference from them. I think it's more about warning if you add a prop that won't be supported on the server. However, technically any prop can be passed. It's more about whether a server component actually tries to do that. We're missing a TypeScript feature to do that check (which Flow has). If you have full coverage of all your consumers, then you can just rely on that.

The warning is useful if you publish packages to npm or internal repositories that don't immediately have coverage. You don't accidentally want to add a prop that consumers can't use.

@sebmarkbage
Copy link
Collaborator Author

Would it be too presumptuous to take the "server-only" exports condition instead of making it a React thing? Maybe it can support both.

@huozhi huozhi mentioned this pull request Sep 17, 2022
6 tasks
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Sep 18, 2022
## Feature
Change server components convention from using `.server.js` / `.client.js` file extension to determine it's a server or client component to using `'client'` js literal as a directive for determine client components boundary.
React RFC: reactjs/rfcs#189
New behavior doesn't consume `.server.js` as server components any more, if you're enabling `serverComponents` flag, every `page.js` in app dir will become server components by default. If you adding a `'client'` directive to the page, then that page will become a client component. This rule also applies to the normal js components, client components will require a `'client'` directive to indicate its identity, instead of having a `.client.js` extension.
- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

Co-authored-by: Shu Ding <[email protected]>
@wardpeet
Copy link

wardpeet commented Sep 19, 2022

We at Gatsby have launched our first alpha surrounding server components using the "client export" directive, and it is 10x times better than .client.js & .server.js

"client export" isn't the best directive either. For some reason, my brain always wants to type "use client" or "use client export" instead. It also doesn't feel native to me as a Javascript developer. I haven't written a directive like "use strict" for ages. Moving to an import {} from 'react/client" feels a bit more natural, or if we want to live on the bleeding edge, perhaps decorators are a good fit for this.

Decorators also help with the library bundling issue, where utilities and React components are bundled into a single file.

"client export";

export function MyCustomComponent() {
  return <div>{myUtility()}</div>
}
export function myUtility() {
  return 'hello world';
}

A naïve module reference transformer will transform all exports into module reference structure leading to broken code. However, when using decorators, it can be scoped per function and tell the compiler only to transform these functions.


Rfc for gatsby: gatsbyjs/gatsby#36608

@sebmarkbage
Copy link
Collaborator Author

Btw, the latest is "client". I plan on writing it up in a new RFC to gather the latest in the same place to summarize this thread.

@sebmarkbage
Copy link
Collaborator Author

@wardpeet are you implementing the "react-server" export condition? I plan on adding an error when the wrong assets are used in a Server environment - such as the full "react". This enforces a compatible environment that uses the export condition.

@sebmarkbage
Copy link
Collaborator Author

Note that in your example, myUtility is valid to export as a reference to be passed in as a prop back into the client.

import {myUtility} from "MyCustomComponent";

<ClientComponent util={myUtility} />

Decorators also don't work on every type of export like exporting a static large object to be repassed.

It's a very intentional design decision that modules are never split into two environments. We started out that way but that leads to a lot of fragile assumptions about where the split should be and you have to encode a lot of semantics about what might be side-effects and what happens with overlap. Especially I don't think we'll find a way to get the same semantics of that.

It's unclear what "react-server" condition should mean in that world. Presumably it would apply before the file and somehow for any import referenced by the client side that's still remaining after DCE.

It's too flaky semantically.

@sebmarkbage
Copy link
Collaborator Author

"use client" would be ok with me. It's more googleable too. Might be confusing in language to confusing with Hooks.

@brillout
Copy link

It's a very intentional design decision that modules are never split into two environments. We started out that way but that leads to a lot of fragile assumptions about where the split should be and you have to encode a lot of semantics about what might be side-effects and what happens with overlap. Especially I don't think we'll find a way to get the same semantics of that.

Next.js does it though (e.g. getServerSideProps()), and I've yet to hear someone complain about it. So I'm thinking that, even though many edge cases cannot be analyzed at compile time, it's stable enough. Maybe forgoing these edge cases is worth it, given the much improved DX.

@karlhorky
Copy link

karlhorky commented Sep 19, 2022

I've complained about the mysterious / unconfigurable ways the compiler split code in Next.js :) It was very unusual to me. vercel/next.js#36514 (comment)

@sebmarkbage
Copy link
Collaborator Author

Yea, we hear those complaints. The original complaints actually came from Next.js folks. It also ended up being way worse for DX in early experiments because it was so confusing.

Note that even though we could there's also an additional constraint here that every compiler implementation would have to implement it the same way or code published to npm would break in unexpected and potentially security compromised ways. Need something easier to specify.

@sebmarkbage
Copy link
Collaborator Author

Would it be too presumptuous to take the "server-only" exports condition instead of making it a React thing? Maybe it can support both.

For code that currently runs on the server but in SSR, there could be a "server" convention. People use "node" for that today.

My rationale for "server-only" is that there is a growing ecosystem for server-only code in other forms - typically the Client Island architectures (e.g. Astro, Fresh, etc). I'm guessing those currently work more similar to Hydrogen where they're all really just bundled into a single SSR graph. However, I could see a similar need. Although I'm guessing those libraries aren't currently interested in this because if they're all a single bundle. I could foresee those libraries adopting such a strategy in the future though or new libraries being inspired by the RSC system.

If I build framework agnostic code that I can fork for the server-only environment, then it would be nice to have a generic term for that.

"server-only" would indicate that this is not just for the case where this is running on the server like SSR but it's intended to kick in only for graphs that will never run on the client.

@frehner
Copy link

frehner commented Sep 19, 2022

In an ideal world the names “client” and “server” are changed as that still remains the biggest point of confusion for devs. :)

@sebmarkbage
Copy link
Collaborator Author

@frehner Any suggestions? The brand name "React Server Components" is pretty strong already now.

The thing that gives me a bit of hope is that currently the initial HTML render part of SSR is such a huge part of the ecosystem and how people have learned to do React in the past decade. However, it wasn't always like that and it can change again.

Currently, Server Components adoption is so minuscule so nobody comes in with the notion that Server is mostly Server-only. In the future that might not be the case though. So we shouldn't over-optimize for the current mindset. Intuitions can be changed. However, there's also a risk that this doesn't play out that way.

@frehner
Copy link

frehner commented Sep 19, 2022

Any suggestions?

I believe @jplhomer had a list of suggestions at some point, were they in this thread already?

The brand name "React Server Components" is pretty strong already now.

I don't think the name necessarily has to change - as you mentioned, it's got a lot of brand recognition behind it. And it still does a good job of describing what is happening.

@brillout
Copy link

I've complained about the mysterious / unconfigurable ways the compiler split code in Next.js :) It was very unusual to me. vercel/next.js#36514 (comment)

Good point, I see the confusion.

In an ideal world the names “client” and “server” are changed as that still remains the biggest point of confusion for devs. :)

Over here at vite-plugin-ssr we are going to replace the term client with browser, which I think is 10x clearer. But it's a misnomer when using React Native, so I guess that's a blocker for React?

As for server, the only alternative we came up with was node (for Node.js) but that isn't ideal because with the advent of Deno and Bun, this will become a misnomer. But I'm thinking that node is still better than server. I mean, server is clearly the wrong term here (because of SSGs).

The brand name "React Server Components" is pretty strong already now.

For a long time I was thinking that RSC needs SSR. But turns out that I'm wrong and that RSC is also great for SSGs. I think the name "React Server Components" played a role in why I was confused.

I think it can be very well worth it to change the branding while it's still possible.

It may sound crazy at first, but how about we coin a new term? Because I don't think there is an existing term that matches the notion of "it runs on the server or at build-time". Idea: noder (like server and as a reference to Node.js being the first popular non-browser JS runtime). Other idea: nclient for non-client. Or nbrowser. Or simply, albeit verbose, non-client/non-browser. None of them are perfect, but I think they are still better than server.

@wardpeet
Copy link

@wardpeet are you implementing the "react-server" export condition? I plan on adding an error when the wrong assets are used in a Server environment - such as the full "react". This enforces a compatible environment that uses the export condition.

We're not, what benefits does it give?

"use client" would be ok with me. It's more googleable too. Might be confusing in language to confusing with Hooks.

I would go with "use client", instead of the plain "client". I'm still not convinced that it makes sense for developers.

Note that in your example, myUtility is valid to export as a reference to be passed in as a prop back into the client.

import {myUtility} from "MyCustomComponent";

<ClientComponent util={myUtility} />

Decorators also don't work on every type of export like exporting a static large object to be repassed.

It's a very intentional design decision that modules are never split into two environments. We started out that way but that leads to a lot of fragile assumptions about where the split should be and you have to encode a lot of semantics about what might be side-effects and what happens with overlap. Especially I don't think we'll find a way to get the same semantics of that.

It's unclear what "react-server" condition should mean in that world. Presumably it would apply before the file and somehow for any import referenced by the client side that's still remaining after DCE.

It's too flaky semantically.

What do you mean exactly with the above statement? Because my example is with rollup gives me after bundling, even if they are separate files. If I'm a library author, should I pre-compile my component for server-components? As in use the react-server condition?

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Sep 19, 2022

We're not, what benefits does it give?

You can see the discussion above but it allows users to publish third-party packages that have a separate implementation when it's imported from a Server Component from when it's used on the Client. E.g. you can have useEffect etc only when it's not on the server.

It also implements the poisoning where you can enforce that certain modules are only imported by one or the other.

The reason I bring it up is that it would be required to be compatible with third-party code so everyone has to implement it. React would also start enforcing it on itself so you couldn't use the "react" package from "Flight" without implementing it in the compiler.

You wouldn't use it to precompile components (although technically you can use it to precompile client references, that's not the intention).

@sebmarkbage
Copy link
Collaborator Author

The things that make this hard to implement (e.g. you need split module graphs) are also things needed for an optimizing compiler to be able to treat SSR, Client and RSC separately.

@sebmarkbage
Copy link
Collaborator Author

What do you mean exactly with the above statement? Because my example is with rollup gives me after bundling, even if they are separate files. If I'm a library author, should I pre-compile my component for server-components? As in use the react-server condition?

I see. I interpreted it as a library that the author writes in their own code - like a utility.

It is a potential problem, especially with rollup, that you end up with directives sprinkled inside the code. Rollup thankfully tends to error on most directives for that reason though. Other compilers might also hoist their library helpers above the directive.

If you publish third party code that's bundled, then that bundling needs to be split into separate bundles just like the server/client does. So you publish a bundle with a single directive at the top that has all the client code and another bundle for the server that doesn't. The server one can import the client one.

Especially something like rollup that puts everything in the same scope wouldn't just work anyway since all the side-effects would be treated as a single scope too and it'd be hard to disentangle them.

@gaearon
Copy link
Member

gaearon commented Sep 19, 2022

Over here at vite-plugin-ssr we are going to replace the term client with browser, which I think is 10x clearer. But it's a misnomer when using React Native, so I guess that's a blocker for React?

I don’t think browser solves one of the primary points of confusion, which is that “client” (and thus, “browser”) components still run on the server during SSR phase.

@brillout
Copy link

brillout commented Oct 5, 2022

How about this:

  • "Normal components": same as today, i.e. components that usually run in the client but can also run on the server upon SSR.
  • "Server components": components that only run on the server
  • "Shared components": components that can be imported by "normal components" as well as "server components".

So basically replacing the term "client" with "normal".

As for syntax:

  • Normal components => no magic comment.
  • Server components => magic comment // @server
  • Shared components => magic comment // @shared

@sebmarkbage
Copy link
Collaborator Author

The thing we're talking about here is overrides. I believe that for things that are truly universal, that's just "default". So lack of specific overrides is the same as isomorphic / universal.

@kylemh
Copy link

kylemh commented Oct 11, 2022

Using a directive will have a few pain points:

  • There will be no indication as to the type of component being imported during import statements which will be an important consideration since certain hooks and APIs will be unavailable in server vs. client contexts. This means during development developers will be left to potentially dive around an entire project when converting components in either direction.
  • During code review, simply adding or removing a directive for a component (and even some in-component code) won't give a developer easy access to the entire component which is absolutely necessary for the sake of review and catching mistakes.
  • ESLint, TypeScript, VS Code, and any other tool that cares about file naming won't - by default - be given an opportunity to help developers with the different context. The React compiler it appears will bear a lot of the load in terms of catching mistakes and/or educating developers, but ESLint rules file suffixes could be leaned on heavily to help teach developers about the different contexts and help them avoid mistakes.

I think it's a huge mistake to not use file suffixes or folders, but - if that decision is passed - I just want to suggest that any/all documentation around RSCs (especially for Vite, Hydrogen, Gatsby, and Next.js) remind developers that they can still use file suffixes themselves to leverage the aforementioned tools effectively. The directive is mandatory, but nudge them about file suffixes.

@kylemh
Copy link

kylemh commented Oct 11, 2022

As an aside, I hope the React team is reaching out to TypeScript, VS Code, and ESLint teams about this RFC since this could indirectly lead to a big traffic increase in their repositories for things mentioned above.

An example: #189 (comment) could be achieved with microsoft/TypeScript#35395 and 2 sets of tsconfig files (for server / client), but - realistically - VS Code isn't prepared for mass adoption of multiple root-level tsconfigs. That being said ESLint can easily leverage multiple TSConfigs to great affect.

@sebmarkbage
Copy link
Collaborator Author

One thought experiment that can help here is to think about the multiple environments currently in the ecosystem.

This is a good list for example: https://webpack.js.org/guides/package-exports/#target-environment

There's another for more recent runtimes: https://github.com/wintercg/proposal-runtime-keys

Some of these are different runtimes with the same use cases, some are different use cases in the same runtimes.

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.

@sebmarkbage
Copy link
Collaborator Author

Since this thread is getting difficult to follow I created a new RFC to summarize the current most recent proposal "use client" directive + "react-server" exports condition.

Please direct feedback to that thread instead so we can talk about the latest state. I'll close this one since this proposal is withdrawn and v2 is the active one.

#227

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.