This refactors key warning to happen inline after we've matched a Fiber.
I didn't want to do that originally because it was riskier. But it turns
out to be straightforward enough.
This lets us use that Fiber as the source of the warning which matters
to DevTools because then DevTools can associate it with the right
component after it mounts.
We can also associate the duplicate key warning with this Fiber. That
way we'll get the callsite with the duplicate key on the stack and can
associate this warning with the child that had the duplicate.
I kept the forked DevTools tests because the warning now is counted on
the Child instead of the Parent (18 behavior).
However, this won't be released in 19.0.0 so I only test this in
whatever the next version is.
Doesn't seem worth it to have a test for just the 19.0.0 behavior.
## Overview
We didn't have any tests that ran in persistent mode with the xplat
feature flags (for either variant).
As a result, invalid test gating like in
https://github.com/facebook/react/pull/29664 were not caught.
This PR adds test flavors for `ReactFeatureFlag-native-fb.js` in both
variants.
This information is available in the regular stack but since that's
hidden behind an expando and our appended stack to logs is not hidden,
it hides the most important frames like the name of the current
component.
This is closer to what happens to the native stack.
We only include stacks if they're within a ReactFiberCallUserSpace call
frame. This should be most that have a current fiber but this is
critical to filtering out most React frames if the regular node_modules
filter doesn't work.
Most React warnings fire during the rendering phase and not inside a
user space function but some do like hooks warnings and setState in
render. This feature is more important if we port this to React DevTools
appending stacks to all logs where it's likely to originate from inside
a component and you want the line within that component to immediately
part of the visible stack.
One thing that kind sucks is that we don't have a reliable way to
exclude React internal stack frames. We filter node_modules but it might
not match. For other cases I try hard to only track the stack frame at
the root of React (e.g. immediately inside createElement) until the
ReactFiberCallUserSpace so we don't need the filtering to work. In this
case it's hard to achieve the same thing though. This is easier in RDT
because we have the start/end line and parsing of stack traces so we can
use that to exclude internals but that's a lot of code/complexity for
shipping within the library.
For example in Safari:
<img width="590" alt="Screenshot 2024-05-31 at 6 15 27 PM"
src="https://github.com/facebook/react/assets/63648/2820c8c0-8a03-42e9-8678-8348f66b051a">
Ideally warnOnUseFormStateInDev and useFormState wouldn't be included
since they're React internals. Before this change, the Counter.js line
also wasn't included though which points to exactly where the error is
within the user code.
(Note Server Components have V8 formatted lines and Client Components
have JSC formatted lines.)
Based on
- #29694
---
If an action in the useActionState queue errors, we shouldn't run any
subsequent actions. The contract of useActionState is that the actions
run in sequence, and that one action can assume that all previous
actions have completed successfully.
For example, in a shopping cart UI, you might dispatch an "Add to cart"
action followed by a "Checkout" action. If the "Add to cart" action
errors, the "Checkout" action should not run.
An implication of this change is that once useActionState falls into an
error state, the only way to recover is to reset the component tree,
i.e. by unmounting and remounting. The way to customize the error
handling behavior is to wrap the action body in a try/catch.
Mini-refactor of useActionState to only wrap the action in a transition
context if the dispatch is called during a transition. Conceptually, the
action starts as soon as the dispatch is called, even if the action is
queued until earlier ones finish.
We will also warn if an async action is dispatched outside of a
transition, since that is almost certainly a mistake. Ideally we would
automatically upgrade these to a transition, but we don't have a great
way to tell if the action is async until after it's already run.
Host Components can exist as four semantic types
1. regular Components (Vanilla obv)
2. singleton Components
2. hoistable components
3. resources
Each of these component types have their own rules related to mounting
and reconciliation however they are not direclty modeled as their own
unique fiber type. This is partly for code size but also because
reconciling the inner type of these components would be in a very hot
path in fiber creation and reconciliation and it's just not practical to
do this logic check here.
Right now we have three Fiber types used to implement these 4 concepts
but we probably need to reconsider the model and think of Host
Components as a single fiber type with an inner implementation. Once we
do this we can regularize things like transitioning between a resource
and a regular component or a singleton and a hoistable instance. The
cases where these transitions happen today aren't particularly common
but they can be observed and currently the handling of these transitions
is incomplete at best and buggy at worst. The most egregious case is the
link type. This can be a regular component (stylesheet without
precedence) a hoistable component (non stylesheet link tags) or a
resource (stylesheet with a precedence) and if you have a single jsx
slot that tries to reconcile transitions between these types it just
doesn't work well.
This commit adds an error for when a Hoistable goes from Instance to
Resource. Currently this is only possible for `<link>` elements going to
and from stylesheets with precedence. Hopefully we'll be able to remove
this error and implement as an inner type before we encounter new
categories for the Hoistable types
detecting type shifting to and from regular components is harder to do
efficiently because we don't want to reevaluate the type on every update
for host components which is currently not required and would add
overhead to a very hot path
singletons can't really type shift in their one practical implementation
(DOM) so they are only a problem in theroy not practice
When a component suspends with `use`, we switch to the "re-render"
dispatcher during the subsequent render attempt, so that we can reuse
the work from the initial attempt. However, once we run out of hooks
from the previous attempt, we should switch back to the regular "update"
dispatcher.
This is conceptually the same fix as the one introduced in
https://github.com/facebook/react/pull/26232. That fix only accounted
for initial mount, but the useTransition regression test added in
f82973302b3f490ec120c3b102e8c3792452dfc9 illustrates that we need to
handle updates, too.
The issue affects more than just useTransition but because most of the
behavior between the "re-render" and "update" dispatchers is the same
it's hard to contrive other scenarios in a test, which is probably why
it took so long for someone to notice.
Closes#28923 and #29209
---------
Co-authored-by: eps1lon <sebastian.silbermann@vercel.com>
Fixes the behavior of actions that are queued by useActionState to use
the action function that was current at the time it was dispatched, not
at the time it eventually executes.
The conceptual model is that the action is immediately dispatched, as if
it were sent to a remote server/worker. It's the remote worker that
maintains the queue, not the client.
This is another property of actions makes them more like event handlers
than like reducers.
Stacked on #29206 and #29221.
This disables appending owner stacks to console when
`console.createTask` is available in the environment. Instead we rely on
native "async" stacks that end up looking like this with source maps and
ignore list enabled.
<img width="673" alt="Screenshot 2024-05-22 at 4 00 27 PM"
src="https://github.com/facebook/react/assets/63648/5313ed53-b298-4386-8f76-8eb85bdfbbc7">
Unfortunately Chrome requires a string name for each async stack and,
worse, a suffix of `(async)` is automatically added which is very
confusing since it seems like it might be an async component or
something which it is not.
In this case it's not so bad because it's nice to refer to the host
component which otherwise doesn't have a stack frame since it's
internal. However, if there were more owners here there would also be a
`<Counter> (async)` which ends up being kind of duplicative.
If the Chrome DevTools is not open from the start of the app, then
`console.createTask` is disabled and so you lose the stack for those
errors (or those parents if the devtools is opened later). Unlike our
appended ones that are always added. That's unfortunate and likely to be
a bit of a DX issue but it's also nice that it saves on perf in DEV mode
for those cases. Framework dialogs can still surface the stack since we
also track it in user space in parallel.
This currently doesn't track Server Components yet. We need a more
clever hack for that part in a follow up.
I think I probably need to also add something to React DevTools to
disable its stacks for this case too. Since it looks for stacks in the
console.error and adds a stack otherwise. Since we don't add them
anymore from the runtime, the DevTools adds them instead.
Stacked on #29044.
To work with `console.createTask(...).run(...)` we need to be able to
run a function in the scope of the task.
The main concern with this, other than general performance, is that it
might add more stack frames on very deep stacks that hit the stack
limit. Such as with the commit phase where we recursively go down the
tree. These callbacks aren't really necessary in the recursive part but
only in the shallow invocation of the commit phase for each tag. So we
could refactor the commit phase so that only the shallow part at each
level is covered this way.
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.
Note: Despite the similar-sounding description, this fix is unrelated to
the issue where updates that occur after an `await` in an async action
must also be wrapped in their own `startTransition`, due to the absence
of an AsyncContext mechanism in browsers today.
---
Discovered a flaw in the current implementation of the isomorphic
startTransition implementation (React.startTransition), related to async
actions. It only creates an async scope if something calls setState
within the synchronous part of the action (i.e. before the first
`await`). I had thought this was fine because if there's no update
during this part, then there's nothing that needs to be entangled. I
didn't think this through, though — if there are multiple async updates
interleaved throughout the rest of the action, we need the async scope
to have already been created so that _those_ are batched together.
An even easier way to observe this is to schedule an optimistic update
after an `await` — the optimistic update should not be reverted until
the async action is complete.
To implement, during the reconciler's module initialization, we compose
its startTransition implementation with any previous reconciler's
startTransition that was already initialized. Then, the isomorphic
startTransition is the composition of every
reconciler's startTransition.
```js
function startTransition(fn) {
return startTransitionDOM(() => {
return startTransitionART(() => {
return startTransitionThreeFiber(() => {
// and so on...
return fn();
});
});
});
}
```
This is basically how flushSync is implemented, too.
Previously Suspensey recursion would only trigger if the
ShouldSuspendCommit flag was true. However there is a dependence on the
Visibility flag embedded in this logic because these flags share a bit.
To make it clear that the semantics of Suspensey resources require
considering both flags I've added it to the condition even though this
extra or-ing is a noop when the bit is shared
This is necessary to simplify the component stack handling to make way
for owner stacks. It also solves some hacks that we used to have but
don't quite make sense. It also solves the problem where things like key
warnings get silenced in RSC because they get deduped. It also surfaces
areas where we were missing key warnings to begin with.
Almost every type of warning is issued from the renderer. React Elements
are really not anything special themselves. They're just lazily invoked
functions and its really the renderer that determines there semantics.
We have three types of warnings that previously fired in
JSX/createElement:
- Fragment props validation.
- Type validation.
- Key warning.
It's nice to be able to do some validation in the JSX/createElement
because it has a more specific stack frame at the callsite. However,
that's the case for every type of component and validation. That's the
whole point of enableOwnerStacks. It's also not sufficient to do it in
JSX/createElement so we also have validation in the renderers too. So
this validation is really just an eager validation but also happens
again later.
The problem with these is that we don't really know what types are valid
until we get to the renderer. Additionally, by placing it in the
isomorphic code it becomes harder to do deduping of warnings in a way
that makes sense for that renderer. It also means we can't reuse logic
for managing stacks etc.
Fragment props validation really should just be part of the renderer
like any other component type. This also matters once we add Fragment
refs and other fragment features. So I moved this into Fiber. However,
since some Fragments don't have Fibers, I do the validation in
ChildFiber instead of beginWork where it would normally happen.
For `type` validation we already do validation when rendering. By
leaving it to the renderer we don't have to hard code an extra list.
This list also varies by context. E.g. class components aren't allowed
in RSC but client references are but we don't have an isomorphic way to
identify client references because they're defined by the host config so
the current logic is flawed anyway. I kept the early validation for now
without the `enableOwnerStacks` since it does provide a nicer stack
frame but with that flag on it'll be handled with nice stacks anyway. I
normalized some of the errors to ensure tests pass.
For `key` validation it's the same principle. The mechanism for the
heuristic is still the same - if it passes statically through a parent
JSX/createElement call then it's considered validated. We already did
print the error later from the renderer so this also disables the early
log in the `enableOwnerStacks` flag.
I also added logging to Fizz so that key warnings can print in SSR logs.
Flight is a bit more complex. For elements that end up on the client we
just pass the `validated` flag along to the client and let the client
renderer print the error once rendered. For server components we log the
error from Flight with the server component as the owner on the stack
which will allow us to print the right stack for context. The factoring
of this is a little tricky because we only want to warn if it's in an
array parent but we want to log the error later to get the right debug
info.
Fiber/Fizz has a similar factoring problem that causes us to create a
fake Fiber for the owner which means the logs won't be associated with
the right place in DevTools.
This lets us expose the component stack to the error reporting that
happens here as `console.error` patching. Now if you just call
`console.error` in the error handlers it'll get the component stack
added to the end by React DevTools.
However, unfortunately this happens a little too late so the Fiber will
be disconnected with its `.return` pointer set to null already. So it'll
be too late to extract a parent component stack from but you can at
least get the stack from source to error boundary. To work around this I
manually add the parent component stack in our default handlers when
owner stacks are off. We could potentially fix this but you can also
just include it yourself if you're calling `console.error` and it's not
a problem for owner stacks.
This is not a problem for owner stacks because we'll still have those
and so for those just calling `console.error` just works. However, the
main feature is that by letting React add them, we can switch to using
native error stacks when available.
We previously had two slightly different concepts for "current fiber".
There's the "owner" which is set inside of class components in prod if
string refs are enabled, and sometimes inside function components in DEV
but not other contexts.
Then we have the "current fiber" which is only set in DEV for various
warnings but is enabled in a bunch of contexts.
This unifies them into a single "current fiber".
The concept of string refs shouldn't really exist so this should really
be a DEV only concept. In the meantime, this sets the current fiber
inside class render only in prod, however, in DEV it's now enabled in
more contexts which can affect the string refs. That was already the
case that a string ref in a Function component was only connecting to
the owner in prod. Any string ref associated with any non-class won't
work regardless so that's not an issue. The practical change here is
that an element with a string ref created inside a life-cycle associated
with a class will work in DEV but not in prod. Since we need the current
fiber to be available in more contexts in DEV for the debugging
purposes. That wouldn't affect any old code since it would have a broken
ref anyway. New code shouldn't use string refs anyway.
The other implication is that "owner" doesn't necessarily mean
"rendering" since we need the "owner" to track other debug information
like stacks - in other contexts like useEffect, life cycles, etc.
Internally we have a separate `isRendering` flag that actually means
we're rendering but even that is a very overloaded concept. So anything
that uses "owner" to imply rendering might be wrong with this change.
This is a first step to a larger refactor for tracking current rendering
information.
---------
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
Updates Suspensey instances and resources to preload even during urgent
updates and to potentially suspend.
The current implementation is unchanged for transitions but for sync
updates if there is a suspense boundary above the resource/instance it
will be rendered in fallback mode instead.
Note: This behavior is not what we want for images once we make them
suspense enabled. We will need to have forked behavior here to
distinguish between stylesheets which should never commit when not
loaded and images which should commit after a small delay
Before this change, `useFormStatus` is only activated if a form is
submitted by an action function (either `<form action={actionFn}>` or
`<button formAction={actionFn}>`).
After this change, `useFormStatus` will also be activated if you call
`startTransition(actionFn)` inside a submit event handler that is
`preventDefault`-ed.
This is the last missing piece for implementing a custom `action` prop
that is progressively enhanced using `onSubmit` while maintaining the
same behavior as built-in form actions.
Here's the basic recipe for implementing a progressively-enhanced form
action. This would typically be implemented in your UI component
library, not regular application code:
```js
import {requestFormReset} from 'react-dom';
// To implement progressive enhancement, pass both a form action *and* a
// submit event handler. The action is used for submissions that happen
// before hydration, and the submit handler is used for submissions that
// happen after.
<form
action={action}
onSubmit={(event) => {
// After hydration, we upgrade the form with additional client-
// only behavior.
event.preventDefault();
// Manually dispatch the action.
startTransition(async () => {
// (Optional) Reset any uncontrolled inputs once the action is
// complete, like built-in form actions do.
requestFormReset(event.target);
// ...Do extra action-y stuff in here, like setting a custom
// optimistic state...
// Call the user-provided action
const formData = new FormData(event.target);
await action(formData);
});
}}
/>
```
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>
It turns out we already made refs writable in #25696, which has been in
canary for over a year. The approach in that PR also has the benefit of
being slightly more perf sensitive because it still uses a shared object
until the fiber is mounted. So let's just go back to that.
Add new reconciler methods since last breaking change to the README
based on usage and comments.
---------
Co-authored-by: Josh Story <josh.c.story@gmail.com>
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.
We have changed the shape (and the runtime) of React Elements. To help
avoid precompiled or inlined JSX having subtle breakages or deopting
hidden classes, I renamed the symbol so that we can early error if
private implementation details are used or mismatching versions are
used.
Why "transitional"? Well, because this is not the last time we'll change
the shape. This is just a stepping stone to removing the `ref` field on
the elements in the next version so we'll likely have to do it again.
This doesn't change production behavior. We always render Iterables to
our best effort in prod even if they're Iterators.
But this does change the DEV warnings which indicates which are valid
patterns to use.
It's a footgun to use an Iterator as a prop when you pass between
components because if an intermediate component rerenders without its
parent, React won't be able to iterate it again to reconcile and any
mappers won't be able to re-apply. This is actually typically not a
problem when passed only to React host components but as a pattern it's
a problem for composability.
We used to warn only for Generators - i.e. Iterators returned from
Generator functions. This adds a warning for Iterators created by other
means too (e.g. Flight or the native Iterator utils). The heuristic is
to check whether the Iterator is the same as the Iterable because that
means it's not possible to get new iterators out of it. This case used
to just yield non-sense like empty sets in DEV but not in prod.
However, a new realization is that when the Component itself is a
Generator Function, it's not actually a problem. That's because the
React Element itself works as an Iterable since we can ask for new
generators by calling the function again. So this adds a special case to
allow the Generator returned from a Generator Function's direct child.
The principle is “don’t pass iterators around” but in this case there is
no iterator floating around because it’s between React and the JS VM.
Also see #28849 for context on AsyncIterables.
Related to this, but Hooks should ideally be banned in these for the
same reason they're banned in Async Functions.
This disables symbol renaming in production builds. The original
variable and function names are preserved. All other forms of
compression applied by Closure (dead code elimination, inlining, etc)
are unchanged — the final program is identical to what we were producing
before, just in a more readable form.
The motivation is to make it easier to debug React issues that only
occur in production — the same reason we decided to start shipping
sourcemaps in #28827 and #28827.
However, because most apps run their own minification step on their npm
dependencies, it's not necessary for us to minify the symbols before
publishing — it'll be handled the app, if desired.
This is the same strategy Meta has used to ship React for years. The
React build itself has unminified symbols, but they get minified as part
of Meta's regular build pipeline.
Even if an app does not minify their npm dependencies, gzip covers most
of the cost of symbol renaming anyway.
This saves us from having to ship sourcemaps, which means even apps that
don't have sourcemaps configured will be able to debug the React build
as easily as they would any other npm dependency.
Adds an experimental feature flag to the implementation of useMemoCache,
the internal cache used by the React Compiler (Forget).
When enabled, instead of treating the cache as copy-on-write, like we do
with fibers, we share the same cache instance across all render
attempts, even if the component is interrupted before it commits.
If an update is interrupted, either because it suspended or because of
another update, we can reuse the memoized computations from the previous
attempt. We can do this because the React Compiler performs atomic
writes to the memo cache, i.e. it will not record the inputs to a
memoization without also recording its output.
This gives us a form of "resuming" within components and hooks.
This only works when updating a component that already mounted. It has
no impact during initial render, because the memo cache is stored on the
fiber, and since we have not implemented resuming for fibers, it's
always a fresh memo cache, anyway.
However, this alone is pretty useful — it happens whenever you update
the UI with fresh data after a mutation/action, which is extremely
common in a Suspense-driven (e.g. RSC or Relay) app.
So the impact of this feature is faster data mutations/actions (when the
React Compiler is used).
Previously, the `refs` property of a class component instance was
read-only by user code — only React could write to it, and until/unless
a string ref was used, it pointed to a shared empty object that was
frozen in dev to prevent userspace mutations.
Because string refs are deprecated, we want users to be able to codemod
all their string refs to callback refs. The safest way to do this is to
output a callback ref that assigns to `this.refs`.
So to support this, we need to make `this.refs` writable by userspace.
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
## How did you test this change?
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
This wasn't clearly articulated and tested why the code structure is
like this but I think the logic is correct - or at least consistent with
the weird semantics.
We place this top-level fragment check inside the recursion so that you
can resolve how many every Lazy or Usable wrappers you want and it still
preserves the same semantics if they weren't there (which they might not
be as a matter of a race condition).
However, we don't actually recurse with the top-level fragment
unwrapping itself because nesting a bunch of keyless fragments isn't the
same as a single fragment/element.
Based on:
- #28808
- #28804
---
This adds a React DOM method called requestFormReset that schedules a
form reset to occur when the current transition completes.
Internally, it's the same method that's called automatically whenever a
form action is submitted. It only affects uncontrolled form inputs. See
https://github.com/facebook/react/pull/28804 for details.
The reason for the public API is so UI libraries can implement their own
action-based APIs and maintain the form-resetting behavior, something
like this:
```js
function onSubmit(event) {
// Disable default form submission behavior
event.preventDefault();
const form = event.target;
startTransition(async () => {
// Request the form to reset once the action
// has completed
requestFormReset(form);
// Call the user-provided action prop
await action(new FormData(form));
})
}
```
Based on:
- #28804
---
This sets adds a new ReactDOM export called requestFormReset, including
setting up the export and creating a method on the internal ReactDOM
dispatcher. It does not yet add any implementation.
Doing this in its own commit for review purposes.
The API itself will be explained in the next PR.
This updates the behavior of form actions to automatically reset the
form's uncontrolled inputs after the action finishes.
This is a frequent feature request for people using actions and it
aligns the behavior of client-side form submissions more closely with
MPA form submissions.
It has no impact on controlled form inputs. It's the same as if you
called `form.reset()` manually, except React handles the timing of when
the reset happens, which is tricky/impossible to get exactly right in
userspace.
The reset shouldn't happen until the UI has updated with the result of
the action. So, resetting inside the action is too early.
Resetting in `useEffect` is better, but it's later than ideal because
any effects that run before it will observe the state of the form before
it's been reset.
It needs to happen in the mutation phase of the transition. More
specifically, after all the DOM mutations caused by the transition have
been applied. That way the `defaultValue` of the inputs are updated
before the values are reset. The idea is that the `defaultValue`
represents the current, canonical value sent by the server.
Note: this change has no effect on form submissions that aren't
triggered by an action.
These test don't `assertLog` or `waitFor` so we don't need to
`Scheduler.log`. Ideally we would, but since they're fuzzers it's a bit
difficult to know what the expected log is from the helper.
Since this doesn't regress current test behavior, we can improve them
after this to unblock https://github.com/facebook/react/pull/28737
Fixes a bug that happens when an error occurs during hydration, React
switches to client rendering, and then the client render suspends. It
works correctly if there's a Suspense boundary on the stack, but not if
it happens in the shell of the app.
Prior to this fix, the app would crash with an "Unknown root exit
status" error.
I left a TODO comment for how we might refactor this code to be less
confusing in the future.
Follow up to #28783 and #28786.
Since we've changed the implementations of these we can rename them to
something a bit more descriptive while we're at it, since anyone
depending on them will need to upgrade their code anyway.
"react" with no condition:
`__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
"react" with "react-server" condition:
`__SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
"react-dom":
`__DOM_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE`
Sebastian MarkbågeandGitHub
RickyandGitHub
Andrew ClarkandGitHub
Josh StoryandGitHub
Sophie AlpertandGitHub
Jan KassensandGitHub
zhangenmingandGitHub
