Commit Graph

199 Commits

Author SHA1 Message Date
Sebastian Markbåge
1bb056363c [Fizz] Use RequestInstance constructor for resuming (#30947)
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.
2024-09-11 11:35:01 -04:00
Sebastian Markbåge
96aca5f4f3 Spawn new task if we hit stack overflow (#30419)
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.
2024-08-27 13:10:37 -04:00
Josh Story
ab24f643d0 [Fizz] use microtasks rather than tasks when scheduling work while prerendering (#30770)
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.
2024-08-21 07:55:56 -07:00
Josh Story
85180b8cf8 [Fizz][Static] when aborting a prerender halt unfinished boundaries instead of erroring (#30732)
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.
2024-08-20 13:30:51 -07:00
Josh Story
2505bf9b34 [Fizz] track postpones when aborting boundaries with a postpone (#30751)
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.
2024-08-20 09:49:41 -07:00
Josh Story
7954db9398 [Fizz] handle throwing after abort during render (#30730)
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
2024-08-16 18:29:18 -07:00
Josh Story
a7d1240c96 [Fizz] Update postpone abort semantics when prerendering (#30541)
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.
2024-07-31 08:33:43 -07:00
Josh Story
a451de014c [Fizz] Allow aborting during render (#30488)
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
2024-07-29 13:18:15 -07:00
Josh Story
9938e248fe [Fizz] Don't perform work when closing (#30497)
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.
2024-07-29 11:09:54 -07:00
Josh Story
d17e9d1ce5 [Fizz] Prerender fallbacks before children (#30483)
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.
2024-07-26 14:06:47 -07:00
Sebastian Markbåge
e8df0cf9f7 Switch to binding the console with badging instead of calling it directly (#30461)
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.
2024-07-25 12:32:16 -04:00
Jan Kassens
b7e7f1a3fa [BE] upgrade prettier to 3.3.3 (#30420)
Mostly just changes in ternary formatting.
2024-07-22 16:09:01 -04:00
Sebastian Markbåge
b15c1983dc [Flight] Normalize Stack Using Fake Evals (#30401)
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`.
2024-07-22 11:03:15 -04:00
Sebastian Markbåge
792f192114 Tag all user space call sites with the "react-stack-bottom-frame" name (#30369)
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.
2024-07-22 10:47:38 -04:00
Jan Kassens
af28f480e8 Feature flag to disable legacy context for function components (#30319)
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.
2024-07-11 16:21:12 -04:00
Jan Kassens
8e00cf04de easy: add link to legacy context warning (#30315)
Missed this when adding the link to other places of the same warning in
378b305958
2024-07-11 10:42:37 -04:00
Jan Kassens
378b305958 Warn about legacy context when legacy context is not disabled (#30297)
For environments that still have legacy contexts available, this adds a
warning to make the remaining call sites easier to locate and encourage
upgrades.
2024-07-10 11:53:00 -04:00
Sebastian Markbåge
b73dcdc04f [Fizz] Refactor Component Stack Nodes (#30298)
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.
2024-07-09 15:44:01 -04:00
Sebastian Markbåge
df783f9ea1 Add unknown location information to component stacks (#30290)
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.
2024-07-08 11:54:14 -04:00
Sebastian Markbåge
3db98c9177 [Fizz] Track Current debugTask and run it for onError Callbacks (#30182)
Stacked on #30174.

This tracks the current debugTask on the Task so that when an error is
thrown we can use it to run the `onError` (and `onShellError` and
`onFatalError`) callbacks within the Context of that task. Ideally it
would be associated with the error object but neither console.error [nor
reportError](https://crbug.com/350426235) reports this as the async
stack so we have to manually restore it.

That way when you inspect Fizz using node `--inspect` we show the right
async stack.

<img width="616" alt="Screenshot 2024-07-01 at 10 52 29 PM"
src="https://github.com/facebook/react/assets/63648/db68133e-124e-4509-8241-c67160db94fc">

This is equivalent to how we track the task that created the parent
server component or the Fiber:


6d2a97a711/packages/react-reconciler/src/ReactChildFiber.js (L1985)

Then use them when invoking the error callbacks:


6d2a97a711/packages/react-reconciler/src/ReactFiberThrow.js (L104-L108)

---------

Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
2024-07-02 19:46:18 -04:00
Sebastian Markbåge
cfb8945f51 [Fizz] Implement debugInfo (#30174)
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.
2024-07-02 18:26:32 -04:00
Sebastian Markbåge
e60063d9e7 [Fizz] Include a component stack in prod but only lazily generate it (#30132)
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.
2024-07-02 16:05:20 -04:00
Sebastian Markbåge
6d2a97a711 [Fizz] Gate legacyContext field on disableLegacyContext (#30173)
We're running out of fields and this one we can avoid at runtime in any
modern builds.
2024-07-01 15:30:00 -04:00
Sebastian Markbåge
315109b02b [Fizz] Enable owner stacks for SSR (#30152)
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.
2024-07-01 10:27:52 -04:00
Sebastian Markbåge
e6783e7cc9 [Fizz] Run console.createTask during SSR when available (#30142)
Same as #30140 but for Fizz.

This is rarely used but it does allow seeing component stacks when
inspecting the Node.js server running Fizz using `--inspect` and the
Chrome DevTools.

<img width="504" alt="Screenshot 2024-06-29 at 4 08 22 PM"
src="https://github.com/facebook/react/assets/63648/f89bdf89-2598-42b4-8623-3b87f03326c4">
2024-07-01 09:50:45 -04:00
Sebastian Markbåge
349a99a7a3 Badge Environment Name on Thrown Errors from the Server (#29846)
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.
2024-06-26 13:27:26 -04:00
Sebastian Markbåge
01a40570c3 [Flight/Fizz] Use Constructors for Large Request/Response Objects in Flight/Fizz (#29858)
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.
2024-06-11 15:55:29 -04:00
Sebastian Markbåge
d6cfa0f295 [Fiber] Use Owner/JSX Stack When Appending Stacks to Console (#29206)
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.
2024-05-25 11:58:17 -04:00
Sebastian Markbåge
84239da896 Move createElement/JSX Warnings into the Renderer (#29088)
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.
2024-05-23 12:48:57 -04:00
Josh Story
94eed63c49 (Land #28798) Move Current Owner (and Cache) to an Async Dispatcher (#28912)
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>
2024-04-25 10:40:40 -07:00
Sebastian Markbåge
9f2eebd807 [Fiber/Fizz] Support AsyncIterable as Children and AsyncGenerator Client Components (#28868)
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.
2024-04-22 13:25:05 -04:00
Sebastian Markbåge
368202181e Warn for Child Iterator of all types but allow Generator Components (#28853)
This doesn't change production behavior. We always render Iterables to
our best effort in prod even if they're Iterators.

But this does change the DEV warnings which indicates which are valid
patterns to use.

It's a footgun to use an Iterator as a prop when you pass between
components because if an intermediate component rerenders without its
parent, React won't be able to iterate it again to reconcile and any
mappers won't be able to re-apply. This is actually typically not a
problem when passed only to React host components but as a pattern it's
a problem for composability.

We used to warn only for Generators - i.e. Iterators returned from
Generator functions. This adds a warning for Iterators created by other
means too (e.g. Flight or the native Iterator utils). The heuristic is
to check whether the Iterator is the same as the Iterable because that
means it's not possible to get new iterators out of it. This case used
to just yield non-sense like empty sets in DEV but not in prod.

However, a new realization is that when the Component itself is a
Generator Function, it's not actually a problem. That's because the
React Element itself works as an Iterable since we can ask for new
generators by calling the function again. So this adds a special case to
allow the Generator returned from a Generator Function's direct child.
The principle is “don’t pass iterators around” but in this case there is
no iterator floating around because it’s between React and the JS VM.

Also see #28849 for context on AsyncIterables.

Related to this, but Hooks should ideally be banned in these for the
same reason they're banned in Async Functions.
2024-04-21 12:51:45 -04:00
Josh Story
c8a035036d [Fizz] hoistables should never flush before the preamble (#28802)
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.
2024-04-11 15:13:04 -07:00
Sebastian Markbåge
d50323eb84 Flatten ReactSharedInternals (#28783)
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.
2024-04-08 19:23:23 -04:00
Andrew Clark
bfd8da807c Make class prop resolution faster (#28766)
`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.
2024-04-05 13:06:39 -04:00
Andrew Clark
48b4ecc901 Remove defaultProps support (except for classes) (#28733)
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.
2024-04-04 10:59:19 -04:00
Sebastian Markbåge
583eb6770d Emit Server Error Prefix in the .stack Property Too (#28738)
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.
2024-04-03 21:52:54 -04:00
Andrew Clark
3761acb42b Classes consume ref prop during SSR, too (#28731)
Same as #28719 but for SSR.
2024-04-03 12:55:57 -04:00
Jan Kassens
7a2609eedc Cleanup enableBigIntSupport flag (#28711)
Cleanup enableBigIntSupport flag
2024-04-03 09:25:02 -04:00
Josh Story
5998a77519 Reland #28672: Remove IndeterminateComponent (#28681)
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.
2024-04-02 17:42:43 -07:00
Sebastian Markbåge
b9149cc6e6 Include regular stack trace in serialized errors from Fizz (#28684)
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.
2024-03-30 11:08:54 -04:00
Jan Kassens
a73c3450e1 Remove module pattern function component support (flag only) (#28671)
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.
2024-03-29 11:16:17 -04:00
Ricky
f269074723 Revert "Remove module pattern function component support" (#28670)
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.
2024-03-29 10:10:11 -04:00
Josh Story
cc56bed38c Remove module pattern function component support (#27742)
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.
2024-03-28 13:08:08 -07:00
Ricky
05797ccebd s/form state/action state (#28631)
Rename internals from "form state" to "action state"
2024-03-28 11:34:24 -04:00
Ricky
dbfbfb3312 Update error messages (#28652)
## 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.
2024-03-26 17:25:53 -04:00
Jan Kassens
208ceeb46c Cleanup enableFloat flag (#28613)
Cleanup enableFloat flag
2024-03-22 12:22:30 -04:00
Josh Story
113ab9af08 [Flight][Fizz][Fiber] Chain HostDispatcher implementations (#28488)
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.
2024-03-04 12:27:15 -08:00
Sebastian Silbermann
2f240c91ed Add support for rendering BigInt (#24580) 2024-02-26 19:18:50 +01:00
Sebastian Markbåge
d579e77482 Remove method name prefix from warnings and errors (#28432)
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.
2024-02-23 15:16:54 -05:00