We already added this for other thrown errors, not just console.errors.
There's a production form of this. We just missed adding this context.
Mainly the best context is the line number though which comes from owner
stacks.
Stacked on #30197.
This is similar to #30182 and #21610 in Fizz.
Track the current owner/stack/task on the task. This tracks it for
attribution when serializing child properties.
This lets us provide the right owner and createTask when we
console.error from inside Flight itself. This also affects the way we
print those logs on the client since we need the owner and stack. Now
console.errors that originate on the server gets the right stack on the
client:
<img width="760" alt="Screenshot 2024-07-03 at 6 03 13 PM"
src="https://github.com/facebook/react/assets/63648/913300f8-f364-4e66-a19d-362e8d776c64">
Unfortunately, because we don't track the stack we never pop it so it'll
keep tracking for serializing sibling properties. We rely on "children"
typically being the last property in the common case anyway. However,
this can lead to wrong attribution in some cases where the invalid
property is a next property (without a wrapping element) and there's a
previous element that doesn't. E.g. `<ClientComponent title={<div />}
invalid={nonSerializable} />` would use the div as the attribution
instead of ClientComponent.
I also wrap all of our own console.error, onError and onPostpone in the
context of the parent component. It's annoying to have to remember to do
this though.
We could always wrap the whole rendering in such as context but it would
add more overhead since this rarely actually happens. It might make
sense to track the whole current task instead to lower the overhead.
That's what we do in Fizz. We'd still have to remember to restore the
debug task though. I realize now Fizz doesn't do that neither so the
debug task isn't wrapping the console.errors that Fizz itself logs.
There's something off about that Flight and Fizz implementations don't
perfectly align.
Wire up owner stacks in Flight to the shared internals. This exposes it
to `captureOwnerStack()`.
In this case we install it permanently as we only allow one RSC renderer
which then supports async contexts. Same thing we do for owner.
This also ends up adding it to errors logged by React through
`consoleWithStackDev`. The plan is to eventually remove that but this is
inline with what we do in Fizz and Fiber already.
However, at the same time we've instrumented the console so we need to
strip them back out before sending to the client. This lets the client
control whether to add the stack back in or allowing
`console.createTask` to control it.
This is another reason we shouldn't append them from React but for now
we hack it by removing them after the fact.
We use this to encode the binary length of a large string without
escaping it. This is really kind of optional though. This lets a Server
that can't encode strings but just pass them along able to emit RSC -
albeit a less optimal format.
The only build we have that does that today is react-html but the FB
version of Flight had a similar constraint.
It's still possible to support binary data as long as
byteLengthOfBinaryChunk is implemented which doesn't require a text
encoder. Many streams (including Node streams) support binary OR string
chunks.
This is all behind the `enableOwnerStacks` flag.
This is a follow up to #29088. In that I moved type validation into the
renderer since that's the one that knows what types are allowed.
However, I only removed it from `React.createElement` and not the JSX
which was an oversight.
However, I also noticed that for invalid types we don't have the right
stack trace for throws because we're not yet inside the JSX element that
itself is invalid. We should use its stack for the stack trace. That's
the reason it's enough to just use the throw now because we can get a
good stack trace from the owner stack. This is fixed by creating a fake
Throw Fiber that gets assigned the right stack.
Additionally, I noticed that for certain invalid types like the most
common one `undefined` we error in Flight so a missing import in RSC
leads to a generic error. Instead of erroring on the Flight side we
should just let anything that's not a Server Component through to the
client and then let the Client renderer determine whether it's a valid
type or not. Since we now have owner stacks through the server too, this
will still be able to provide a good stack trace on the client that
points to the server in that case.
<img width="571" alt="Screenshot 2024-06-25 at 6 46 35 PM"
src="https://github.com/facebook/react/assets/63648/6812c24f-e274-4e09-b4de-21deda9ea1d4">
To get the best stack you have to expand the little icon and the regular
stack is noisy [due to this Chrome
bug](https://issues.chromium.org/issues/345248263) which makes it a
little harder to find but once that's fixed it might be easier.
When we replay logs we badge them with e.g. `[Server]`. That way it's
easy to identify that the source of the log actually happened on the
Server (RSC). However, when we threw an error we didn't have any such
thing. The error was rethrown on the client and then handled just like
any other client error.
This transfers the `environmentName` in DEV to our restored Error
"sub-class" (conceptually) along with `digest`. That way you can read
`error.environmentName` to print this in your own UI.
I also updated our default for `onCaughtError` (and `onError` in Fizz)
to use the `printToConsole` helper that the Flight Client uses to log it
with the badge format. So by default you get the same experience as
console.error for caught errors:
<img width="810" alt="Screenshot 2024-06-10 at 9 25 12 PM"
src="https://github.com/facebook/react/assets/63648/8490fedc-09f6-4286-9332-fbe6b0faa2d3">
<img width="815" alt="Screenshot 2024-06-10 at 9 39 30 PM"
src="https://github.com/facebook/react/assets/63648/bdcfc554-504a-4b1d-82bf-b717e74975ac">
Unfortunately I can't do the same thing for `onUncaughtError` nor
`onRecoverableError` because they use `reportError` which doesn't have
custom formatting (unless we also prevented default on window.onerror).
However maybe that's ok because 1) you should always have an error
boundary 2) it's not likely that an RSC error can actually recover
because it's not going to be rendered again so shouldn't really happen
outside some parent conditionally rendering maybe.
The other problem with this approach is that the default is no longer
trivial - so reimplementing the default in user space is trickier and
ideally we shouldn't expose our default to be called.
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 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.
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.
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.
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 #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.
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.
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.
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.
Alternative to #28620.
Instead of emitting lazy references to not-yet-emitted models in the
Flight Server, this fixes the observed issue in
https://github.com/unstubbable/ai-rsc-test/pull/1 by adjusting the lazy
model resolution in the Flight Client to update stale blocked root
models, before assigning them as chunk values. In addition, the element
props are not outlined anymore in the Flight Server to avoid having to
also handle their staleness in blocked elements.
fixes#28595
Currently you can accidentally pass React Element to a Server Action. It
warns but in prod it actually works because we can encode the symbol and
otherwise it's mostly a plain object. It only works if you only pass
host components and no function props etc. which makes it potentially
error later. The first thing this does it just early hard error for
elements.
I made Lazy work by unwrapping though since that will be replaced by
Promises later which works.
Our protocol is not fully symmetric in that elements flow from Server ->
Client. Only the Server can resolve Components and only the client
should really be able to receive host components. It's not intended that
a Server can actually do something with them other than passing them to
the client.
In the case of a Reply, we expect the client to be stateful. It's
waiting for a response. So anything we can't serialize we can still pass
by reference to an in memory object. So I introduce the concept of a
TemporaryReferenceSet which is an opaque object that you create before
encoding the reply. This then stashes any unserializable values in this
set and encode the slot by id. When a new response from the Action then
returns we pass the same temporary set into the parser which can then
restore the objects. This lets you pass a value by reference to the
server and back into another slot.
For example it can be used to render children inside a parent tree from
a server action:
```
export async function Component({ children }) {
"use server";
return <div>{children}</div>;
}
```
(You wouldn't normally do this due to the waterfalls but for advanced
cases.)
A common scenario where this comes up accidentally today is in
`useActionState`.
```
export function action(state, formData) {
"use server";
if (errored) {
return <div>This action <strong>errored</strong></div>;
}
return null;
}
```
```
const [errors, formAction] = useActionState(action);
return <div>{errors}<div>;
```
It feels like I'm just passing the JSX from server to client. However,
because `useActionState` also sends the previous state *back* to the
server this should not actually be valid. Before this PR this actually
worked accidentally. You get a DEV warning but it used to work in prod.
Once you do something like pass a client reference it won't work tho. We
could perhaps make client references work by stashing where we got them
from but it wouldn't work with all possible JSX.
By adding temporary references to the action implementation this will
work again - on the client. It'll also be more efficient since we don't
send back the JSX content that you shouldn't introspect on the server
anyway.
However, a flaw here is that the progressive enhancement of this case
won't work because we can't use temporary references for progressive
enhancement since there's no in memory stash. What is worse is that it
won't error if you hydrate. ~It also will error late in the example
above because the first state is "undefined" so invoking the form once
works - it errors on the second attempt when it tries to send the error
state back again.~ It actually errors on the first invocation because we
need to eagerly serialize "previous state" into the form. So at least
that's better.
I think maybe the solution to this particular pattern would be to allow
JSX to serialize if you have no temporary reference set, and remember
client references so that client references can be returned back to the
server as client references. That way anything you could send from the
server could also be returned to the server. But it would only deopt to
serializing it for progressive enhancement. The consequence of that
would be that there's a lot of JSX that might accidentally seem like it
should work but it's only if you've gotten it from the server before
that it works. This would have to have pair them somehow though since
you can't take a client reference from one implementation of Flight and
use it with another.
The idea here is that host dispatchers are not bound to renders so we
need to be able to dispatch to them at any time. This updates the
implementation to chain these dispatchers so that each renderer can
respond to the dispatch. Semantically we don't always want every
renderer to do this for instance if Fizz handles a float method we don't
want Fiber to as well so each dispatcher implementation can decide if it
makes sense to forward the call or not. For float methods server
disaptchers will handle the call if they can resolve a Request otherwise
they will forward. For client dispatchers they will handle the call and
always forward. The choice needs to be made for each dispatcher method
and may have implications on correct renderer import order. For now we
just live with the restriction that if you want to use server and client
together (such as renderToString in the browser) you need to import the
server renderer after the client renderer.
Builds on top of #28384.
This prefixes each log with a badge similar to how we badge built-ins
like "ForwardRef" and "Memo" in the React DevTools. The idea is that we
can add such badges in DevTools for Server Components too to carry on
the consistency.
This puts the "environment" name in the badge which defaults to
"Server". So you know which source it is coming from.
We try to use the same styling as the React DevTools. We use light-dark
mode where available to support the two different color styles, but if
it's not available I use a fixed background so that it's always readable
even in dark mode.
In Terminals, instead of hard coding colors that might not look good
with some themes, I use the ANSI color code to flip
background/foreground colors in that case.
In earlier commits I had it on the end of the line similar to the
DevTools badges but for multiline I found it better to prefix it. We
could try various options tough.
In most cases we can use both ANSI and the `%c` CSS color specifier,
because node will only use ANSI and hide the other. Chrome supports both
but the color overrides ANSI if it comes later (and Chrome doesn't
support color inverting anyway). Safari/Firefox prints the ANSI, so it
can only use CSS colors.
Therefore in browser builds I exclude ANSI.
On the server I support both so if you use Chrome inspector on the
server, you get nice colors on both terminal and in the inspector.
Since Bun uses WebKit inspector and it prints the ANSI we can't safely
emit both there. However, we also can't emit just the color specifier
because then it prints in the terminal.
https://github.com/oven-sh/bun/issues/9021 So we just use a plain string
prefix for now with a bracket until that's fixed.
Screen shots:
<img width="758" alt="Screenshot 2024-02-21 at 12 56 02 AM"
src="https://github.com/facebook/react/assets/63648/4f887ffe-fffe-4402-bf2a-b7890986d60c">
<img width="759" alt="Screenshot 2024-02-21 at 12 56 24 AM"
src="https://github.com/facebook/react/assets/63648/f32d432f-f738-4872-a700-ea0a78e6c745">
<img width="514" alt="Screenshot 2024-02-21 at 12 57 10 AM"
src="https://github.com/facebook/react/assets/63648/205d2e82-75b7-4e2b-9d9c-aa9e2cbedf39">
<img width="489" alt="Screenshot 2024-02-21 at 12 57 34 AM"
src="https://github.com/facebook/react/assets/63648/ea52d1e4-b9fa-431d-ae9e-ccb87631f399">
<img width="516" alt="Screenshot 2024-02-21 at 12 58 23 AM"
src="https://github.com/facebook/react/assets/63648/52b50fac-bec0-471d-a457-1a10d8df9172">
<img width="956" alt="Screenshot 2024-02-21 at 12 58 56 AM"
src="https://github.com/facebook/react/assets/63648/0096ed61-5eff-4aa9-8a8a-2204e754bd1f">
When developing in an RSC environment, you should be able to work in a
single environment as if it was a unified environment. With thrown
errors we already serialize them and then rethrow them on the client.
Since by default we log them via onError both in Flight and Fizz, you
can get the same log in the RSC runtime, the SSR runtime and on the
client.
With console logs made in SSR renders, you typically replay the same
code during hydration on the client. So for example warnings already
show up both in the SSR logs and on the client (although not guaranteed
to be the same). You could just spend your time in the client and you'd
be fine.
Previously, RSC logs would not be replayed because they don't hydrate.
So it's easy to miss warnings for example.
With this approach, we replay RSC logs both during SSR so they end up in
the SSR logs and on the client. That way you can just stay in the
browser window during normal development cycles. You shouldn't have to
care if your component is a server or client component when working on
logical things or iterating on a product.
With this change, you probably should mostly ignore the Flight log
stream and just look at the client or maybe the SSR one. Unless you're
digging into something specific. In particular if you just naively run
both Flight and Fizz in the same terminal you get duplicates. I like to
run out fixtures `yarn dev:region` and `yarn dev:global` in two separate
terminals.
Console logs may contain complex objects which can be inspected. Ideally
a DevTools inspector could reach into the RSC server and remotely
inspect objects using the remote inspection protocol. That way complex
objects can be loaded on demand as you expand into them. However, that
is a complex environment to set up and the server might not even be
alive anymore by the time you inspect the objects. Therefore, I do a
best effort to serialize the objects using the RSC protocol but limit
the depth that can be rendered.
This feature is only own in dev mode since it can be expensive.
In a follow up, I'll give the logs a special styling treatment to
clearly differentiate them from logs coming from the client. As well as
deal with stacks.
Depends on:
- #28317
- #28320
---
Changes the behavior of the JSX runtime to pass through `ref` as a
normal prop, rather than plucking it from the props object and storing
on the element.
This is a breaking change since it changes the type of the receiving
component. However, most code is unaffected since it's unlikely that a
component would have attempted to access a `ref` prop, since it was not
possible to get a reference to one.
`forwardRef` _will_ still pluck `ref` from the props object, though,
since it's extremely common for users to spread the props object onto
the inner component and pass `ref` as a differently named prop. This is
for maximum compatibility with existing code — the real impact of this
change is that `forwardRef` is no longer required.
Currently, refs are resolved during child reconciliation and stored on
the fiber. As a result of this change, we can move ref resolution to
happen only much later, and only for components that actually use them.
Then we can remove the `ref` field from the Fiber type. I have not yet
done that in this step, though.
Same as #28327 but for Fizz.
One thing that's weird about this recoverable error is that we don't
send the regular stack for it, just the component stack it seems. This
is missing some potential information and if we move toward integrated
since stacks it would be one thing.
Also deals with symbols. Alternative to #28312.
We currently always normalize rejections or thrown values into `Error`
objects. Partly because in prod it'll be an error object and you
shouldn't fork behavior on knowing the value outside a digest. We might
want to even make the message always opaque to avoid being tempted and
then discover in prod that it doesn't work.
However, we do include the message in DEV.
If this is a non-Error object we don't know what the properties mean.
Ofc, we don't want to include too much information in the rendered
string, so we use the general `describeObjectForErrorMessage` helper.
Unfortunately it's pretty conservative about emitting values so it's
likely to exclude any embedded string atm. Could potentially expand it a
bit.
We could in theory try to serialize as much as possible and re-throw the
actual object to allow for inspection to be expanded inside devtools
which is what I plan on for consoles, but since we're normalizing to an
Error this is in conflict with that approach.
That way we can use it for debug information like component stacks and
DevTools. I used an extra stack argument in Child Fiber to track this as
it's flowing down since it's not just elements where we have this info
readily available but parent arrays and lazy can merge this into the
Fiber too. It's not great that this is a dev-only argument and I could
track it globally but seems more likely to make mistakes.
It is possible for the same debug info to appear for multiple child
fibers like when it's attached to a fragment or a lazy that resolves to
a fragment at the root. The object identity could be used in these
scenarios to infer if that's really one server component that's a parent
of all children or if each child has a server component with the same
name.
This is effectively a public API because you can use it to stash
information on Promises from a third-party service - not just Server
Components. I started outline the types for this for some things I was
planning to add but it's not final.
I was also planning on storing it from `use(thenable)` for when you
suspend on a Promise. However, I realized that there's no Hook instance
for those to stash it on. So it might need a separate data structure to
stash the previous pass over of `use()` that resets each render.
No tests yet since I didn't want to test internals but it'll be covered
once we have debugging features like component stacks.
A Flight Server can be a consumer of a stream from another Server. In
this case the meta data is attached to debugInfo properties on lazy,
Promises, Arrays or Elements that might in turn get forwarded to the
next stream. In this case we want to forward this debug information to
the client in the stream.
I also added a DEV only `environmentName` option to the Flight Server.
This lets you name the server that is producing the debug info so that
you can trace the origin of where that component is executing. This
defaults to `"server"`. DevTools could use this for badges or different
colors.
In #28123 I switched these to be lazy references. However that creates a
lazy wrapper even if they're synchronously available. We try to as much
as possible preserve the original data structure in these cases.
E.g. here in the dev outlining I only use a lazy wrapper if it didn't
complete synchronously:
https://github.com/facebook/react/pull/28272/files#diff-d4c9c509922b3671d3ecce4e051df66dd5c3d38ff913c7a7fe94abc3ba2ed72eR638
Unfortunately we don't have a data structure that tracks the status of
each emitted row. We could store the task in the map but then they
couldn't be GC:ed as they complete. We could maybe store the status of
each element but seems so heavy.
For now I just went back to direct reference which might be an issue
since it can suspend something higher up when deduped.