While the goal is to remove legacy context completely, I think we can
already land the removal of legacy context for function components. I
didn't even know this feature existed until reading the code recently.
The win is just a couple of property lookups on function renders, but it
trims down the API already as the full removal will likely still take a
bit more time.
www: Starting with enabled test renderer and a feature flag for
production rollout.
RN: Not enabled, will follow up on this.
For environments that still have legacy contexts available, this adds a
warning to make the remaining call sites easier to locate and encourage
upgrades.
Component stacks have a similar problem to the problem with keyPath
where we had to move it down and set it late right before recursing.
Currently we work around that by popping exactly one off when something
suspends. That doesn't work with the new server stacks being added which
are more than one. It also meant that we kept having add a single frame
that could be popped when there shouldn't need to be one.
Unlike keyPath component stacks has this weird property that once
something throws we might need the stack that was attempted for errors
or the previous stack if we're going to retry and just recreate it.
I've tried a few different approaches and I didn't like either but this
is the one that seems least problematic.
I first split out renderNodeDestructive into a retryNode helper. During
retries only retryNode is called. When we first discover a node, we pass
through renderNodeDestructive.
Instead of add a component stack frame deep inside renderNodeDestructive
after we've already refined a node, we now add it before in
renderNodeDestructive. That way it's only added once before being
attempted. This is similar to how Fiber works where in ChildFiber we
match the node once to create the instance and then later do we attempt
to actually render it and it's only the second part that's ever retried.
This unfortunately means that we now have to refine the node down to
element/lazy/thenables twice. To avoid refining the type too I move that
to be done lazily.
This is the same change as in #30289 but for the main runtime - e.g.
parent stacks in errorInfo.componentStack, appended stacks to
console.error coming from React itself and when we add virtual frames to
owner stacks.
Since we don't add location information these frames look weird to some
stack parsers - such as the native one. This is an existing issue when
you want to use some off-the-shelf parsers to parse production component
stacks for example.
While we won't add Error objects to logs ourselves necessarily, some
third party could want to do the same thing we do in DevTools and so we
should provide the same capability to just take this trace and print it
using an Error object.
Stacked on #30170.
This lets us track Server Component parent stacks in Fizz which also
lets us track the correct owner stack for lazy.
In Fiber we're careful not to make any DEV only fibers but since the
ReactFizzComponentStack data structures just exist for debug meta data
anyway we can just expand on that.
When we added component stacks to Fizz in prod it severely slowed down
common cases where you intentionally are throwing error for purposes of
client rendering. Our parent component stack generation is very slow
since call components with fake errors to generate them.
Therefore we disabled them in prod but included them in prerenders.
https://github.com/facebook/react/pull/27850
However, we still kept generating data structures for them and the code
still exists there for the prerenders. We could stop generating the data
structures which are not completely free but also not crazy bad.
What we can do instead is just lazily generate the component stacks.
This is in fact what plain stacks do anyway. This doesn't work as well
in Fiber because the data structures are live but on the server they're
immutable so it's fine to do it later as well.
That way you can choose to not read this getter for intentionally thrown
errors - after inspecting the Error object - yet still get component
stacks in prod for other errors.
Stacked on #30142.
This tracks owners and their stacks in DEV in Fizz. We use the
ComponentStackNode as the data structure to track this information -
effectively like ReactComponentInfo (Server) or Fiber (Client). They're
the instance.
I then port them same logic from ReactFiberComponentStack,
ReactFiberOwnerStack and ReactFiberCallUserSpace to Fizz equivalents.
This gets us both owner stacks from `captureOwnerStack()`, as well as
appended to console.errors logged by Fizz, while rendering and in
onError.
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.
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.
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.
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.
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.
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.
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.
`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.
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.
This PR relands #28672 on top of the flag removal and the test
demonstrating a breakage in Suspense for legacy mode.
React has deprecated module pattern Function Components for many years
at this point. Supporting this pattern required React to have a concept
of an indeterminate component so that when a component first renders it
can turn into either a ClassComponent or a FunctionComponent depending
on what it returns. While this feature was deprecated and put behind a
flag it is still in stable. This change remvoes the flag, removes the
warnings, and removes the concept of IndeterminateComponent from the
React codebase.
While removing IndeterminateComponent type Seb and I discovered that we
needed a concept of IncompleteFunctionComponent to support Suspense in
legacy mode. This new work tag is only needed as long as legacy mode is
around and ideally any code that considers this tag will be excludable
from OSS builds once we land extra gates using `disableLegacyMode` flag.
We previously only included the component stack.
Cleaned up the fields in Fizz server that wasn't using consistent hidden
classes in dev vs prod.
Added a prefix to errors serialized from server rendering. It can be a
bit confusing to see where this error came from otherwise since it
didn't come from elsewhere on the client. It's really kind of confusing
with other recoverable errors that happen on the client too.
Remove module pattern function component support (flag only)
> This is a redo of #27742, but only including the flag removal,
excluding further simplifications.
The module pattern
```
function MyComponent() {
return {
render() {
return this.state.foo
}
}
}
```
has been deprecated for approximately 5 years now. This PR removes
support for this pattern.
This breaks internal tests, so must be something in the refactor. Since
it's the top commit let's revert and split into two PRs, one that
removes the flag and one that does the refactor, so we can find the bug.
The module pattern
```
function MyComponent() {
return {
render() {
return this.state.foo
}
}
}
```
has been deprecated for approximately 5 years now. This PR removes
support for this pattern. It also simplifies a number of code paths in
particular related to the concept of `IndeterminateComponent` types.
## Overview
The error messages that say:
> ReactDOM.hydrate is no longer supported in React 18
Don't make sense in the React 19 release. Instead, they should say:
> ReactDOM.hydrate was removed in React 19.
For legacy mode, they should say:
> ReactDOM.hydrate has not been supported since React 18.
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.
This pattern is a petpeeve of mine. I don't consider this best practice
and so most don't have these prefixes. Very inconsistent.
At best this is useless and noisey that you have to parse because the
information is also in the stack trace.
At worse these are misleading because they're highlighting something
internal (like validateDOMNesting) which even suggests an internal bug.
Even the ones public to React aren't necessarily what you called because
you might be calling a wrapper around it.
That would be properly reflected in a stack trace - which can also
properly ignore list so that the first stack you see is your callsite,
Which might be like `render()` in react-testing-library rather than
`createRoot()` for example.
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.
Also warn for symbols.
It's weird because for objects we throw a hard error but functions we do
a dev only check. Mainly because we have an object branch anyway.
In the object branch we have some built-ins that have bad errors like
forwardRef and memo but since they're going to become functions later, I
didn't bother updating those. Once they're functions those names will be
part of this.
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.
Previously, `<Context>` was equivalent to `<Context.Consumer>`. However,
since the introduction of Hooks, the `<Context.Consumer>` API is rarely
used. The goal here is to make the common case cleaner:
```js
const ThemeContext = createContext('light')
function App() {
return (
<ThemeContext value="dark">
...
</ThemeContext>
)
}
function Button() {
const theme = use(ThemeContext)
// ...
}
```
This is technically a breaking change, but we've been warning about
rendering `<Context>` directly for several years by now, so it's
unlikely much code in the wild depends on the old behavior. [Proof that
it warns today (check
console).](https://codesandbox.io/p/sandbox/peaceful-nobel-pdxtfl)
---
**The relevant commit is 5696782b428a5ace96e66c1857e13249b6c07958.** It
switches `createContext` implementation so that `Context.Provider ===
Context`.
The main assumption that changed is that a Provider's fiber type is now
the context itself (rather than an intermediate object). Whereas a
Consumer's fiber type is now always an intermediate object (rather than
it being sometimes the context itself and sometimes an intermediate
object).
My methodology was to start with the relevant symbols, work tags, and
types, and work my way backwards to all usages.
This might break tooling that depends on inspecting React's internal
fields. I've added DevTools support in the second commit. This didn't
need explicit versioning—the structure tells us enough.
Server Context was never documented, and has been deprecated in
https://github.com/facebook/react/pull/27424.
This PR removes it completely, including the implementation code.
Notably, `useContext` is removed from the shared subset, so importing it
from a React Server environment would now should be a build error in
environments that are able to enforce that.
Semantically if you make your reason for aborting a Postpone instance
the render should not hit the error pathways but should instead follow
the postpone pathways. It's awkward today to actually get your hands on
a Postpone instance because you have to catch the throw from postpone
and then pass that into `abort()` or `AbortController.abort()`
(depending on the renderer API you are using)
This change makes it so that in most circumstances if you abort with a
postpone the `onPostpone` handler will be called and the Suspense
boundaries still pending will be put into client render mode with the
appropriate postpone digest to avoid trigger recoverable error pathways
on the client.
Similar to postponing in the shell during a resume or render however if
you abort before the shell is complete in a resume or render we will
fatally error. The fatal error is contextualized by React to avoid
passing the postpone object itself to the `onError` and related options.
Updates Fizz to handle Hoistables (Resources and Elements) in a way that
better aligns with Suspense fallbacks
1. Hoistable Elements inside a fallback (regardless of how deep and how
many additional boundaries are intermediate) will be ignored. The
reasoning is fallbacks are transient and since there is not good way to
clean up hoistables because they escape their Suspense container its
better to not emit them in the first place. SSR fallbacks are already
not full fidelity because they never hydrate so this aligns with that
somewhat.
2. Hoistable stylesheets in fallbacks will only block the reveal of a
parent suspense boundary if the fallback is going to flush with that
completed parent suspense boundary. Previously if you rendered a
stylesheet Resource inside a fallback any parent suspense boundaries
that completed after the shell flushed would include that resource in
the set required to resolve before the boundary reveal happens on the
client. This is not a semantic change, just a performance optimization
3. preconnect and preload hoistable queues are gone, if you want to
optimize resource loading you shoudl use `ReactDOM.preconnect` and
`ReactDOM.preload`. `viewport` meta tags get their own queue because
they need to go before any preloads since they affect the media state.
In addition to those functional changes this PR also refactors the
boundary resource tracking by moving it to the task rather than using
function calls at the start of each render and flush. Tasks also now
track whether they are a fallback task
supercedes prior work here: https://github.com/facebook/react/pull/27534
Before, we used to reset the thenable state and extract the previous
state very early so that it's only the retried task that can possibly
consume it. This is nice because we can't accidentally consume that
state for any other node.
However, it does add a lot of branches of code that has to pass this
around. It also adds extra bytes on the stack per node. Even though it's
mostly just null.
This changes it so that where ever we can create a thenable state (e.g.
entering a component with hooks) we first extract this from the task.
The principle is that whatever could've created the thenable state in
the first place, must always be rerendered so it'll take the same code
paths to get there and so we'll always consume it.
This refactors the Flight render loop to behave more like Fizz with
similar naming conventions. So it's easier to apply similar techniques
across both. This is not necessarily better/faster - at least not yet.
This doesn't yet implement serialization by writing segments to chunks
but we probably should do that since the built-in parts that
`JSON.stringify` gets us isn't really much anymore (except serializing
strings). When we switch to that it probably makes sense for the whole
thing to be recursive.
Right now it's not technically fully recursive because each recursive
render returns the next JSON value to encode. So it's kind of like a
trampoline. This means we can't have many contextual things on the
stack. It needs to use the Server Context `__POP` trick. However, it
does work for things that are contextual only for one sequence of server
component abstractions in a row. Since those are now recursive.
An interesting observation here is that `renderModel` means that
anything can suspend while still serializing the outer siblings.
Typically only Lazy or Components would suspend but in principle a Proxy
can suspend/postpone too and now that is left serialized by reference to
a future value. It's only if the thing that we rendered was something
that can reduce to Lazy e.g. an Element that we can serialize it as a
lazy.
Similarly to how Suspense boundaries in Fizz can catch errors, anything
that can be reduced to Lazy can also catch an error rather than bubbling
it. It only errors when the Lazy resolves. Unlike Suspense boundaries
though, those things don't render anything so they're otherwise going to
use the destructive form. To ensure that throwing in an Element can
reuse the current task, this must be handled by `renderModel`, not for
example `renderElement`.
If we end up client rendering a boundary due to an error after we have
already injected a postponed hole in that boundary we'll end up trying
to target a missing segment. Since we never insert segments for an
already errored boundary into the HTML. Normally an errored prerender
wouldn't be used but if it is, such as if it was an intentional client
error it triggers this case. Those should really be replaced with
postpones though.
This is a bit annoying since we eagerly build up the postponed path. I
took the easy route here and just cleared out the suspense boundary
itself from having any postponed slots. However, this still creates an
unnecessary replay path along the way to the boundary. We could probably
walk the path and remove any empty parent nodes.
What is worse is that if this is the only thing that postponed, we'd
still generate a postponed state even though there's actually nothing to
resume. Since this is a bit of an edge case already maybe it's fine.
In my test I added a check for the `error` event on `window` since this
error only surfaces by throwing an ignored error. We should really do
that globally for all tests. Our tests should fail by default if there's
an error logged to the window.
Historically React would produce component stacks for dev builds only.
There is a cost to tracking component stacks and given the prod builds
try to optimize runtime performance these stacks were left out. More
recently React added production component stacks to Fiber in because it
can be immensely helpful in tracking down hard to debug production
issues. Fizz was not updated to have a similar behavior.
With the advent of prerendering however stacks for production in Fizz
are more relevant because prerendering is not really a dev-time task. If
you want the ability to reason about errors or postpones that happen
during a prerender having component stacks to interrogate is helpful and
these component stacks need to be available in production otherwise you
are really never going to see them. (it is possible that you could do
dev-mode prerenders but we don't expect this to be a common dev mode
workflow)
To better support the prerender use case and to make error logging in
Fizz more useful the following changes have been made
1. `onPostpone` now accepts a second `postponeInfo` argument which will
contain a componentStack. Postpones always originate from a component
render so the stack should be consistently available. The type however
will indicate the stack is optional so we can remove them in the future
if we decide the overhead is the wrong tradeoff in certain cases
2. `onError` now accepts a second `errorInfo` argument which may contain
a componentStack. If an error originated from a component a stack will
be included in the following cases.
This change entails tracking the component hierarchy in prod builds now.
While this isn't cost free it is implemented in a relatively lean
manner. Deferring the most expensive work (reifying the stack) until we
are actually in an error pathway.
In the course of implementing this change a number of simplifications
were made to the code which should make the stack tracking more
resilient. We no longer use a module global to curry the stack up to
some handler. This was delicate because you needed to always reset it
properly. We now curry the stack on the task itself.
Another change made was to track the component stack on SuspenseBoundary
instances so that we can provide the stack when aborting suspense
boundaries to help you determine which ones were affected by an abort.
`onHeaders` can throw however for now we can assume that headers are
optimistic values since the only things we produce for them are preload
links. This is a pragmatic decision because React could concievably have
headers in the future which were not optimistic and thus non-optional
however it is hard to imagine what these headers might be in practice.
If we need to change this behavior to be fatal in the future it would be
a breaking change.
This commit adds error logging when `onHeaders` throws and ensures the
request can continue to render successfully.
When we postpone during a render we inject a new segment synchronously
which we postpone. That gets assigned an ID so we can refer to it
immediately in the postponed state.
When we do that, the parent segment may complete later even though it's
also synchronous. If that ends up not having any content in it, it'll
inline into the child and that will override the child's segment id
which is not correct since it was already assigned one.
To fix this, we simply opt-out of the optimization in that case which is
unfortunate because we'll generate many more unnecessary empty segments.
So we should come up with a new strategy for segment id assignment but
this fixes the bug.
Co-authored-by: Josh Story <story@hey.com>
Jan KassensandGitHub
Sebastian MarkbågeandGitHub
Andrew ClarkandGitHub
RickyandGitHub
danandGitHub