Need to tighten up this a bit.
react-dom isomorphic currently depends on react-reconciler which is
mostly DCE but it's pulled in which makes it hard to make other bundling
changes.
ReactFlightServer can have a hard dependency on the module that imports
its internals since even if other internals are aliased it still always
needs the server one.
This lets the environment name vary within a request by the context a
component, log or error being executed in.
A potentially different API would be something like
`setEnvironmentName()` but we'd have to extend the `ReadableStream` or
something to do that like we do for `.allReady`. As a function though it
has some expansion possibilities, e.g. we could potentially also pass
some information to it for context about what is being asked for.
If it changes before completing a task, we also emit the change so that
we have the debug info for what the environment was before entering a
component and what it was after completing it.
Stacked on #29807.
This lets the nearest Suspense/Error Boundary handle it even if that
boundary is defined by the model itself.
It also ensures that when we have an error during serialization of
properties, those can be associated with the nearest JSX element and
since we have a stack/owner for that element we can use it to point to
the source code of that line. We can't track the source of any nested
arbitrary objects deeper inside since objects don’t track their stacks
but close enough. Ideally we have the property path but we don’t have
that right now. We have a partial in the message itself.
<img width="813" alt="Screenshot 2024-06-09 at 10 08 27 PM"
src="https://github.com/facebook/react/assets/63648/917fbe0c-053c-4204-93db-d68a66e3e874">
Note: The component name (Counter) is lost in the first message because
we don't print it in the Task. We use `"use client"` instead because we
expect the next stack frame to have the name. We also don't include it
in the actual error message because the Server doesn't know the
component name yet. Ideally Client References should be able to have a
name. If the nearest is a Host Component then we do use the name though.
However, it's not actually inside that Component that the error happens
it's in App and that points to the right line number.
An interesting case is that if something that's actually going to be
consumed by the props to a Suspense/Error Boundary or the Client
Component that wraps them fails, then it can't be handled by the
boundary. However, a counter intuitive case might be when that's on the
`children` props. E.g.
`<ErrorBoundary>{clientReferenceOrInvalidSerialization}</ErrorBoundary>`.
This value can be inspected by the boundary so it's not safe to pass it
so if it's errored it is not caught.
## Implementation
The first insight is that this is best solved on the Client rather than
in the Server because that way it also covers Client References that end
up erroring.
The key insight is that while we don't have a true stack when using
`JSON.parse` and therefore no begin/complete we can still infer these
phases for Elements because the first child of an Element is always
`'$'` which is also a leaf. In depth first that's our begin phase. When
the Element itself completes, we have the complete phase. Anything in
between is within the Element.
Using this idea I was able to refactor the blocking tracking mechanism
to stash the blocked information on `initializingHandler` and then on
the way up do we let whatever is nearest handle it - whether that's an
Element or the root Chunk. It's kind of like an Algebraic Effect.
cc @unstubbable This is something you might want to deep dive into to
find more edge cases. I'm sure I've missed something.
---------
Co-authored-by: eps1lon <sebastian.silbermann@vercel.com>
Stacked on #29807.
Conceptually the error's owner/task should ideally be captured when the
Error constructor is called but neither `console.createTask` does this,
nor do we override `Error` to capture our `owner`. So instead, we use
the nearest parent as the owner/task of the error. This is usually the
same thing when it's thrown from the same async component but not if you
await a promise started from a different component/task.
Before this stack the "owner" and "task" of a Lazy that errors was the
nearest Fiber but if the thing erroring is a Server Component, we need
to get that as the owner from the inner most part of debugInfo.
To get the Task for that Server Component, we need to expose it on the
ReactComponentInfo object. Unfortunately that makes the object not
serializable so we need to special case this to exclude it from
serialization. It gets restored again on the client.
Before (Shell):
<img width="813" alt="Screenshot 2024-06-06 at 5 16 20 PM"
src="https://github.com/facebook/react/assets/63648/7da2d4c9-539b-494e-ba63-1abdc58ff13c">
After (App):
<img width="811" alt="Screenshot 2024-06-08 at 12 29 23 AM"
src="https://github.com/facebook/react/assets/63648/dbf40bd7-c24d-4200-81a6-5018bef55f6d">
We know from Fiber that inline objects with more than 16 properties in
V8 turn into dictionaries instead of optimized objects. The trick is to
use a constructor instead of an inline object literal. I don't actually
know if that's still the case or not. I haven't benchmarked/tested the
output. Better safe than sorry.
It's unfortunate that this can have a negative effect for Hermes and JSC
but it's not as bad as it is for V8 because they don't deopt into
dictionaries. The time to construct these objects isn't a concern - the
time to access them frequently is.
We have to beware the Task objects in Fizz. Those are currently on 16
fields exactly so we shouldn't add anymore ideally.
We should ideally have a lint rule against object literals with more
than 16 fields on them. It might not help since sometimes the fields are
conditional.
Stacked on #29491
Previously if you aborted during a render the currently rendering task
would itself be aborted which will cause the entire model to be replaced
by the aborted error rather than just the slot currently being rendered.
This change updates the abort logic to mark currently rendering tasks as
aborted but allowing the current render to emit a partially serialized
model with an error reference in place of the current model.
The intent is to support aborting from rendering synchronously, in
microtasks (after an await or in a .then) and in lazy initializers. We
don't specifically support aborting from things like proxies that might
be triggered during serialization of props
Stacked on #29551
Flight pings much more often than Fizz because async function components
will always take at least a microtask to resolve . Rather than
scheduling this work as a new macrotask Flight now schedules pings in a
microtask. This allows more microtasks to ping before actually doing a
work flush but doesn't force the vm to spin up a new task which is quite
common give n the nature of Server Components
While most builds of Flight and Fizz schedule work in new tasks some do
execute work synchronously. While this is necessary for legacy APIs like
renderToString for modern APIs there really isn't a great reason to do
this synchronously.
We could schedule works as microtasks but we actually want to yield so
the runtime can run events and other things that will unblock additional
work before starting the next work loop.
This change updates all non-legacy uses to be async using the best
availalble macrotask scheduler.
Browser now uses postMessage
Bun uses setTimeout because while it also supports setImmediate the
scheduling is not as eager as the same API in node
the FB build also uses setTimeout
This change required a number of changes to tests which were utilizing
the sync nature of work in the Browser builds to avoid having to manage
timers and tasks. I added a patch to install MessageChannel which is
required by the browser builds and made this patched version integrate
with the Scheduler mock. This way we can effectively use `act` to flush
flight and fizz work similar to how we do this on the client.
This lets us ensure that we use the original V8 format and it lets us
skip source mapping. Source mapping every call can be expensive since we
do it eagerly for server components even if an error doesn't happen.
In the case of an error being thrown we don't actually always do this in
practice because if a try/catch before us touches it or if something in
onError touches it (which the default console.error does), it has
already been initialized. So we have to be resilient to thrown errors
having other formats.
These are not as perf sensitive since something actually threw but if
you want better perf in these cases, you can simply do something like
`onError(error) { console.error(error.message) }` instead.
The server has to be aware whether it's looking up original or compiled
output. I currently use the file:// check to determine if it's referring
to a source mapped file or compiled file in the fixture. A bundled app
can more easily check if it's a bundle or not.
Normally we take the renderClientElement path but this is an internal
fast path.
No tests because we don't run tests with console.createTask (which is
not easy since we test component stacks).
Ideally this would be covered by types but since the types don't
consider flags and DEV it doesn't really help.
We have three kinds of stacks that we send in the RSC protocol:
- The stack trace where a replayed `console.log` was called on the
server.
- The JSX callsite that created a Server Component which then later
called another component.
- The JSX callsite that created a Host or Client Component.
These stack frames disappear in native stacks on the client since
they're executed on the server. This evals a fake file which only has
one call in it on the same line/column as the server. Then we call
through these fake modules to "replay" the callstack. We then replay the
`console.log` within this stack, or call `console.createTask` in this
stack to recreate the stack.
The main concern with this approach is the performance. It adds
significant cost to create all these eval:ed functions but it should
eventually balance out.
This doesn't yet apply source maps to these. With source maps it'll be
able to show the server source code when clicking the links.
I don't love how these appear.
- Because we haven't yet initialized the client module we don't have the
name of the client component we're about to render yet which leads to
the `<...>` task name.
- The `(async)` suffix Chrome adds is still a problem.
- The VMxxxx prefix is used to disambiguate which is noisy. Might be
helped by source maps.
- The continuation of the async stacks end up rooted somewhere in the
bootstrapping of the app. This might be ok when the bootstrapping ends
up ignore listed but it's kind of a problem that you can't clear the
async stack.
<img width="927" alt="Screenshot 2024-05-28 at 11 58 56 PM"
src="https://github.com/facebook/react/assets/63648/1c9d32ce-e671-47c8-9d18-9fab3bffabd0">
<img width="431" alt="Screenshot 2024-05-28 at 11 58 07 PM"
src="https://github.com/facebook/react/assets/63648/52f57518-bbed-400e-952d-6650835ac6b6">
<img width="327" alt="Screenshot 2024-05-28 at 11 58 31 PM"
src="https://github.com/facebook/react/assets/63648/d311a639-79a1-457f-9a46-4f3298d07e65">
<img width="817" alt="Screenshot 2024-05-28 at 11 59 12 PM"
src="https://github.com/facebook/react/assets/63648/3aefd356-acf4-4daa-bdbf-b8c8345f6d4b">
This didn't actually fail before but I'm just adding an extra check.
Currently Client References are always "function" proxies so they never
fall into this branch. However, we do in theory support objects as
client references too depending on environment. We have checks
elsewhere. So this just makes that consistent.
This one should be fully behind the `enableOwnerStacks` flag.
Instead of printing the parent Component stack all the way to the root,
this now prints the owner stack of every JSX callsite. It also includes
intermediate callsites between the Component and the JSX call so it has
potentially more frames. Mainly it provides the line number of the JSX
callsite. In terms of the number of components is a subset of the parent
component stack so it's less information in that regard. This is usually
better since it's more focused on components that might affect the
output but if it's contextual based on rendering it's still good to have
parent stack. Therefore, I still use the parent stack when printing DOM
nesting warnings but I plan on switching that format to a diff view
format instead (Next.js already reformats the parent stack like this).
__Follow ups__
- Server Components show up in the owner stack for client logs but logs
done by Server Components don't yet get their owner stack printed as
they're replayed. They're also not yet printed in the server logs of the
RSC server.
- Server Component stack frames are formatted as the server and added to
the end but this might be a different format than the browser. E.g. if
server is running V8 and browser is running JSC or vice versa. Ideally
we can reformat them in terms of the client formatting.
- This doesn't yet update Fizz or DevTools. Those will be follow ups.
Fizz still prints parent stacks in the server side logs. The stacks
added to user space `console.error` calls by DevTools still get the
parent stacks instead.
- It also doesn't yet expose these to user space so there's no way to
get them inside `onCaughtError` for example or inside a custom
`console.error` override.
- In another follow up I'll use `console.createTask` instead and
completely remove these stacks if it's available.
This is necessary to simplify the component stack handling to make way
for owner stacks. It also solves some hacks that we used to have but
don't quite make sense. It also solves the problem where things like key
warnings get silenced in RSC because they get deduped. It also surfaces
areas where we were missing key warnings to begin with.
Almost every type of warning is issued from the renderer. React Elements
are really not anything special themselves. They're just lazily invoked
functions and its really the renderer that determines there semantics.
We have three types of warnings that previously fired in
JSX/createElement:
- Fragment props validation.
- Type validation.
- Key warning.
It's nice to be able to do some validation in the JSX/createElement
because it has a more specific stack frame at the callsite. However,
that's the case for every type of component and validation. That's the
whole point of enableOwnerStacks. It's also not sufficient to do it in
JSX/createElement so we also have validation in the renderers too. So
this validation is really just an eager validation but also happens
again later.
The problem with these is that we don't really know what types are valid
until we get to the renderer. Additionally, by placing it in the
isomorphic code it becomes harder to do deduping of warnings in a way
that makes sense for that renderer. It also means we can't reuse logic
for managing stacks etc.
Fragment props validation really should just be part of the renderer
like any other component type. This also matters once we add Fragment
refs and other fragment features. So I moved this into Fiber. However,
since some Fragments don't have Fibers, I do the validation in
ChildFiber instead of beginWork where it would normally happen.
For `type` validation we already do validation when rendering. By
leaving it to the renderer we don't have to hard code an extra list.
This list also varies by context. E.g. class components aren't allowed
in RSC but client references are but we don't have an isomorphic way to
identify client references because they're defined by the host config so
the current logic is flawed anyway. I kept the early validation for now
without the `enableOwnerStacks` since it does provide a nicer stack
frame but with that flag on it'll be handled with nice stacks anyway. I
normalized some of the errors to ensure tests pass.
For `key` validation it's the same principle. The mechanism for the
heuristic is still the same - if it passes statically through a parent
JSX/createElement call then it's considered validated. We already did
print the error later from the renderer so this also disables the early
log in the `enableOwnerStacks` flag.
I also added logging to Fizz so that key warnings can print in SSR logs.
Flight is a bit more complex. For elements that end up on the client we
just pass the `validated` flag along to the client and let the client
renderer print the error once rendered. For server components we log the
error from Flight with the server component as the owner on the stack
which will allow us to print the right stack for context. The factoring
of this is a little tricky because we only want to warn if it's in an
array parent but we want to log the error later to get the right debug
info.
Fiber/Fizz has a similar factoring problem that causes us to create a
fake Fiber for the owner which means the logs won't be associated with
the right place in DevTools.
In #29201 a fix was made to ensure we don't "forget" about some
listeners when handling cyclic chunks.
In #29204 another fix was made for a special case when the chunk already
has listeners before it first resolves.
This implements the followup fix for Flight Reply which was originally
missed in #29204
Co-authored-by: Janka Uryga <lolzatu2@gmail.com>
Fixes#29200
The cyclic state might have added listeners that will still need to be
invoked. This happens if we have a cyclic reference AND end up blocked.
We have already cleared these before entering the parsing when we enter
the CYCLIC state so we they already have the right type. If listeners
are added during this phase they should carry over to the blocked state.
---------
Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
This errors on the client normally but in the case the `type` is a
function - i.e. a Server Component - it wouldn't be transferred to error
on the client so you end up with a worse error message. So this just
implements the same check as ChildFiber.
Stacked on #28997.
We can use the technique of referencing an object by its row + property
name path for temporary references - like we do for deduping. That way
we don't need to generate an ID for temporary references. Instead, they
can just be an opaque marker in the slot and it has the implicit ID of
the row + path.
Then we can stash all objects, even the ones that are actually available
to read on the server, as temporary references. Without adding anything
to the payload since the IDs are implicit. If the same object is
returned to the client, it can be referenced by reference instead of
serializing it back to the client. This also helps preserve object
identity.
We assume that the objects are immutable when they pass the boundary.
I'm not sure if this is worth it but with this mechanism, if you return
the `FormData` payload from a `useActionState` it doesn't have to be
serialized on the way back to the client. This is a common pattern for
having access to the last submission as "default value" to the form
fields. However you can still control it by replacing it with another
object if you want. In MPA mode, the temporary references are not
configured and so it needs to be serialized in that case. That's
required anyway for hydration purposes.
I'm not sure if people will actually use this in practice though or if
FormData will always be destructured into some other object like with a
library that turns it into typed data, and back. If so, the object
identity is lost.
Uses the same technique as in #28996 to encode references to already
emitted objects. This now means that Reply can support cyclic objects
too for parity.
Instead of forcing an object to be outlined to be able to refer to it
later we can refer to it by the property path inside another parent
object.
E.g. this encodes such a reference as `'$123:props:children:foo:bar'`.
That way we don't have to preemptively outline object and we can dedupe
after the first time we've found it.
There's no cost on the client if it's not used because we're not storing
any additional information preemptively.
This works mainly because we only have simple JSON objects from the root
reference. Complex objects like Map, FormData etc. are stored as their
entries array in the look up and not the complex object. Other complex
objects like TypedArrays or imports don't have deeply nested objects in
them that can be referenced.
This solves the problem that we only dedupe after the third instance.
This dedupes at the second instance. It also solves the problem where
all nested objects inside deduped instances also are outlined.
The property paths can get pretty large. This is why a test on payload
size increased. We could potentially outline the reference itself at the
first dupe. That way we get a shorter ID to refer to in the third
instance.
This is the first step to experimenting with a new type of stack traces
behind the `enableOwnerStacks` flag - in DEV only.
The idea is to generate stacks that are more like if the JSX was a
direct call even though it's actually a lazy call. Not only can you see
which exact JSX call line number generated the erroring component but if
that's inside an abstraction function, which function called that
function and if it's a component, which component generated that
component. For this to make sense it really need to be the "owner" stack
rather than the parent stack like we do for other component stacks. On
one hand it has more precise information but on the other hand it also
loses context. For most types of problems the owner stack is the most
useful though since it tells you which component rendered this
component.
The problem with the platform in its current state is that there's two
ways to deal with stacks:
1) `new Error().stack`
2) `console.createTask()`
The nice thing about `new Error().stack` is that we can extract the
frames and piece them together in whatever way we want. That is great
for constructing custom UIs like error dialogs. Unfortunately, we can't
take custom stacks and set them in the native UIs like Chrome DevTools.
The nice thing about `console.createTask()` is that the resulting stacks
are natively integrated into the Chrome DevTools in the console and the
breakpoint debugger. They also automatically follow source mapping and
ignoreLists. The downside is that there's no way to extract the async
stack outside the native UI itself so this information cannot be used
for custom UIs like errors dialogs. It also means we can't collect this
on the server and then pass it to the client for server components.
The solution here is that we use both techniques and collect both an
`Error` object and a `Task` object for every JSX call.
The main concern about this approach is the performance so that's the
main thing to test. It's certainly too slow for production but it might
also be too slow even for DEV.
This first PR doesn't actually use the stacks yet. It just collects them
as the first step. The next step is to start utilizing this information
in error printing etc.
For RSC we pass the stack along across over the wire. This can be
concatenated on the client following the owner path to create an owner
stack leading back into the server. We'll later use this information to
restore fake frames on the client for native integration. Since this
information quickly gets pretty heavy if we include all frames, we strip
out the top frame. We also strip out everything below the functions that
call into user space in the Flight runtime. To do this we need to figure
out the frames that represents calling out into user space. The
resulting stack is typically just the one frame inside the owner
component's JSX callsite. I also eagerly strip out things we expect to
be ignoreList:ed anyway - such as `node_modules` and Node.js internals.
This is the same change as #28780 but for the Flight Reply receiver.
While it's not possible to create an "async module" reference in this
case - resolving a server reference can still be async if loading it
requires loading chunks like in a new server instance.
Since extracting a typed array from a Blob is async, that's also a case
where a dependency can be async.
Same as #28847 but in the other direction.
Like other promises, this doesn't actually stream in the outgoing
direction. It buffers until the stream is done. This is mainly due to
our protocol remains compatible with Safari's lack of outgoing streams
until recently.
However, the stream chunks are encoded as separate fields and so does
support the busboy streaming on the receiving side.
Stacked on #28798.
Add another AsyncLocalStorage to the FlightServerConfig. This context
tracks data on a per component level. Currently the only thing we track
is the owner in DEV.
AsyncLocalStorage around each component comes with a performance cost so
we only do it DEV. It's not generally a particularly safe operation
because you can't necessarily associate side-effects with a component
based on execution scope. It can be a lazy initializer or cache():ed
code etc. We also don't support string refs anymore for a reason.
However, it's good enough for optional dev only information like the
owner.
Rebasing and landing https://github.com/facebook/react/pull/28798
This PR was approved already but held back to give time for the sync.
Rebased and landing here without pushing to seb's remote to avoid
possibility of lost updates
---------
Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>
Stacked on #28849, #28854, #28853. Behind a flag.
If you're following along from the side-lines. This is probably not what
you think it is.
It's NOT a way to get updates to a component over time. The
AsyncIterable works like an Iterable already works in React which is how
an Array works. I.e. it's a list of children - not the value of a child
over time.
It also doesn't actually render one component at a time. The way it
works is more like awaiting the entire list to become an array and then
it shows up. Before that it suspends the parent.
To actually get these to display one at a time, you have to opt-in with
`<SuspenseList>` to describe how they should appear. That's really the
interesting part and that not implemented yet.
Additionally, since these are effectively Async Functions and uncached
promises, they're not actually fully "supported" on the client yet for
the same reason rendering plain Promises and Async Functions aren't.
They warn. It's only really useful when paired with RSC that produces
instrumented versions of these. Ideally we'd published instrumented
helpers to help with map/filter style operations that yield new
instrumented AsyncIterables.
The way the implementation works basically just relies on unwrapThenable
and otherwise works like a plain Iterator.
There is one quirk with these that are different than just promises. We
ask for a new iterator each time we rerender. This means that upon retry
we kick off another iteration which itself might kick off new requests
that block iterating further. To solve this and make it actually
efficient enough to use on the client we'd need to stash something like
a buffer of the previous iteration and maybe iterator on the iterable so
that we can continue where we left off or synchronously iterate if we've
seen it before. Similar to our `.value` convention on Promises.
In Fizz, I had to do a special case because when we render an iterator
child we don't actually rerender the parent again like we do in Fiber.
However, it's more efficient to just continue on where we left off by
reusing the entries from the thenable state from before in that case.
Stacked on #28853 and #28854.
React supports rendering `Iterable` and will soon support
`AsyncIterable`. As long as it's multi-shot since during an update we
may have to rerender with new inputs an loop over the iterable again.
Therefore the `Iterator` and `AsyncIterator` types are not supported
directly as a child of React - and really it shouldn't pass between
Hooks or components neither for this reason. For parity, that's also the
case when used in Server Components.
However, there is a special case when the component rendered itself is a
generator function. While it returns as a child an `Iterator`, the React
Element itself can act as an `Iterable` because we can re-evaluate the
function to create a new generator whenever we need to.
It's also very convenient to use generator functions over constructing
an `AsyncIterable`. So this is a proposal to special case the
`Generator`/`AsyncGenerator` returned by a (Async) Generator Function.
In Flight this means that when we render a Server Component we can
serialize this value as an `Iterable`/`AsyncIterable` since that's
effectively what rendering it on the server reduces down to. That way if
Fiber can receive the result in any position.
For SuspenseList this would also need another special case because the
children of SuspenseList represent "rows".
`<SuspenseList><Component /></SuspenseList>` currently is a single "row"
even if the component renders multiple children or is an iterator. This
is currently different if Component is a Server Component because it'll
reduce down to an array/AsyncIterable and therefore be treated as one
row per its child. This is different from `<SuspenseList><Component
/><Component /></SuspenseList>` since that has a wrapper array and so
this is always two rows.
It probably makes sense to special case a single-element child in
`SuspenseList` to represent a component that generates rows. That way
you can use an `AsyncGeneratorFunction` to do this.
For [`AsyncIterable`](https://github.com/facebook/react/pull/28847) we
encode `AsyncIterator` as a separate tag.
Previously we encoded `Iterator` as just an Array. This adds a special
encoding for this. Technically this is a breaking change.
This is kind of an edge case that you'd care about the difference but it
becomes more important to treat these correctly for the warnings here
#28853.
This doesn't change production behavior. We always render Iterables to
our best effort in prod even if they're Iterators.
But this does change the DEV warnings which indicates which are valid
patterns to use.
It's a footgun to use an Iterator as a prop when you pass between
components because if an intermediate component rerenders without its
parent, React won't be able to iterate it again to reconcile and any
mappers won't be able to re-apply. This is actually typically not a
problem when passed only to React host components but as a pattern it's
a problem for composability.
We used to warn only for Generators - i.e. Iterators returned from
Generator functions. This adds a warning for Iterators created by other
means too (e.g. Flight or the native Iterator utils). The heuristic is
to check whether the Iterator is the same as the Iterable because that
means it's not possible to get new iterators out of it. This case used
to just yield non-sense like empty sets in DEV but not in prod.
However, a new realization is that when the Component itself is a
Generator Function, it's not actually a problem. That's because the
React Element itself works as an Iterable since we can ask for new
generators by calling the function again. So this adds a special case to
allow the Generator returned from a Generator Function's direct child.
The principle is “don’t pass iterators around” but in this case there is
no iterator floating around because it’s between React and the JS VM.
Also see #28849 for context on AsyncIterables.
Related to this, but Hooks should ideally be banned in these for the
same reason they're banned in Async Functions.
This disables symbol renaming in production builds. The original
variable and function names are preserved. All other forms of
compression applied by Closure (dead code elimination, inlining, etc)
are unchanged — the final program is identical to what we were producing
before, just in a more readable form.
The motivation is to make it easier to debug React issues that only
occur in production — the same reason we decided to start shipping
sourcemaps in #28827 and #28827.
However, because most apps run their own minification step on their npm
dependencies, it's not necessary for us to minify the symbols before
publishing — it'll be handled the app, if desired.
This is the same strategy Meta has used to ship React for years. The
React build itself has unminified symbols, but they get minified as part
of Meta's regular build pipeline.
Even if an app does not minify their npm dependencies, gzip covers most
of the cost of symbol renaming anyway.
This saves us from having to ship sourcemaps, which means even apps that
don't have sourcemaps configured will be able to debug the React build
as easily as they would any other npm dependency.
So that when we end up referring to it in more places, it's only one.
We don't do this same pattern for regular `Symbol.iterator` because we
also support the string `"@@iterator"` for backwards compatibility.
This adds support in Flight for serializing four kinds of streams:
- `ReadableStream` with objects as a model. This is a single shot
iterator so you can read it only once. It can contain any value
including Server Components. Chunks are encoded as is so if you send in
10 typed arrays, you get the same typed arrays out on the other side.
- Binary `ReadableStream` with `type: 'bytes'` option. This supports the
BYOB protocol. In this mode, the receiving side just gets `Uint8Array`s
and they can be split across any single byte boundary into arbitrary
chunks.
- `AsyncIterable` where the `AsyncIterator` function is different than
the `AsyncIterable` itself. In this case we assume that this might be a
multi-shot iterable and so we buffer its value and you can iterate it
multiple times on the other side. We support the `return` value as a
value in the single completion slot, but you can't pass values in
`next()`. If you want single-shot, return the AsyncIterator instead.
- `AsyncIterator`. These gets serialized as a single-shot as it's just
an iterator.
`AsyncIterable`/`AsyncIterator` yield Promises that are instrumented
with our `.status`/`.value` convention so that they can be synchronously
looped over if available. They are also lazily parsed upon read.
We can't do this with `ReadableStream` because we use the native
implementation of `ReadableStream` which owns the promises.
The format is a leading row that indicates which type of stream it is.
Then a new row with the same ID is emitted for every chunk. Followed by
either an error or close row.
`AsyncIterable`s can also be returned as children of Server Components
and then they're conceptually the same as fragment arrays/iterables.
They can't actually be used as children in Fizz/Fiber but there's a
separate plan for that. Only `AsyncIterable` not `AsyncIterator` will be
valid as children - just like sync `Iterable` is already supported but
single-shot `Iterator` is not. Notably, neither of these streams
represent updates over time to a value. They represent multiple values
in a list.
When the server stream is aborted we also close the underlying stream.
However, closing a stream on the client, doesn't close the underlying
stream.
A couple of possible follow ups I'm not planning on doing right now:
- [ ] Free memory by releasing the buffer if an Iterator has been
exhausted. Single shots could be optimized further to release individual
items as you go.
- [ ] We could clean up the underlying stream if the only pending data
that's still flowing is from streams and all the streams have cleaned
up. It's not very reliable though. It's better to do cancellation for
the whole stream - e.g. at the framework level.
- [ ] Implement smarter Binary Stream chunk handling. Currently we wait
until we've received a whole row for binary chunks and copy them into
consecutive memory. We need this to preserve semantics when passing
typed arrays. However, for binary streams we don't need that. We can
just send whatever pieces we have so far.
With the enableBinaryFlight flag on we should encode typed arrays and
blobs in the Reply direction too for parity.
It's already possible to pass Blobs inside FormData but you should be
able to pass them inside objects too.
We encode typed arrays as blobs and then unwrap them automatically to
the right typed array type.
Unlike the other protocol, I encode the type as a reference tag instead
of row tag. Therefore I need to rename the tags to avoid conflicts with
other tags in references. We are running out of characters though.
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
The ReadableStreamController for [direct
streams](https://bun.sh/docs/api/streams#direct-readablestream) in Bun
supports a flush() method to flush all buffered items to its underlying
sink.
Without manually calling flush(), all buffered items are only flushed to
the underlying sink when the stream is closed. This behavior causes the
shell rendered against Suspense boundaries never to be flushed to the
underlying sink.
## How did you test this change?
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
A lot of changes to the test runner will need to be made in order to
support the Bun runtime. A separate test was manually run in order to
ensure that the changes made are correct.
The test works by sanity-checking that the shell rendered against
Suspense boundaries are emitted first in the stream.
This test was written and run on Bun v1.1.3.
```ts
import { Suspense } from "react";
import { renderToReadableStream } from "react-dom/server";
if (!import.meta.resolveSync("react-dom/server").endsWith("server.bun.js")) {
throw new Error("react-dom/server is not the correct version:\n " + import.meta.resolveSync("react-dom/server"));
}
const A = async () => {
await new Promise(resolve => setImmediate(resolve));
return <div>hi</div>;
};
const B = async () => {
return (
<Suspense fallback={<div>loading</div>}>
<A />
</Suspense>
);
};
const stream = await renderToReadableStream(<B />);
let text = "";
let count = 0;
for await (const chunk of stream) {
text += new TextDecoder().decode(chunk);
count++;
}
if (
text !==
`<!--$?--><template id="B:0"></template><div>loading</div><!--/$--><div hidden id="S:0"><div>hi</div></div><script>$RC=function(b,c,e){c=document.getElementById(c);c.parentNode.removeChild(c);var a=document.getElementById(b);if(a){b=a.previousSibling;if(e)b.data="$!",a.setAttribute("data-dgst",e);else{e=b.parentNode;a=b.nextSibling;var f=0;do{if(a&&8===a.nodeType){var d=a.data;if("/$"===d)if(0===f)break;else f--;else"$"!==d&&"$?"!==d&&"$!"!==d||f++}d=a.nextSibling;e.removeChild(a);a=d}while(a);for(;c.firstChild;)e.insertBefore(c.firstChild,a);b.data="$"}b._reactRetry&&b._reactRetry()}};$RC("B:0","S:0")</script>`
) {
throw new Error("unexpected output");
}
if (count !== 2) {
throw new Error("expected 2 chunks from react ssr stream");
}
```
Hoistables should never flush before the preamble however there is a
surprisingly easy way to trigger this to happen by suspending in the
shell of the app. This change modifies the flushing behavior to not emit
any hoistables before the preamble has written. It accomplishes this by
aborting the flush early if there are any pending root tasks remaining.
It's unfortunate we need this extra condition but it's essential that we
don't emit anything before the preamble and at the moment I don't see a
way to do that without introducing a new condition.
There is a test that began to fail with this update. It turns out that
in node the root can be blocked during a resume even for a component
inside a Suspense boundary if that boundary was part of the prerender.
This means that with the current heuristic in this PR boundaries cannot
be flushed during resume until the root is unblocked. This is not ideal
but this is already how Edge works because the root blocks the stream in
that case. This just makes Node deopt in a similar way to edge. We
should improve this but we ought to do so in a way that works for edge
too and it needs to be more comprehensive.
Follow up to #28783 and #28786.
Since we've changed the implementations of these we can rename them to
something a bit more descriptive while we're at it, since anyone
depending on them will need to upgrade their code anyway.
"react" with no condition:
`__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
"react" with "react-server" condition:
`__SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
"react-dom":
`__DOM_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
We have a different set of dispatchers that Flight uses. This also
includes the `jsx-runtime` which must also be aliased to use the right
version.
To ensure the right versions are used together we rename the export of
the SharedInternals from 'react' and alias it in relevant bundles.
This is similar to #28771 but for isomorphic. We need a make over for
these dispatchers anyway so this is the first step. Also helps flush out
some internals usage that will break anyway.
It flattens the inner mutable objects onto the ReactSharedInternals.
We used to assume that outlined models are emitted before the reference
(which was true before Blobs). However, it still wasn't safe to assume
that all the data will be available because an "import" (client
reference) can be async and therefore if it's directly a child of an
outlined model, it won't be able to update in place.
This is a similar problem as the one hit by @unstubbable in #28669 with
elements, but a little different since these don't follow the same way
of wrapping.
I don't love the structuring of this code which now needs to pass a
first class mapper instead of just being known code. It also shares the
host path which is just an identity function. It wouldn't necessarily
pass my own review but I don't have a better one for now. I'd really
prefer if this was done at a "row" level but that ends up creating even
more code.
Add test for Blob in FormData and async modules in Maps.
We currently support FormData for Replies mainly for Form Actions. This
supports it in the other direction too which lets you return it from an
action as the response. Mainly for parity.
We don't really recommend that you just pass the original form data back
because the action is supposed to be able to clear fields and such but
you could potentially at least use this as the format and could clear
some fields.
We could potentially optimize this with a temporary reference if the
same object was passed to a reply in case you use it as a round trip to
avoid serializing it back again. That way the action has the ability to
override it to clear fields but if it doesn't you get back the same as
you sent.
#28755 adds support for Blobs when the `enableBinaryFlight` is enabled
which allows them to be used inside FormData too.
`delete` causes an object (in V8, and maybe other engines) to deopt to a
dictionary instead of a class. Instead of `assign` + `delete`, manually
iterate over all the properties, like the JSX runtime does.
To avoid copying the object twice I moved the `ref` prop removal to come
before handling default props. If we already cloned the props to remove
`ref`, then we can skip cloning again to handle default props.
We currently support Blobs when passing from Client to Server so this
adds it in the other direction for parity - when `enableFlightBinary` is
enabled.
We intentionally only support the `Blob` type to pass-through, not
subtype `File`. That's because passing additional meta data like
filename might be an accidental leak. You can still pass a `File`
through but it'll appear as a `Blob` on the other side. It's also not
possible to create a faithful File subclass in all environments without
it actually being backed by a file.
This implementation isn't great but at least it works. It creates a few
indirections. This is because we need to be able to asynchronously emit
the buffers but we have to "block" the parent object from resolving
while it's loading.
Ideally, we should be able to create the Blob on the client early and
then stream in it lazily. Because the Blob API doesn't guarantee that
the data is available synchronously. Unfortunately, the native APIs
doesn't have this. We could implement custom versions of all the data
read APIs but then the blobs still wouldn't work with native APIs. So we
just have to wait until Blob accepts a stream in the constructor.
We should be able to stream each chunk early in the protocol though even
though we can't unblock the parent until they've all loaded. I didn't do
this yet mostly because of code structure and I'm lazy.
This implements the concept of a DEV-only "owner" for Server Components.
The owner concept isn't really super useful. We barely use it anymore,
but we do have it as a concept in DevTools in a couple of cases so this
adds it for parity. However, this is mainly interesting because it could
be used to wire up future owner-based stacks.
I do this by outlining the DebugInfo for a Server Component
(ReactComponentInfo). Then I just rely on Flight deduping to refer to
that. I refer to the same thing by referential equality so that we can
associate a Server Component parent in DebugInfo with an owner.
If you suspend and replay a Server Component, we have to restore the
same owner. To do that, I did a little ugly hack and stashed it on the
thenable state object. Felt unnecessarily complicated to add a stateful
wrapper for this one dev-only case.
The owner could really be anything since it could be coming from a
different implementation. Because this is the first time we have an
owner other than Fiber, I have to fix up a bunch of places that assumes
Fiber. I mainly did the `typeof owner.tag === 'number'` to assume it's a
Fiber for now.
This also doesn't actually add it to DevTools / RN Inspector yet. I just
ignore them there for now.
Because Server Components can be async the owner isn't tracked after an
await. We need per-component AsyncLocalStorage for that. This can be
done in a follow up.
This removes defaultProps support for all component types except for
classes. We've chosen to continue supporting defaultProps for classes
because lots of older code relies on it, and unlike function components,
(which can use default params), there's no straightforward alternative.
By implication, it also removes support for setting defaultProps on
`React.lazy` wrapper. So this will not work:
```js
const MyClassComponent = React.lazy(() => import('./MyClassComponent'));
// MyClassComponent is not actually a class; it's a lazy wrapper. So
// defaultProps does not work.
MyClassComponent.defaultProps = { foo: 'bar' };
```
However, if you set the default props on the class itself, then it's
fine.
For classes, this change also moves where defaultProps are resolved.
Previously, defaultProps were resolved by the JSX runtime. This change
is only observable if you introspect a JSX element, which is relatively
rare but does happen.
In other words, previously `<ClassWithDefaultProp />.props.aDefaultProp`
would resolve to the default prop value, but now it does not.
Follow up to #28684.
V8 includes the message in the stack and printed errors include just the
stack property which is assumed to contain the message. Without this,
the prefix doesn't get printed in the console.
<img width="578" alt="Screenshot 2024-04-03 at 6 32 04 PM"
src="https://github.com/facebook/react/assets/63648/d98a2db4-6ebc-4805-b669-59f449dfd21f">
A possible alternative would be to use a nested error with a `cause`
like #28736 but that would need some more involved serializing since
this prefix is coming from the server. Perhaps as a separate attribute.
