This is a follow-up from #30528 to not only handle props (the critical
change), but also the owner ~and stack~ of a referenced element.
~Handling stacks here is rather academic because the Flight Server
currently does not deduplicate owner stacks. And if they are really
identical, we should probably just dedupe the whole element.~ EDIT:
Removed from the PR.
Handling owner objects on the other hand is an actual requirement as
reported in https://github.com/vercel/next.js/issues/69545. This problem
only affects the stable release channel, as the absence of owner stacks
allows for the specific kind of shared owner deduping as demonstrated in
the unit test.
When aborting we currently don't produce a componentStack when aborting
the shell. This is likely just an oversight and this change updates this
behavior to be consistent with what we do when there is a boundary
In a past update we made render and prerender have different work
scheduling behavior because these methods are meant to be used in
differeent environments with different performance tradeoffs in mind.
For instance to prioritize streaming we want to allow as much IO to
complete before triggering a round of work because we want to flush as
few intermediate UI states. With Prerendering there will never be any
intermediate UI states so we can more aggressively render tasks as they
complete.
One thing we've found is that even during render we should ideally kick
off work immediately. This update normalizes the intitial work for
render and prerender to start in a microtask. Choosing microtask over
sync is somewhat arbitrary but there really isn't a reason to make them
different between render/prerender so for now we'll unify them and keep
it as a microtask for now.
This change also updates pinging behavior. If the request is still in
the initial task that spawned it then pings will schedule on the
microtask queue. This allows immediately available async APIs to resolve
right away. The concern with doing this for normal pings is that it
might crowd out IO events but since this is the initial task there would
be IO to already be scheduled.
When the environment name changes for a chunk we issue a new debug chunk
which updates the environment name. This chunk was not beign included in
the pendingChunks count so the count was off when flushing
This means that the owner of a Component rendered on the remote server
becomes the Component on this server.
Ideally we'd support this for the Client side too. In particular Fiber
but currently ReactComponentInfo's owner is typed as only supporting
other ReactComponentInfo and it's a bigger lift to support that.
This is only in the same experimental exports as `resume`. Useful with
Postpone/Halt.
We already have `prerender()` to create a partial tree with postponed
state. We also have `resume()` to dynamically resume such a tree.
This lets you do a new prerender by resuming an already existing
postponed state. Basically creating a chain of preludes. The next
prelude would include the scripts to patch up the document.
This mostly just works since both prerender and resume are already
implemented using the same code so we just enable both at the root. I'm
sure we'll find some edge cases since this wasn't considered when it was
first written but so far I've only found an unrelated existing bug with
`keyPath` fixed here.
We added enough fields to need a constructor instead of inline object in
V8.
We didn't update the resumeRequest path though so it wasn't using the
constructor and had a different hidden class.
## Summary
This PR bumps Flow all the way to the latest 0.245.2.
Most of the suppressions comes from Flow v0.239.0's change to include
undefined in the return of `Array.pop`.
I also enabled `react.custom_jsx_typing=true` and added custom jsx
typing to match the old behavior that `React.createElement` is
effectively any typed. This is necessary since various builtin
components like `React.Fragment` is actually symbol in the React repo
instead of `React.AbstractComponent<...>`. It can be made more accurate
by customizing the `React$CustomJSXFactory` type, but I will leave it to
the React team to decide.
## How did you test this change?
`yarn flow` for all the renderers
If we see the "Maximum call stack size exceeded" error we know we've hit
stack overflow. We can recover from this by spawning a new task and
trying again. Effectively a zero-cost trampoline in the normal case. The
new task will have a clean stack. If you have a lot of siblings at the
same depth that hits the limit you can end up hitting this once for each
sibling but within that new sibling you're unlikely to hit this again.
So it's not too expensive.
If it errors again in the retryTask pass, the other error handling takes
over which causes this to be able to still not infinitely stall. E.g.
when the component itself throws an error like this.
It's still better to increase the stack limit for performance if you
have a really deep tree but it doesn't really hurt to be able to recover
since it's zero cost when it doesn't happen.
We could do the same thing for Flight. Those trees don't tend to be as
deep but could happen.
Similar to https://github.com/facebook/react/pull/30768 we want to
schedule work during prerendering in microtasks both for the root task
and pings. We continue to schedule flushes as Tasks to allow as much
work to be batched up as possible.
In https://github.com/facebook/react/pull/29491 I updated the work
scheduler for Flight to use microtasks to perform work when something
pings. This is useful but it does have some downsides in terms of our
ability to do task prioritization. Additionally the initial work is not
instantiated using a microtask which is inconsistent with how pings
work.
In this change I update the scheduling logic to use microtasks
consistently for prerenders and use regular tasks for renders both for
the initial work and pings.
When we introduced prerendering for flight we modeled an abort of a
flight prerender as having unfinished rows. This is similar to how
postpone was already implemented when you postponed from "within" a
prerender using React.unstable_postpone. However when aborting with a
postponed instance every boundary would be eagerly marked for client
rendering which is more akin to prerendering and then resuming with an
aborted signal.
The insight with the flight work was that it's not so much the postpone
that describes the intended semantics but the abort combined with a
prerender. So like in flight when you abort a prerender and enableHalt
is enabled boundaries and the shell won't error for any reason. Fizz
will still call onPostpone and onError according to the abort reason but
the consuemr of the prerender should expect to resume it before trying
to use it.
When aborting a prerender we should leave references unfulfilled, not
share a common unfullfilled reference. functionally today this doesn't
matter because we don't have resuming but the semantic is that the row
was not available when the abort happened and in a resume the row should
fill in. But by pointing each task to a common unfulfilled chunk we lose
the ability for these references to resolves to distinct values on
resume.
When aborting with a postpone value boundaries are put into client
rendered mode even during prerenders. This doesn't follow the postpoen
semantics of the rest of fizz where during a prerender a postpone is
tracked and it will leave holes in tracked postpone state that can be
resumed. This change updates this behavior to match the postpones
semantics between aborts and imperative postpones.
stacked on: #30731
We've refined the model of halting a prerender. Now when you abort
during a prerender we simply omit the rows that would complete the
flight render. This is analagous to prerendering in Fizz where you must
resume the prerender to actually result in errors propagating in the
postponed holes. We don't have a resume yet for flight and it's not
entirely clear how that will work however the key insight here is that
deciding whether the never resolving rows are an error or not should
really be done on the consuming side rather than in the producer.
This PR also reintroduces the logs for the abort error/postpone when
prerendering which will give you some indication that something wasn't
finished when the prerender was aborted.
Stacked on #30731.
When logging a Promise we emit it as an infinite promise instead of
blocking the replay on it.
This models that as a halted row instead. No need for this special case.
I unflag the receiving side since now it's used to replace a feature
that's already unflagged so it's used.
using infinitely suspending promises isn't right because this will parse
as a promise which is only appropriate if the value we're halting at is
a promise. Instead we need to have a special marker type that says this
reference will never resolve. Additionally flight client needs to not
error any halted references when the stream closes because they will
otherwise appear as an error
addresses:
https://github.com/facebook/react/pull/30705#discussion_r1720479974
This uses a similar technique to what we use to generate fake stack
frames for server components. This generates an eval:ed wrapper function
around the Server Reference proxy we create on the client. This wrapper
function gets the original `name` of the action on the server and I also
add a source map if `findSourceMapURL` is defined that points back to
the source of the server function.
For `"use server"` on the server, there's no new API. It just uses the
callsite of `registerServerReference()` on the Server. We can infer the
function name from the actual function on the server and we already have
the `findSourceMapURL` on the client receiving it.
For `"use server"` imported from the client, there's two new options
added to `createServerReference()` (in addition to the optional
[`encodeFormAction`](#27563)). These are only used in DEV mode. The
[`findSourceMapURL`](#29708) option is the same one added in #29708. We
need to pass this these references aren't created in the context of any
specific request but globally. The other weird thing about this case is
that this is actually a case where the compiled environment is the
client so any source maps are the same as for the client layer, so the
environment name here is just `"Client"`.
```diff
createServerReference(
id: string,
callServer: CallServerCallback,
encodeFormAction?: EncodeFormActionCallback,
+ findSourceMapURL?: FindSourceMapURLCallback, // DEV-only
+ functionName?: string, // DEV-only
)
```
The key is that we use the location of the
`registerServerReference()`/`createServerReference()` call as the
location of the function. A compiler can either emit those at the same
locations as the original functions or use source maps to have those
segments refer to the original location of the function (or in the case
of a re-export the original location of the re-export is also a fine
approximate). The compiled output must call these directly without a
wrapper function because the wrapper adds a stack frame. I decided
against complicated and fragile dev-only options to skip n number of
frames that would just end up in prod code. The implementation just
skips one frame - our own. Otherwise it'll just point all source mapping
to the wrapper.
We don't have a `"use server"` imported from the client implementation
in the reference implementation/fixture so it's a bit tricky to test
that. In the case of CJS on the server, we just use a runtime instead of
compiler so it's tricky to source map those appropriately. We can
implement it for ESM on the server which is the main thing we're testing
in the fixture. It's easier in a real implementation where all the
compilation is just one pass. It's a little tricky since we have to
parse and append to other source maps but I'd like to do that as a
follow up. Or maybe that's just an exercise for the reader.
You can right click an action and click "Go to Definition".
<img width="1323" alt="Screenshot 2024-08-17 at 6 04 27 PM"
src="https://github.com/user-attachments/assets/94d379b3-8871-4671-a20d-cbf9cfbc2c6e">
For now they simply don't point to the right place but you can still
jump to the right file in the fixture:
<img width="1512" alt="Screenshot 2024-08-17 at 5 58 40 PM"
src="https://github.com/user-attachments/assets/1ea5d665-e25a-44ca-9515-481dd3c5c2fe">
In Firefox/Safari given that the location doesn't exist in the source
map yet, the browser refuses to open the file. Where as Chrome does
nearest (last) line.
It is possible to throw after aborting during a render and we were not
properly tracking this. We use an AbortSigil to mark whether a rendering
task needs to abort but the throw interrupts that and we end up handling
an error on the error pathway instead.
This change reworks the abort-while-rendering support to be robust to
throws after calling abort
enableHalt turns on a mode for flight prerenders where aborts are
treated like infinitely stalled outcomes while still completing the
prerender. For regular tasks we simply serialize the slot as a promise
that never settles. For ReadableStream, Blob, and Async Iterators we
just never advance the serialization so they remain unfinished when
consumed on the client.
When enableHalt is turned on aborts of prerenders will halt rather than
error. The abort reason is forwarded to the upstream produces of the
aforementioned async iterators, blobs, and ReadableStreams. In the
future if we expose a signal that you can consume from within a render
to cancel additional work the abort reason will also be forwarded there
Prerendering in flight is similar to prerendering in Fizz. Instead of
receiving a result (the stream) immediately a promise is returned which
resolves to the stream when the prerender is complete. The promise will
reject if the flight render fatally errors otherwise it will resolve
when the render is completed or is aborted.
Supports showing the key in DevTools on the Server Component that the
key was applied to. We can also use this to reconcile to preserve
instance equality when they're reordered.
One thing that's a bit weird about this is that if you provide an
explicit key on a Server Component that alone doesn't have any
semantics. It's because we pass the key down and let the nearest child
inherit the key or get prefixed by the key.
So you might see the same key as a prefix on the child of the Server
Component too which might be a bit confusing. We could remove the prefix
from children but that might also be a bit confusing if they collide.
The div in this case doesn't have a key explicitly specified. It gets it
from the Server Component parent.
<img width="1107" alt="Screenshot 2024-08-14 at 10 06 36 PM"
src="https://github.com/user-attachments/assets/cfc517cc-e737-44c3-a1be-050049267ee2">
Overall keys get a bit confusing when you apply filter. Especially since
it's so common to actually apply the key on a Host Instance. So you
often don't see the key.
This commit updates the file locations and bulid configurations for
flight in preparation for new static entrypoints. This follows a
structure similar to Fizz which has a unified build but exports methods
from different top level entrypoints. This PR doesn't actually add the
new top level entrypoints however, that will arrive in a later update.
When synchronously aborting in a non-async Function Component if you
throw after aborting the task would error rather than abort because
React never observed the AbortSignal.
Using a sigil to throw after aborting during render isn't effective b/c
the user code itself could throw so insteead we just read the request
status. This is ok b/c we don't expect any tasks to still be pending
after the currently running task finishes.
However I found one instance where that wasn't true related to
serializing thenables which I've fixed so we may find other cases. If we
do, though it's almost certainly a bug in our task bookkeeping so we
should just fix it if it comes up.
I also updated `abort` to not set the status to ABORTING unless the
status was OPEN. we don't want to ever leave CLOSED or CLOSING status
When I implemented the ability to abort synchronoulsy in flight I made
it possible for erroring async server components to cause an unhandled
rejection error. In the current implementation if you abort during the
synchronous phase of a Function Component and then throw an error in the
synchronous phase React will not attach any promise handlers because it
short circuits the thenable treatment and throws an AbortSigil instead.
This change updates the rendering logic to ignore the rejecting
component.
When aborting with a postpone value in Fizz if any tasks are still
pending in the root while prerendering the prerender will fatally error.
This is different from postponing imperatively in a root task and really
the semantics should be the same. This change updates React to treat an
abort with a postpone value as a postponed root rather than a fatal
error.
There's a special case that happens when we replay logs on the client
because this doesn't happen within the context of any particular
rendered component. So we need to reimplement things that would normally
be handled by a full client like Fiber.
The implementation of `getOwnerStackByComponentInfoInDev` is the
simplest version since it doesn't have any client components in it so I
move it to `shared/`. It's only used by Flight but both `react-server/`
and `react-client/` packages. The ReactComponentInfo type is also more
generic than just Flight anyway.
In a follow up I still need to implement this in React DevTools when
native tasks are not available so that it appends it to the console.
Currently if you abort a Fizz render during rendering the render will
not complete correctly because there are inconsistencies with task
counting. This change updates the abort implementation to allow you to
abort from within a render itself. We already landed a similar change
for Flight in #29764
When a Fizz render is closing but not yet closed it's possible that
pinged tasks can spawn more work. The point of the closing state is to
allow time to start piping/reading the underlying stream but
semantically the render is finished at that point so work should no
longer happen.
When prerendering it can be convenient to abort the prerender while
rendering. However if any Suspense fallbacks have not yet rendered
before the abort happens the fallback itself will error and cause the
nearest parent Suspense boundary to render a fallback instead.
Prerenders are by definition not time critical so the prioritization of
children over fallbacks which makes sense for render isn't similarly
motivated for prerender. Given this, this change updates fallback
rendering during a prerender to attempt the fallback before attempting
children.
This is a major nit but this avoids an extra stack frame when we're
replaying logs.
Normally the `printToConsole` frame doesn't show up because it'd be
ignore listed.
<img width="421" alt="Screenshot 2024-07-25 at 11 49 39 AM"
src="https://github.com/user-attachments/assets/81334c2f-e19e-476a-871e-c4db9dee294e">
When you expand to show ignore listed frames a ton of other frames show
up.
<img width="516" alt="Screenshot 2024-07-25 at 11 49 47 AM"
src="https://github.com/user-attachments/assets/2ab8bdfb-464c-408d-9176-ee2fabc114b6">
The annoying thing about this frame is that it's at the top of the stack
where as typically framework stuff ends up at the bottom and something
you can ignore. The user space stack comes first.
With this fix there's no longer any `printToConsole` frame.
<img width="590" alt="Screenshot 2024-07-25 at 12 09 09 PM"
src="https://github.com/user-attachments/assets/b8365d53-31f3-43df-abce-172d608d3c9c">
Am I wiling to eat the added complexity and slightly slower performance
for this nit? Definitely.
This way you can use the environment to know where to look for the
source map in case you have multiple server environments.
This becomes part of the public protocol since it's part of what you'll
parse out of the `rsc://React/` prefixed URLs inside of
`captureOwnerStack`.
This lets you customize the filter, for example allowing node_modules or
filter out additional functions that you don't want to include when
sending the stack to the client.
Notably this doesn't filter out Server Components out of the parent
stack. Those are just like a view of the tree by name. Not virtual stack
frames.
We still filter them before passing from server to client in Flight
Server but when presenting a native stack, we don't need to filter them.
That's left to ignore listing in the presentation.
The stacks are pretty clean regardless thanks to the bottom stack
frames.
We can also unify the owner stack formatters into one shared module
since Fizz/Flight/Fiber all do the same thing. DevTools currently does
the same thing but is forked so it can support multiple versions.
Stacked on #30410.
If we've parsed another RSC stream on the server from a different RSC
server, while using `findSourceMapURL`, the Flight Client ends up adding
a `rsc://React/` prefix and a numeric suffix to the URL. It's a virtual
file that represents the virtual eval:ed frame in that environment.
If we then see that same stack again, we'd serialize a virtual frame to
another virtual. Meaning `findSourceMapURL` on the client would see the
virtual frame of the intermediate server and it would have to strip it
to figure out what source map to use.
This PR strips it in the Server if we see a virtual frame. At each new
client it always refers to the original stack.
We don't have to do this. We could leave it to each `findSourceMapURL`
implementation and `captureOwnerStack` parser to recursively strip each
layer. It could maybe be useful to have the environment name in the
virtual frame to know which server to look for the source map in.
Stacked on #30401.
Previously we were transferring the original V8 stack trace string to
the client and then parsing it there. However, really the server is the
one that knows what format it is and it should be able to vary by server
environment.
We also don't use the raw string anymore (at least not in
enableOwnerStacks). We always create the native Error stacks.
The string also made it unclear which environment it is and it was
tempting to just use it as is.
Instead I parse it on the server and make it a structured stack in the
transfer format. It also makes it clear that it needs to be formatted in
the current environment before presented.
Stacked on https://github.com/facebook/react/pull/30400 and
https://github.com/facebook/react/pull/30369
Previously we were using fake evals to recreate a stack for console
replaying and thrown errors. However, for owner stacks we just used the
raw string that came from the server.
This means that the format of the owner stack could include different
formats. Like Spidermonkey format for the client components and V8 for
the server components. This means that this stack can't be parsed
natively by the browser like when printing them as error like in
https://github.com/facebook/react/pull/30289. Additionally, since
there's no source file registered with that name and no source mapping
url, it can't be source mapped.
Before:
<img width="1329" alt="before-firefox"
src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4">
Instead, we need to create a fake stack like we do for the other things.
That way when it's printed as an Error it gets source mapped. It also
means that the format is consistently in the native format of the
current browser.
After:
<img width="753" alt="after-firefox"
src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3">
So this is nice because you can just take the result from
`captureOwnerStack()` and append it to an `Error` stack and print it
natively. E.g. this is what React DevTools will do.
If you want to parse and present it yourself though it's a bit awkward
though. The `captureOwnerStack()` API now includes a bunch of
`rsc://React/` URLs. These don't really have any direct connection to
the source map. Only the browser knows this connection from the eval.
You basically have to strip the prefix and then manually pass the
remainder to your own `findSourceMapURL`.
Another awkward part is that since Safari doesn't support eval sourceURL
exposed into `error.stack` - it means that `captureOwnerStack()` get an
empty location for server components since the fake eval doesn't work
there. That's not a big deal since these stacks are already broken even
for client modules for many because the `eval-source-map` strategy in
Webpack doesn't work in Safari for this same reason.
A lot of this refactoring is just clarifying that there's three kind of
ReactComponentInfo fields:
- `stack` - The raw stack as described on the original server.
- `debugStack` - The Error object containing the stack as represented in
the current client as fake evals.
- `debugTask` - The same thing as `debugStack` but described in terms of
a native `console.createTask`.
Ideally we wouldn't need to filter out React internals and it'd just be
covered by ignore listing by any downstream tool. E.g. a framework using
captureOwnerStack could have its own ignore listing. Printed owner
stacks would get browser source map ignore-listing. React DevTools could
have its own ignore list for internals. However, it's nice to be able to
provide nice owner stacks without a bunch of noise by default.
Especially on the server since they have to be serialized.
We currently call each function that calls into user space and track its
stack frame. However, this needs code for checking each one and doesn't
let us work across bundles.
Instead, we can name each of these frame something predictable by giving
the function a name.
Unfortunately, it's a common practice to rename functions or inline them
in compilers. Even if we didn't, others downstream from us or a dev-mode
minifier could. I use this `.bind()` trick to avoid minifying these
functions and ensure they get a unique name added to them in all
browsers. It's not 100% fool proof since a smart enough compiler could
also discover that the `this` value is not used and strip out the
function and then inline it but nobody does this yet at least.
This lets us find the bottom stack easily from stack traces just by
looking for the name.
React transpiles some of its own `console.error` calls into a helper
that appends component stacks to those calls. However, this doesn't
cover user space `console.error` calls - which includes React helpers
that React has moved into third parties like createClass and prop-types.
The idea is that any user space component can add a warning just like
React can which is why React DevTools adds them too if they don't
already exist. Having them appended in both places is tricky because now
you have to know whether to remove them from React's logs.
Similarly it's often common for server-side frameworks to forget to
cover the `console.error` logs from other sources since React DevTools
isn't active there. However, it's also annoying to get component stacks
clogging the terminal - depending on where the log came from.
In the future `console.createTask()` will cover this use case natively
and when available we don't append them at all.
The new strategy relies on either:
- React DevTools existing to add them to React logs as well as third
parties.
- `console.createTask` being supported and surfaced.
- A third party framework showing the component stack either in an Error
Dialog or appended to terminal output.
For a third party to be able to implement this they need to be able to
get the component stack. To get the component stack from within a
`console.error` call you need to use the `React.captureOwnerStack()`
helper which is only available in `enableOwnerStacks` flag. However,
it's possible to polyfill with parent stacks using internals as a stop
gap. There's a question of whether React 19 should just go out with
`enableOwnerStacks` to expose this but regardless I think it's best it
doesn't include component stacks from the runtime for consistency.
In practice it's not really a regression though because typically either
of the other options exists and error dialogs don't implement
`console.error` overrides anyway yet. SSR terminals might miss them but
they'd only have them in DEV warnings to begin with an a subset of React
warnings. Typically those are either going to happen on the client
anyway or replayed.
Our tests are written to assert that component stacks work in various
scenarios all over the place. To ensure that this keeps working I
implement a "polyfill" that is similar to that expected a server
framework might do - in `assertConsoleErrorDev` and `toErrorDev`.
This PR doesn't yet change www or RN since they have their own forks of
consoleWithStackDev for now.
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.
