-
Notifications
You must be signed in to change notification settings - Fork 47.4k
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
[Flight Parcel] Implement prepareDestinationForModule #31799
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
function Content() { | ||
data ??= createFromReadableStream<ReactElement>(s1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW this took me a long time to debug why preinits weren't working. This must be initialized during render so the current request is set in AsyncLocalStorage. Might be nice if there was some kind of error/warning instead of silently not inserting any scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An issue is that it's legit to have an optimistic preload or preinit in module scope. E.g. the first time you load a module on the client. The silent ignoring allows those same modules to keep working with SSR or RSC albeit without the preload.
However, this is also related to another issue that it should be possible to consume values from the RSC stream (like formState) before SSR:ing the payload. It's also useful for other routing concerns. You can see this awkwardness in the fixtures/flight
where it has to parse it twice to deal with the cycle of somethings having to be outside the render and somethings inside.
This is also related to console replaying when doing RSC to RSC since the replays have to be in the scope that needs to replay.
Another case is expanding the React.cache scope to longer than just one RSC render.
We have plans to add something that allows expanding the scopes for these AsyncLocalStorage contexts. It's just a little unclear the exact shape atm.
@sebmarkbage if you have a chance, lmk if this approach looks ok and if you have any thoughts on how we could make CSS a bit more automatic. |
I think the essential part of making CSS automatic is to somehow detect that the boundary itself is a React Node and converting it into to something that renders basically the equivalent of For example, detecting that a client reference is only used within a specific JSX call: import {ClientComponent} from ...
...
<ClientComponent /> Compiles to import {ClientComponent, Resources} from ...
...
<>
<Resources />
<ClientComponent />
</> Now the inner CSS can be excluded from the outer parent (e.g. route/page). The same principle can be applied to React specific async loading like const LazyComponent = React.lazy(() => import(...));
...
<LazyComponent /> Compiles to const LazyComponent = React.lazy(async () => {
const {default: Component, Resources} = await import(...)
return {
default: function Wrapper(props) {
return <>
<Resources />
<Component {...props} />
</>;
}
}
});
...
<LazyComponent /> Just as examples. The key here is that if we just assigned something to the ClientReference or something it might be easy to lose the additional information in a way that's not safe using a wrapper. So it's more about being conservative and only do this when it's safe. There are ofc, non-React centric ways of lazy loading code. Even RSC lets you pass non-Components. import {ClientComponent} from ...
import {clientFunction} from ...
...
<ClientComponent createDOMNode={clientFunction} /> "use client";
export function ClientComponent({createDOMNode}) {
const ref = useRef();
useEffect(() => {
ref.current.appendChild(createDOMNode())
}, [])
return <div ref={ref} />
} or the equivalent to export function ClientComponent(props) {
const ref = useRef();
useEffect(() => {
import(...).then(({ createDOMNode }) => {
ref.current.appendChild(createDOMNode())
});
}, [])
return <div ref={ref} />
} In these cases, the lazy import might itself depend on CSS which needs to be loaded. There's basically two approaches to this. You can either hoist into the parent route so that it's eagerly loaded as part of the parent module loading, or you can do it using the old technique of just treating it like a JS module so it's loaded before the The downsize of the latter is that it doesn't work if that async dependency ends up as SSR because in this case React wouldn't know that you really needed the CSS for this to load because showing this on the client since there's no suspensey CSS: const lazy = import(...); // hoisting this out just to show that AsyncLocalStorage isn't enough
export function ClientComponent(props) {
const getText = use(lazy.then(({default}) => default));
return getText();
} The same case with RSC + SSR: "use client";
export function ClientComponent({ clientFunction }) {
return clientFunction(); // This might be another Client Reference which depends on CSS.
} Another tricky case is if the same CSS chunks are depended upon by both a React Node aware module (which can treat it as suspensey) and another plain JS dynamic So one possible approach would be: a) Detect things that are definitely only used by React Nodes and treat those as suspensey CSS. Another option is to just ignore the SSR problem. I.e. treat all of c as b. That's really the old status quo of the React ecosystem anyway. You didn't used to be able to await arbitrary imports in React before neither. It's a lost standing preexisting problem. |
In general we opted to for more hoisting into the parent Page for Client References in Next.js which has the downside that you might overload CSS. However, the overhead added of tracking this at each component when you have a lot of small Client References is also a downside. Adding CSS late can also be a significant performance downside since each time it has to recompute all the styles for all existing DOM nodes essentially so streaming in CSS isn't that beneficial. Combined with the benefits of just using Atomic CSS in general that flattens out over time meant that hoisting into coarser boundaries in general is more beneficial. However, ideally even that should be at the prerogative of the bundler to make the choice since it has more information about things like how large the chunks are. |
This general problem space is also just a specific version of the problem where native browser CSS modules return a StyleSheet that needs to be injected somewhere instead of a global side-effect. |
Yeah, I was thinking about generating wrapper components to inject the Resources automatically like you described. Maybe dynamic importing a module that has a CSS dependency but not rendering anything from it as a JSX element is enough of an edge case that it could be handled manually. The hard part is actually detecting statically if a dynamic import is used as a JSX element. There are lots of different ways that could happen - property access (could be dynamic or through some function), destructuring, Another problem is that adding wrapper components messes with the object identity of the component being imported. If someone attached additional properties to their function or something, those would also need to be proxied. function Component() {
// ...
}
// This would need to be proxied by the wrapper.
Components.something = 'whatever'; Another idea I thought of is to "tag" the exports of a boundary module with the resources it uses, and then React itself could potentially inject them at runtime. Not sure if this is a good idea. Basically like this: function Component() {
// ...
}
// Injected by compiler
Component[Symbol.for('react.resources')] = <>
<link rel="stylesheet" ... />
</>; And then when React sees a JSX element with a type that has a resources property attached to it, it could add those resources into the tree as siblings. |
Followup to #31725
This implements
prepareDestinationForModule
in the Parcel Flight client. On the Parcel side, the<Resources>
component now only inserts<link>
elements for stylesheets (along with a bootstrap script when needed), and React is responsible for inserting scripts. This ensures that components that are conditionally dynamic imported during render are also preloaded.CSS must be added to the RSC tree using
<Resources>
to avoid FOUC. This must be manually rendered in both the top-level page, and in any component that is dynamic imported. It would be nice if there was a way for React to automatically insert CSS as well, but unfortunatelyprepareDestinationForModule
only knows about client components and not CSS for server components. Perhaps there could be a way we could annotate components at code splitting boundaries with the resources they need? More thoughts in this thread: #31725 (comment)