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.
Treat async (boolean prop) consistently with Float. Previously float
checked if `props.async === true` (or not true) but the rest of
react-dom considers anything truthy that isn't a function or symbol as
`true`. This PR normalizes the Float behavior.
Stacked on #28751
Historically explicit hydration scheduling used the reconciler's update
priority to schedule the hydration. There was a lingering todo to switch
to using event priority in the absence of an explicit update priority.
This change updates the hydration priority by referring to the event
priority if no update priority is set
Stacked on #28771
ReactDOMCurrentDispatcher has longer property names for various methods.
These methods are only ever called internally and don't need to be
represented with as many characters. This change shortens the names and
aligns them with the hint codes we use in Flight. This alignment is
passive since not all dispatcher methods will exist as flight
instructions but where they can line up it seems reasonable to make them
do so
Stacked on #28751
ReactDOMSharedInternals uses properties of considerable length to model
mutuable state. These properties are not mangled during minification and
contribute a not insigificant amount to the uncompressed bundle size and
to a lesser degree compressed bundle size.
This change rewrites the DOMInternals in a way that shortens property
names so we can have smaller builds.
It also treats the entire object as a mutable container rather than
having different mutable sub objects.
The same treatment should be given to ReactSharedInternals
This PR moves `flushSync` out of the reconciler. there is still an
internal implementation that is used when these semantics are needed for
React methods such as `unmount` on roots.
This new isomorphic `flushSync` is only used in builds that no longer
support legacy mode.
Additionally all the internal uses of flushSync in the reconciler have
been replaced with more direct methods. There is a new
`updateContainerSync` method which updates a container but forces it to
the Sync lane and flushes passive effects if necessary. This combined
with flushSyncWork can be used to replace flushSync for all instances of
internal usage.
We still maintain the original flushSync implementation as
`flushSyncFromReconciler` because it will be used as the flushSync
implementation for FB builds. This is because it has special legacy mode
handling that the new isomorphic implementation does not need to
consider. It will be removed from production OSS builds by closure
though
Currently updatePriority is tracked in the reconciler. `flushSync` is
going to be implemented reconciler agnostic soon and we need to move the
tracking of this state to the renderer and out of reconciler. This
change implements new renderer bin dings for getCurrentUpdatePriority
and setCurrentUpdatePriority.
I was originally going to have the getter also do the event priority
defaulting using window.event so we eliminate getCur rentEventPriority
but this makes all the callsites where we store the true current
updatePriority on the stack harder to work with so for now they remain
separate.
I also moved runWithPriority to the renderer since it really belongs
whereever the state is being managed and it is only currently exposed in
the DOM renderer.
Additionally the current update priority is not stored on
ReactDOMSharedInternals. While not particularly meaningful in this
change it opens the door to implementing `flushSync` outside of the
reconciler
In React DOM, in general, we don't differentiate between `null` and
`undefined` because we expect to target DOM APIs. When we're setting a
property on a Custom Element, in the new heuristic, the goal is to allow
passing whatever data type instead of normalizing it. Switching between
`undefined` and `null` as an explicit value should therefore be
respected.
However, in this mode if `undefined` is used for the initial value, we
don't actually set the property at all. If passing `null` we will now
initialize it to the value `null`. Meaning `undefined` kind of
represents the default.
### Removing Properties
There is a pretty complex edge case which is what should happen when a
prop used to exist but was removed from the props object. This doesn't
have any kind of defined semantics. It really should mean - return to
"default". Because in the declarative world it means the same as if it
was just created - i.e. we can't just leave it as it was.
The closest might be `delete object.property` but that's not really the
intended way that properties on custom elements / classes are supposed
to operate. Additionally, for a property to even hit our heuristic it
must pass the `in` test and must exist to being with so the default must
have a value.
Since the point of these properties is to contain any kind of type,
there isn't really a conceptual default value. E.g. a numeric default
value might be zero `0` while a default string might be empty `""` and
default object might `null`. Additionally, the conceptual default can
really be initialized to anything. There's also varied precedence in the
ecosystem here and really no consensus. Anything we pick would be kind
of wrong, so we used to just pick `null`.
_The safest way to consume a Custom Element is to always pass the same
set of props._
JS does have a concept of a "default value" though and that is described
as the value `undefined`. That's why default argument / object property
initializers are initialized if the value is `undefined`.
The problem with using `undefined` as value is that [you shouldn't
actually ever set the value of a class property to
`undefined`](https://twitter.com/sebmarkbage/status/1774082540296388752).
A property should always be initialized to some value. It can't be left
missing and shouldn't be initialized to the value `undefined` for hidden
class optimizations. If we just mutate it to be `undefined` it would be
potentially bad for perf and shouldn't really be the value after
removing property - it should be returned to default.
Every property should really have a setter to be useful since it is what
is used to trigger reactivity when it changes. Sometimes you can just
use the properties passively when something else happens but most of the
time it should be a setter but to reach parity with DOM it should really
be always so that the active value can be normalized.
Those setters can have default argument initializers to represent what
the default value should be. Therefore Custom Element properties should
be used like this:
```js
class CustomElement extends HTMLElement {
_textLabel = '';
_price = 0;
_items = null;
constructor() {
super();
}
set textLabel(value = '') {
this._textLabel = value;
}
get textLabel() {
return this._textLabel;
}
set price(value = 0) {
this._price = value;
}
get price() {
return this._price;
}
set items(value = null) {
this._items = value;
}
get items() {
return this._items;
}
}
```
The default initializer can be used to initialize a value back to its
original default when `undefined` is passed to it. Therefore, we pass
`undefined`, not because we expect that to be the value of a property
but because that's the value that represents "return to default".
This fixes#28203 but not really for the reason specified in the issue.
We don't expect you to actually store the `undefined` value but to use a
setter to set the property to something else that represents the
default. When we initialize the element the first time, we won't set
anything if it's the value `undefined` so we assume that the property
initializers running in the constructor is going to set the same default
value as if we set the property to `undefined`.
cc @josepharhar
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.
We've rolled out this flag internally on WWW. This PR removed flag
`enableCustomElementPropertySupport`
Test plan:
-- `yarn test`
Co-authored-by: Ricky Hanlon <rickhanlonii@gmail.com>
## Overview
This has landed, so we can remove the flag
## Changelog
This change blocks using javascript URLs such as:
```html
<a href="javascript:notfine">p0wned</a>
```
We previously announced dropping support for this via a warning:
> A future version of React will block javascript: URLs as a security
precaution. Use event handlers instead if you can. If you need to
generate unsafe HTML try using dangerouslySetInnerHTML instead.
Stacked on top of #28498 for test fixes.
### Don't Rethrow
When we started React it was 1:1 setState calls a series of renders and
if they error, it errors where the setState was called. Simple. However,
then batching came and the error actually got thrown somewhere else.
With concurrent mode, it's not even possible to get setState itself to
throw anymore.
In fact, all APIs that can rethrow out of React are executed either at
the root of the scheduler or inside a DOM event handler.
If you throw inside a React.startTransition callback that's sync, then
that will bubble out of the startTransition but if you throw inside an
async callback or a useTransition we now need to handle it at the hook
site. So in 19 we need to make all React.startTransition swallow the
error (and report them to reportError).
The only one remaining that can throw is flushSync but it doesn't really
make sense for it to throw at the callsite neither because batching.
Just because something rendered in this flush doesn't mean it was
rendered due to what was just scheduled and doesn't mean that it should
abort any of the remaining code afterwards. setState is fire and forget.
It's send an instruction elsewhere, it's not part of the current
imperative code.
Error boundaries never rethrow. Since you should really always have
error boundaries, most of the time, it wouldn't rethrow anyway.
Rethrowing also actually currently drops errors on the floor since we
can only rethrow the first error, so to avoid that we'd need to call
reportError anyway. This happens in RN events.
The other issue with rethrowing is that it logs an extra console.error.
Since we're not sure that user code will actually log it anywhere we
still log it too just like we do with errors inside error boundaries
which leads all of these to log twice.
The goal of this PR is to never rethrow out of React instead, errors
outside of error boundaries get logged to reportError. Event system
errors too.
### Breaking Changes
The main thing this affects is testing where you want to inspect the
errors thrown. To make it easier to port, if you're inside `act` we
track the error into act in an aggregate error and then rethrow it at
the root of `act`. Unlike before though, if you flush synchronously
inside of act it'll still continue until the end of act before
rethrowing.
I expect most user code breakages would be to migrate from `flushSync`
to `act` if you assert on throwing.
However, in the React repo we also have `internalAct` and the
`waitForThrow` helpers. Since these have to use public production
implementations we track these using the global onerror or process
uncaughtException. Unlike regular act, includes both event handler
errors and onRecoverableError by default too. Not just render/commit
errors. So I had to account for that in our tests.
We restore logging an extra log for uncaught errors after the main log
with the component stack in it. We use `console.warn`. This is not yet
ignorable if you preventDefault to the main error event. To avoid
confusion if you don't end up logging the error to console I just added
`An error occurred`.
### Polyfill
All browsers we support really supports `reportError` but not all test
and server environments do, so I implemented a polyfill for browser and
node in `shared/reportGlobalError`. I don't love that this is included
in all builds and gets duplicated into isomorphic even though it's not
actually needed in production. Maybe in the future we can require a
polyfill for this.
### Follow Ups
In a follow up, I'll make caught vs uncaught error handling be
configurable too.
---------
Co-authored-by: Ricky Hanlon <rickhanlonii@gmail.com>
Stacked on #28502.
This builds on the mechanism in #28502 by adding a diff of everything
we've collected so far to the thrown error or logged error.
This isn't actually a longest common subsequence diff. This means that
there are certain cases that can appear confusing such as a node being
added/removed when it really would've appeared later in the list. In
fact once a node mismatches, we abort rendering so we don't have the
context of what would've been rendered. It's not quite right to use the
result of the recovery render because it can use client-only code paths
using useSyncExternalStore which would yield false differences. That's
why diffing the HTML isn't quite right.
I also present abstract components in the stack, these are presented
with the client props and no diff since we don't have the props that
were on the server. The lack of difference might be confusing but it's
useful for context.
The main thing that's data new here is that we're adding some siblings
and props for context.
Examples in the [snapshot
commit](https://github.com/facebook/react/pull/28512/commits/e14532fd8df858a319d3eca687d83227209a498c).
Stacked on #28458.
This doesn't actually really change the messages yet, it's just a
refactor.
Hydration warnings can be presented either as HTML or React JSX format.
If presented as HTML it makes more sense to make that a DOM specific
concept, however, I think it's actually better to present it in terms of
React JSX.
Most of the time the errors aren't going to be something messing with
them at the HTML/HTTP layer. It's because the JS code does something
different. Most of the time you're working in just React. People don't
necessarily even know what the HTML form of it looks like. So this takes
the approach that the warnings are presented in React JSX in their rich
object form.
Therefore, I'm moving the approach to yield diff data to the reconciler
but it's the reconciler that's actually printing all the warnings.
While Meta is still using legacy mode and we can't remove completely,
Meta is not using legacy hydration so we should be able to remove that.
This is just the first step. Once removed, we can vastly simplify the
DOMConfig for hydration.
This will have to be rebased when tests are upgraded.
This bumps the canary versions to v19 to communicate that the next
release will be a major. Once this lands, we can start merging breaking
changes into `main`.
This a follow up to #28564. It's alternative to #28609 which takes
#28610 into account.
It used to be possible to return JSX from an action with
`useActionState`.
```js
async function action(errors, payload) {
"use server";
try {
...
} catch (x) {
return <div>Error message</div>;
}
}
```
```js
const [errors, formAction] = useActionState(action);
return <div>{errors}</div>;
```
Returning JSX from an action is itself not anything problematic. It's
that it also has to return the previous state to the action reducer
again that's the problem. When this happens we accidentally could
serialize an Element back to the server.
I fixed this in #28564 so it's now blocked if you don't have a temporary
reference set.
However, you can't have that for the progressive enhancement case. The
reply is eagerly encoded as part of the SSR render. Typically you
wouldn't have these in the initial state so the common case is that they
show up after the first POST back that yields an error and it's only in
the no-JS case where this happens so it's hard to discover.
As noted in #28609 there's a security implication with allowing elements
to be sent across this kind of payload, so we can't just make it work.
When an error happens during SSR our general policy is to try to recover
on the client instead. After all, SSR is mainly a perf optimization in
React terms and it's not primarily intended for a no JS solution.
This PR takes the approach that if we fail to generate the progressive
enhancement payload. I.e. if the serialization of previous state /
closures throw. Then we fallback to the replaying semantics just client
actions instead which will succeed.
The effect of this is that this pattern mostly just works:
- First render in the typical case doesn't have any JSX in it so it just
renders a progressive enhanced form.
- If JS fails to hydrate or you click early we do a form POST. If that
hits an error and it tries to render it using JSX, then the new page
will render successfully - however this time with a Replaying form
instead.
- If you try to submit the form again it'll have to be using JS.
Meaning if you use JSX as the error return value of form state and you
make a first attempt that fails, then no JS won't work because either
the first or second attempt has to hydrate.
We have ideas for potentially optimizing away serializing unused
arguments like if you don't actually use previous state which would also
solve it but it wouldn't cover all cases such as if it was deeply nested
in complex state.
Another approach that I considered was to poison the prev state if you
passed an element back but let it through to the action but if you try
to render the poisoned value, it wouldn't work. The downside of this is
when to error. Because in the progressive enhancement case it wouldn't
error early but when you actually try to invoke it at which point it
would be too late to fallback to client replaying. It would probably
have to always error even on the client which is unfortunate since this
mostly just works as long as it hydrates.
A while back we implemented a heuristic that if a chunk was large it was
assumed to be produced by the render and thus was safe to stream which
results in transferring the underlying object memory. Later we ran into
an issue where a precomputed chunk grew large enough to trigger this
hueristic and it started causing renders to fail because once a second
render had occurred the precomputed chunk would not have an underlying
buffer of bytes to send and these bytes would be omitted from the
stream. We implemented a technique to detect large precomputed chunks
and we enforced that these always be cloned before writing.
Unfortunately our test coverage was not perfect and there has been for a
very long time now a usage pattern where if you complete a boundary in
one flush and then complete a boundary that has stylehsheet dependencies
in another flush you can get a large precomputed chunk that was not
being cloned to be sent twice causing streaming errors.
I've thought about why we even went with this solution in the first
place and I think it was a mistake. It relies on a dev only check to
catch paired with potentially version specific order of operations on
the streaming side. This is too unreliable. Additionally the low limit
of view size for Edge is not used in Node.js but there is not real
justification for this.
In this change I updated the view size for edge streaming to match Node
at 2048 bytes which is still relatively small and we have no data one
way or another to preference 512 over this. Then I updated the assertion
logic to error anytime a precomputed chunk exceeds the size. This
eliminates the need to clone these chunks by just making sure our view
size is always larger than the largest precomputed chunk we can possibly
write. I'm generally in favor of this for a few reasons.
First, we'll always know during testing whether we've violated the limit
as long as we exercise each stream config because the precomputed chunks
are created in module scope. Second, we can always split up large chunks
so making sure the precomptued chunk is smaller than whatever view size
we actually desire is relatively trivial.
## Overview
Adds a `pending` state to useFormState, which will be replaced by
`useActionState` in the next diff. We will keep `useFormState` around
for backwards compatibility, but functionally it will work the same as
`useActionState`, which has an `isPending` state returned.
We broke the ability to "break on uncaught exceptions" by adding a
try/catch higher up in the scheduling. We're giving up on fixing that so
we can remove the replay trick inside an event handler.
The issue with that approach is that we end up double logging a lot of
errors in DEV since they get reported to the page.
It's also a lot of complexity around this feature.
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.
If there's invalid dom nesting, there will be mismatches following but
the nesting is the most important cause of the problem.
Previously we would include the DOM nesting when rerendering thanks to
the new model of throw and recovery. However, the log would come during
the recovery phase which is after we've already logged that there was a
hydration mismatch.
People would consistently miss this log. Which is fair because you
should always look at the first log first as the most probable cause.
This ensures that we log in the hydration phase if there's a dom nesting
issue. This assumes that the consequence of nesting will appear such
that the won't have a mismatch before this. That's typically the case
because the node will move up and to be a later sibling. So as long as
that happens and we keep hydrating depth first, it should hold true.
There might be an issue if there's a suspense boundary between the nodes
we'll find discover the new child in the outer path since suspense
boundaries as breadth first.
Before:
<img width="996" alt="Screenshot 2024-02-23 at 7 34 01 PM"
src="https://github.com/facebook/react/assets/63648/af70cf7f-898b-477f-be39-13b01cfe585f">
After:
<img width="853" alt="Screenshot 2024-02-23 at 7 22 24 PM"
src="https://github.com/facebook/react/assets/63648/896c6348-1620-4f99-881d-b6069263925e">
Cameo: RSC stacks.
This pattern is a petpeeve of mine. I don't consider this best practice
and so most don't have these prefixes. Very inconsistent.
At best this is useless and noisey that you have to parse because the
information is also in the stack trace.
At worse these are misleading because they're highlighting something
internal (like validateDOMNesting) which even suggests an internal bug.
Even the ones public to React aren't necessarily what you called because
you might be calling a wrapper around it.
That would be properly reflected in a stack trace - which can also
properly ignore list so that the first stack you see is your callsite,
Which might be like `render()` in react-testing-library rather than
`createRoot()` for example.
Depends on:
- #28317
- #28320
---
Changes the behavior of the JSX runtime to pass through `ref` as a
normal prop, rather than plucking it from the props object and storing
on the element.
This is a breaking change since it changes the type of the receiving
component. However, most code is unaffected since it's unlikely that a
component would have attempted to access a `ref` prop, since it was not
possible to get a reference to one.
`forwardRef` _will_ still pluck `ref` from the props object, though,
since it's extremely common for users to spread the props object onto
the inner component and pass `ref` as a differently named prop. This is
for maximum compatibility with existing code — the real impact of this
change is that `forwardRef` is no longer required.
Currently, refs are resolved during child reconciliation and stored on
the fiber. As a result of this change, we can move ref resolution to
happen only much later, and only for components that actually use them.
Then we can remove the `ref` field from the Fiber type. I have not yet
done that in this step, though.
Treat `<a href="" />` the same with and without
`enableFilterEmptyStringAttributesDOM`
in https://github.com/facebook/react/pull/18513 we started to warn and
ignore for empty `href` and `src` props since it usually hinted at a
mistake. However, for anchor tags there's a valid use case since `<a
href=""></a>` will by spec render a link to the current page. It could
be used to reload the page without having to rely on browser
affordances.
The implementation for Fizz is in the spirit of
https://github.com/facebook/react/pull/21153. I gated the fork behind
the flag so that the fork is DCE'd when the flag is off.
Updates Fizz to handle Hoistables (Resources and Elements) in a way that
better aligns with Suspense fallbacks
1. Hoistable Elements inside a fallback (regardless of how deep and how
many additional boundaries are intermediate) will be ignored. The
reasoning is fallbacks are transient and since there is not good way to
clean up hoistables because they escape their Suspense container its
better to not emit them in the first place. SSR fallbacks are already
not full fidelity because they never hydrate so this aligns with that
somewhat.
2. Hoistable stylesheets in fallbacks will only block the reveal of a
parent suspense boundary if the fallback is going to flush with that
completed parent suspense boundary. Previously if you rendered a
stylesheet Resource inside a fallback any parent suspense boundaries
that completed after the shell flushed would include that resource in
the set required to resolve before the boundary reveal happens on the
client. This is not a semantic change, just a performance optimization
3. preconnect and preload hoistable queues are gone, if you want to
optimize resource loading you shoudl use `ReactDOM.preconnect` and
`ReactDOM.preload`. `viewport` meta tags get their own queue because
they need to go before any preloads since they affect the media state.
In addition to those functional changes this PR also refactors the
boundary resource tracking by moving it to the task rather than using
function calls at the start of each render and flush. Tasks also now
track whether they are a fallback task
supercedes prior work here: https://github.com/facebook/react/pull/27534
## Summary
In the precendences Map every key is prefixed with `p`. This fixes one
case where this is missing.
## 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 PR adds a new FB-specific configuration of Flight. We also need to
bundle a version of ReactSharedSubset that will be used for running
Flight on the server.
This initial implementation does not support server actions yet.
The FB-Flight still uses the text protocol on the server (the flag
`enableBinaryFlight` is set to false). It looks like we need some
changes in Hermes to properly support this binary format.
`onHeaders` can throw however for now we can assume that headers are
optimistic values since the only things we produce for them are preload
links. This is a pragmatic decision because React could concievably have
headers in the future which were not optimistic and thus non-optional
however it is hard to imagine what these headers might be in practice.
If we need to change this behavior to be fatal in the future it would be
a breaking change.
This commit adds error logging when `onHeaders` throws and ensures the
request can continue to render successfully.
I introduced a bug in a recent change to how bootstrap scripts are
handled. Rather than clearing out the bootstrap script state from
ResumableState on completion of the prerender I did it during the
flushing phase which comes later after the postponed state has likely
been serialized. We should freeze these objects in dev so this is not
possible to do easily in test (nor in actual code in real systems).
This fixes the bug by eliminating the bootstrap config during
getPostponedState which is before the state can be serialized.
Previously it was possible to postpone in the shell during a prerender
and then during a resume the bootstrap scripts would not be emitted
leading to no hydration on the client. This change moves the bootstrap
configuration to `ResumableState` where it can be serialized after
postponing if it wasn't flushed as part of the static shell.
Sebastian Markbåge
Josh Story
Hugo Sales
Jan Kassens
Sebastian Silbermann
Jack Pope
Sophie Alpert
Andrey LunyovandRicky Hanlon
Andrew Clark
Justin Dhillon
Tobias Koppers
BIKI DAS
