Commit Graph
194 Commits
Author SHA1 Message Date
Josh StoryandGitHub 9f3eaff63b [Fiber] render boundary in fallback if it contains a new stylesheet during sync update (#28965)
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
2024-05-21 16:03:46 -07:00
9e473083fe (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 <[email protected]>
2024-04-25 10:40:40 -07:00
Andrew ClarkandGitHub e042459ff1 Fix: Suspend while recovering from hydration error (#28800)
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.
2024-04-09 17:11:46 -04:00
Sebastian MarkbågeandGitHub 8322c6e77e 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
Josh StoryandGitHub d401e9be7f [DOM] move flushSync out of the reconciler (#28500)
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
2024-04-08 09:03:20 -07:00
Josh StoryandGitHub dd90cb0b45 [Fiber] Move updatePriority tracking to renderers (#28751)
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
2024-04-08 08:53:17 -07:00
Andrew ClarkandGitHub 8b3961db42 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ågeandGitHub 1d01310075 Use a Wrapper Error for onRecoverableError with a "cause" Field for the real Error (#28736)
We basically have four kinds of recoverable errors:

- Hydration mismatches.
- Server errored but client didn't.
- Hydration render errored but client render didn't (in Root or Suspense
boundary).
- Concurrent render errored but synchronous render didn't.

For the first three we log an additional error that the root or Suspense
boundary didn't error. This provides some context about what happened.
However, the problem is that for hydration mismatches that's unnecessary
extra context that is confusing. We also don't log any additional
context for concurrent render errors that could recover. This used to be
the only recoverable error so it didn't need extra context but now we
need to distinguish them. When we log these to `reportError` it's
confusing to just see the error because you didn't see anything error on
the page. It's also hard to group them together as one.

In this PR, I remove the unnecessary context for hydration mismatches.

For hydration and concurrent errors, I now wrap them in an error that
describes that what happened but then use the new `cause` field to link
the original error so we can keep that as the cause. The error that
happened was that hydration client rendered or you deopted to sync
render, the cause of that error is some other error.

For server errors, we control the Error object so I already had added
some context to that error object's message. Since we hide the message
in prod, it's nice not to have the raw message in DEV neither. We could
potentially split these into two errors for parity though.
2024-04-03 21:53:07 -04:00
Sebastian MarkbågeandGitHub 005b9ee577 Use the disableLegacyMode where ever we check the ConcurrentMode mode (#28657)
Saves some bytes and ensures that we're actually disabling it.

Turns out this flag wasn't disabling React Native/Fabric, React Noop and
React ART legacy modes so those are updated too.

Should be rebased on #28681.
2024-04-02 21:07:28 -04:00
Josh StoryandGitHub 1c19419d0a 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ågeandGitHub a822c20911 Finish cleaning up digest from onRecoverableError (#28686)
Don't need to track it separately on the captured value anymore.

Shouldn't be in the types.

I used a getter for the warning instead because Proxies are kind of
heavy weight options for this kind of warning. We typically use getters.
2024-03-30 18:32:20 -04:00
RickyandGitHub 7016d73b97 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 StoryandGitHub 85e300f637 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
RickyandGitHub 43cae547a1 Noop unstable_batchedUpdates (#28120)
## Overview

`unstable_batchedUpdates` is effectively a no-op outside of legacy mode,
this PR makes it an actual no-op outside legacy mode.
2024-03-28 14:15:37 -04:00
Josh StoryandGitHub c5fd9aba45 [Fiber] Remove the digest property from errorInfo passed to onRecoverableError (#28222)
Removes the digest property from errorInfo passed to onRecoverableError
when handling an error propagated from the server. Previously we warned
in Dev but still provided the digest on the errorInfo object. This
change removes digest from error info but continues to warn if it is
accessed. The reason for retaining the warning is the version with the
warning was not released as stable but we will include this deprecated
removal in our next major so we should communicate this change at
runtime.
2024-03-28 08:01:35 -07:00
Sebastian MarkbågeandGitHub 196550bef2 Don't log onRecoverableError if the current commit fail (#28665)
We didn't recover after all.

Currently we might log a recoverable error in the recovery pass. E.g.
the SSR server had an error. Then the client component fails to render
which errors again. This ends up double logging.

So if we fail to actually complete a fully successful commit, we ignore
any recoverable errors because we'll get real errors logged.

It's possible that this might cover up some other error that happened at
the same time.
2024-03-28 10:39:49 -04:00
Sebastian MarkbågeandGitHub 6864518952 Remove errorHydratingContainer (#28664)
I originally added this in #21021 but I didn't mention why and I don't
quite remember why. Maybe because there were no other message? However
at the time the recoverable errors mechanism didn't exist.

Today I believe all cases where this happens will trigger another
recoverable error. Namely these two:


https://github.com/facebook/react/blob/87cd859e92e72ec6e6fe77ac195512fc0e959deb/packages/react-reconciler/src/ReactFiberBeginWork.js#L1442-L1446


https://github.com/facebook/react/blob/87cd859e92e72ec6e6fe77ac195512fc0e959deb/packages/react-reconciler/src/ReactFiberBeginWork.js#L2962-L2965

Therefore this is just an extra unnecessary log.
2024-03-27 23:48:18 -04:00
Sebastian MarkbågeandGitHub afac7ac649 Make onUncaughtError and onCaughtError Configurable (#28641)
Stacked on #28627.

This makes error logging configurable using these
`createRoot`/`hydrateRoot` options:

```
onUncaughtError(error: mixed, errorInfo: {componentStack?: ?string}) => void
onCaughtError(error: mixed, errorInfo: {componentStack?: ?string, errorBoundary?: ?React.Component<any, any>}) => void
onRecoverableError(error: mixed, errorInfo: {digest?: ?string, componentStack?: ?string}) => void
```

We already have the `onRecoverableError` option since before.

Overriding these can be used to implement custom error dialogs (with
access to the `componentStack`).

It can also be used to silence caught errors when testing an error
boundary or if you prefer not getting logs for caught errors that you've
already handled in an error boundary.

I currently expose the error boundary instance but I think we should
probably remove that since it doesn't make sense for non-class error
boundaries and isn't very useful anyway. It's also unclear what it
should do when an error is rethrown from one boundary to another.

Since these are public APIs now we can implement the
ReactFiberErrorDialog forks using these options at the roots of the
builds. So I unforked those files and instead passed a custom option for
the native and www builds.

To do this I had to fork the ReactDOMLegacy file into ReactDOMRootFB
which is a duplication but that will go away as soon as the FB fork is
the only legacy root.
2024-03-27 00:51:37 -04:00
64c0a31254 [Fiber] Don't Rethrow Errors at the Root (#28627)
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 <[email protected]>
2024-03-26 23:44:07 -04:00
Sebastian MarkbågeandGitHub 79d757c34d Move Hydration Warnings from the DOM Config into the Fiber reconciliation (#28476)
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.
2024-03-26 19:04:18 -04:00
Sebastian MarkbågeandGitHub 84a5958ee3 Remove invokeGuardedCallback and replay trick (#28515)
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.
2024-03-11 20:17:07 -04:00
Noah LemenandGitHub 4f7e2b8279 cleanup enableProfilerNestedUpdateScheduledHook feature flag (#28509)
## Summary

Looks like this was added years ago for instrumentation at meta that
never ended up rolling out. Should be safe to clean up.

## How did you test this change?
`yarn test`
2024-03-11 11:37:17 -04:00
RickyandGitHub 1713240176 Update /link URLs to react.dev (#28477)
Depends on https://github.com/reactjs/react.dev/pull/6670 [merged]
2024-03-03 17:34:33 -05:00
RickyandGitHub 0b8f4be273 Move modern strict to experimental (#28152)
Turn this on 

Edited: ope, nvm
<details>
Looks like there's still an outstanding issue with this. The original PR
turned off a strict effects test, which causes a stray
`componentWillUnmount` to fire.


https://github.com/facebook/react/commit/f74ca29bd2976732f5123ed2520f9950e774745d#diff-19df471970763c4790c2cc0811fd2726cc6a891b0e1d5dedbf6d0599240c127aR70


Before:
```js
expect(log).toEqual([
      'constructor',
      'constructor',
      'getDerivedStateFromProps',
      'getDerivedStateFromProps',
      'render',
      'render',
      'componentDidMount',
    ]);
```

After:

```js
expect(log).toEqual([
      'constructor',
      'constructor',
      'getDerivedStateFromProps',
      'getDerivedStateFromProps',
      'render',
      'render',
      'componentDidMount',
      'componentWillUnmount',
      'componentDidMount',
    ]);
```

So there's a bug somewhere
</details>
2024-02-09 16:59:47 -05:00
Jan KassensandGitHub 5529dfade0 Add infinite update loop detection (#28279)
This is a partial redo of https://github.com/facebook/react/pull/26625.
Since that was unlanded due to some detected breakages. This now
includes a feature flag to be careful in rolling this out.
2024-02-09 11:14:37 -05:00
Andrew ClarkandGitHub 26b1531680 Async action support for React.startTransition (#28097)
This adds support for async actions to the "isomorphic" version of
startTransition (i.e. the one exported by the "react" package).
Previously, async actions were only supported by the startTransition
that is returned from the useTransition hook.

The interesting part about the isomorphic startTransition is that it's
not associated with any particular root. It must work with updates to
arbitrary roots, or even arbitrary React renderers in the same app. (For
example, both React DOM and React Three Fiber.)

The idea is that React.startTransition should behave as if every root
had an implicit useTransition hook, and you composed together all the
startTransitions provided by those hooks. Multiple updates to the same
root will be batched together. However, updates to one root will not be
batched with updates to other roots.

Features like useOptimistic work the same as with the hook version.

There is one difference from from the hook version of startTransition:
an error triggered inside an async action cannot be captured by an error
boundary, because it's not associated with any particular part of the
tree. You should handle errors the same way you would in a regular
event, e.g. with a global error event handler, or with a local
`try/catch`.
2024-01-25 21:54:45 -05:00
Andrew ClarkandGitHub aca9a01944 Batch async actions even if useTransition is unmounted (#28078)
If there are multiple updates inside an async action, they should all be
rendered in the same batch, even if they are separate by an async
operation (`await`). We currently implement this by suspending in the
`useTransition` hook to block the update from committing until all
possible updates have been scheduled by the action. The reason we did it
this way is so you can "cancel" an action by navigating away from the UI
that triggered it.

The problem with that approach, though, is that even if you navigate
away from the `useTransition` hook, the action may have updated shared
parts of the UI that are still in the tree. So we may need to continue
suspending even after the `useTransition` hook is deleted.

In other words, the lifetime of an async action scope is longer than the
lifetime of a particular `useTransition` hook.

The solution is to suspend whenever _any_ update that is part of the
async action scope is unwrapped during render. So, inside useState and
useReducer.

This fixes a related issue where an optimistic update is reverted before
the async action has finished, because we were relying on the
`useTransition` hook to prevent the optimistic update from finishing.

This also prepares us to support async actions being passed to the
non-hook form of `startTransition` (though this isn't implemented yet).
2024-01-24 23:54:53 -05:00
Andrew ClarkandGitHub aeb777533a Fix: useDeferredValue initialValue suspends forever without switching to final (#27888)
Fixes a bug in the experimental `initialValue` option for
`useDeferredValue` (added in #27500).

If rendering the `initialValue` causes the tree to suspend, React should
skip it and switch to rendering the final value instead. It should not
wait for `initialValue` to resolve.

This is not just an optimization, because in some cases the initial
value may _never_ resolve — intentionally. For example, if the
application does not provide an instant fallback state. This capability
is, in fact, the primary motivation for the `initialValue` API.

I mostly implemented this correctly in the original PR, but I missed
some cases where it wasn't working:

- If there's no Suspense boundary between the `useDeferredValue` hook
and the component that suspends, and we're not in the shell of the
transition (i.e. there's a parent Suspense boundary wrapping the
`useDeferredValue` hook), the deferred task would get incorrectly
dropped.
- Similarly, if there's no Suspense boundary between the
`useDeferredValue` hook and the component that suspends, and we're
rendering a synchronous update, the deferred task would get incorrectly
dropped.

What these cases have in common is that it causes the `useDeferredValue`
hook itself to be replaced by a Suspense fallback. The fix was the same
for both. (It already worked in cases where there's no Suspense fallback
at all, because those are handled differently, at the root.)

The way I discovered this was when investigating a particular bug in
Next.js that would happen during a 'popstate' transition (back/forward),
but not during a regular navigation. That's because we render popstate
transitions synchronously to preserve browser's scroll position — which
in this case triggered the second scenario above.
2024-01-07 23:17:20 -05:00
Jan KassensandGitHub 01c73ce4c5 [activity] rename unstable_Offscreen to unstable_Activity (#27640)
`Activity` is the current candidate name. This PR starts the rename work
by renaming the exported unstable component name.

NOTE: downstream consumers need to rename the import when updating to
this commit.
2023-11-02 16:13:21 -04:00
Andrew ClarkandGitHub 40c791578e useDeferredValue has higher priority than partial hydration (#27550)
By default, partial hydration is given the lowest possible priority,
because until a tree is updated, the server-rendered HTML is assumed to
match the final resolved HTML.

However, this isn't completely true because a component may choose to
"upgrade" itself upon hydration. The simplest example is a component
that calls setState in a useEffect to switch to a richer implementation
of the UI. Another example is a component that doesn't have a server-
rendered implementation, so it intentionally suspends to force a client-
only render.

useDeferredValue is an example, too: the server only renders the first
pass (the initialValue) argument, and relies on the client to upgrade to
the final value.

What we should really do in these cases is emit some information into
the Fizz stream so that Fiber knows to prioritize the hydration of
certain trees. We plan to add a mechanism for this in the future.

In the meantime, though, we can at least ensure that the priority of the
upgrade task is correct once it's "discovered" during hydration. In this
case, the priority of the task spawned by useDeferredValue should have
Transition priority, not Offscreen priority.
2023-10-23 13:48:48 -04:00
Andrew ClarkandGitHub d956cf2ca3 useDeferredValue should skip initialValue if it suspends (#27509)
### Based on https://github.com/facebook/react/pull/27505

If a parent render spawns a deferred task with useDeferredValue, but the
parent render suspends, we should not wait for the parent render to
complete before attempting to render the final value.

The reason is that the initialValue argument to useDeferredValue is
meant to represent an immediate preview of the final UI. If we can't
render it "immediately", we might as well skip it and go straight to the
"real" value.

This is an improvement over how a userspace implementation of
useDeferredValue would work, because a userspace implementation would
have to wait for the parent task to commit (useEffect) before spawning
the deferred task, creating a waterfall.
2023-10-17 12:48:11 -04:00
Andrew ClarkandGitHub c35fb40d0d Track entangled lanes separately from update lane (#27505)
A small refactor to how the lane entanglement mechanism works. We can
now distinguish between the lane that "spawned" a render task (i.e. a
new update) versus the lanes that it's entangled with. Both the update
lane and the entangled lanes will be included while rendering, but by
keeping them separate, we don't lose the original priority.

In practical terms, this means we can now entangle a low priority update
with a higher priority lane while rendering at the lower priority.

To do this, lanes that are entangled at the root are now tracked using
the same variable that we use to track the "base lanes" when revealing a
previously hidden tree — conceptually, they are the same thing. I also
renamed this variable (from subtreeLanes to entangledRenderLanes) to
better reflect how it's used.

My primary motivation is related to useDeferredValue, which I'll address
in a later PR.
2023-10-15 12:29:52 -04:00
Andrew ClarkandGitHub 405ce2e308 Bugfix: Selective hydration triggers false update loop error (#27439)
This adds a regression test and fix for a case where a sync update
triggers selective hydration, which then leads to a "Maximum update
depth exceeded" error, even though there was only a single update. This
happens when a single sync update flows into many sibling dehydrated
Suspense boundaries.

This fix is, if a commit was the result of selective hydration, we
should not increment the nested update count, because those renders
conceptually are not updates.

Ideally, they wouldn't even be in a separate commit — we should be able
to hydrate a tree and apply an update on top of it within the same
render phase. We could do this once we implement resumable context
stacks.
2023-09-29 14:29:35 -04:00
Andrew ClarkandGitHub bc2a62d690 Pass lanes as argument to performSyncWorkOnRoot (#26763)
`performSyncWorkOnRoot` has only a single caller, and the caller already
computes the next lanes (`getNextLanes`) before deciding to call the
function. So we can pass them as an argument instead of computing the
lanes again.

There was already a TODO comment about this, but it was mostly perf
related. However, @rickhanlonii noticed a discrepancy where the inner
`getNextLanes` call was not being passed the current work-in- progress
lanes. Usually this shouldn't matter because there should never be
work-in-progress sync work; it should finish immediately. There is one
case I'm aware of where we exit the work loop without finishing a sync
render, which is selective hydration, but even then it should switch to
the sync hydration lane, not the normal sync lane. So something else is
probably going on. I suspect it might be related to the
`enableUnifiedSyncLane` experiment.

This is likely related to a regression found internally at Meta. We're
still working on getting a proper regression test; I can come up with a
contrived one but I'm not confident it'll be the same as the actual
regression until we get a better repro.
2023-09-28 16:29:32 -04:00
Andrew ClarkandGitHub 2cef050982 Fix: Stylesheet in error UI suspends indefinitely (#27265)
This fixes the regression test added in the previous commit. The
"Suspensey commit" implementation relies on the
`shouldRemainOnPreviousScreen` function to determine whether to 1)
suspend the commit 2) activate a parent fallback and schedule a retry.
The issue was that we were sometimes attempting option 2 even when there
was no parent fallback.

Part of the reason this bug landed is due to how `throwException` is
structured. In the case of Suspensey commits, we pass a special "noop"
thenable to `throwException` as a way to trigger the Suspense path. This
special thenable must never have a listener attached to it. This is not
a great way to structure the logic, it's just a consequence of how the
code evolved over time. We should refactor it into multiple functions so
we can trigger a fallback directly without having to check the type. In
the meantime, I added an internal warning to help detect similar
mistakes in the future.
2023-08-22 11:22:30 -04:00
Andrew ClarkandGitHub 08bba120e5 Bugfix: Fix crash when suspending in shell during useSES re-render (#27199)
This adds a regression test for a bug where, after a store mutation, the
updated data causes the shell of the app to suspend.

When re-rendering due to a concurrent store mutation, we must check for
the RootDidNotComplete exit status again.

As a follow-up, I'm going to look into to cleaning up the places where
we check the exit status, so bugs like these are less likely. I think we
might be able to combine most of it into a single switch statement.
2023-08-08 14:11:11 -04:00
Tianyu YaoandGitHub e362395050 Make temporary NoStrictPassiveEffects option work with useModernStrictMode (#27105)
## Summary
Since we are enabling `useModernStrictMode` flag internally, to make
sure the internal testing of half StrictMode doesn't suddenly break,
this PR makes sure it also works with `useModernStrictMode` true.


## Test plan:
Manually set `useModernStrictMode` to true.
`yarn test ReactOffscreenStrictMode-test -r=www-modern --env=development
--variant=true`
`yarn test ReactStrictMode-test.internal -r=www-modern --env=development
--variant=true`
2023-07-18 11:58:01 -07:00
Jan KassensandGitHub 4c3e286f4b Revert "Fix: Detect infinite update loops caused by render phase updates (#26625)" (#27027)
This reverts commit a5c46e9d31.

This broke a number of tests when synced internally. We'll need to
investigate the breakages before relanding this.
2023-06-30 12:51:11 -04:00
Andrew ClarkandGitHub 2df8f81b45 Detect crashes caused by Async Client Components (#27019)
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)

This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.

This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.

In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.

There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any loading
UI.)

Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.

So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.

In the future, we will consider ways to make this case a warning instead
of a hard error.

See https://github.com/facebook/react/issues/26801 for more details.
2023-06-29 16:05:00 -04:00
Andrew ClarkandGitHub a5c46e9d31 Fix: Detect infinite update loops caused by render phase updates (#26625)
This PR contains a regression test and two separate fixes: a targeted
fix, and a more general one that's designed as a last-resort guard
against these types of bugs (both bugs in app code and bugs in React).

I confirmed that each of these fixes separately are sufficient to fix
the regression test I added.

We can't realistically detect all infinite update loop scenarios because
they could be async; even a single microtask can foil our attempts to
detect a cycle. But this improves our strategy for detecting the most
common kind.

See commit messages for more details.
2023-06-27 13:26:35 -04:00
Andrew ClarkandGitHub e4e779bcad Lower Suspense throttling heuristic to 300ms (#26803)
Now that the throttling mechanism applies more often, we've decided to
lower this a tad to ensure it's not noticeable. The idea is it should be
just large enough to prevent jank when lots of different parts of the UI
load in rapid succession, but not large enough to make the UI feel
sluggish. There's no perfect number, it's just a heuristic.
2023-05-16 15:59:15 -04:00
Andrew ClarkandGitHub 7fb5541de9 Fix Suspense throttling mechanism (#26802)
The throttling mechanism for fallbacks should apply to both their
appearance _and_ disappearance.

This was mostly addressed by #26611. See that PR for additional context.

However, a flaw in the implementation is that we only update the the
timestamp used for throttling when the fallback initially appears. We
don't update it when the real content pops in. If lots of content in
separate Suspense trees loads around the same time, you can still get
jank.

The issue is fixed by updating the throttling timestamp whenever the
visibility of a fallback changes. Not just when it appears.
2023-05-16 15:03:30 -04:00
Andrew ClarkandGitHub bbd00c9635 Fix: Update while suspended fails to interrupt (#26739)
This fixes a bug with `use` where if you update a component that's
currently suspended, React will sometimes mistake it for a render phase
update.

This happens because we don't reset `currentlyRenderingFiber` until the
suspended is unwound. And with `use`, that can happen asynchronously,
most commonly when the work loop is suspended during a transition.

The fix is to make sure `currentlyRenderingFiber` is only set when we're
in the middle of rendering, which used to be true until `use` was
introduced.

More specifically this means clearing `currentlyRenderingFiber` when
something throws and setting it again when we resume work.

In many cases, this bug will fail "gracefully" because the update is
still added to the queue; it's not dropped completely. It's also
somewhat rare because it has to be the exact same component that's
currently suspended. But it's still a bug. I wrote a regression test
that shows a sync update failing to interrupt a suspended component.
2023-04-28 15:35:14 -04:00
Andrew ClarkandGitHub 1b58efc5ef Fix: Resolve entangled actions independently (#26726)
When there are multiple async actions at the same time, we entangle them
together because we can't be sure which action an update might be
associated with. (For this, we'd need AsyncContext.) However, if one of
the async actions fails with an error, it should only affect that
action, not all the other actions it may be entangled with.

Resolving each action independently also means they can have independent
pending state types, rather than being limited to an `isPending`
boolean. We'll use this to implement an upcoming form API.
2023-04-25 20:43:20 -04:00
Andrew ClarkandGitHub c0e2beec0e Rethrow errors from form actions (#26689)
This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the `<form />` had an internal useTransition hook, which
is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.
2023-04-21 13:29:46 -04:00
Josh StoryandGitHub 242d8512d8 [Float][Fiber] Enable Float methods to be called outside of render (#26557)
Stacked on #26570 

Previously we restricted Float methods to only being callable while
rendering. This allowed us to make associations between calls and their
position in the DOM tree, for instance hoisting preinitialized styles
into a ShadowRoot or an iframe Document.

When considering how we are going to support Flight support in Float
however it became clear that this restriction would lead to compromises
on the implementation because the Flight client does not execute within
the context of a client render. We want to be able to disaptch Float
directives coming from Flight as soon as possible and this requires
being able to call them outside of render.

this patch modifies Float so that its methods are callable anywhere. The
main consequence of this change is Float will always use the Document
the renderer script is running within as the HoistableRoot. This means
if you preinit as style inside a component render targeting a ShadowRoot
the style will load in the ownerDocument not the ShadowRoot. Practially
speaking it means that preinit is not useful inside ShadowRoots and
iframes.

This tradeoff was deemed acceptable because these methods are
optimistic, not critical. Additionally, the other methods, preconntect,
prefetchDNS, and preload, are not impacted because they already operated
at the level of the ownerDocument and really only interface with the
Network cache layer.

I added a couple additional fixes that were necessary for getting tests
to pass that are worth considering separately.

The first commit improves the diff for `waitForThrow` so it compares
strings if possible.

The second commit makes invokeGuardedCallback not use metaprogramming
pattern and swallows any novel errors produced from trying to run the
guarded callback. Swallowing may not be the best we can do but it at
least protects React against rapid failure when something causes the
dispatchEvent to throw.
2023-04-20 14:40:25 -07:00
Andrew ClarkandGitHub 8baa58aa78 Add alwaysThrottleRetries flag (#26685)
This puts the change introduced by #26611 behind a flag until Meta is
able to roll it out. Disabling the flag reverts back to the old
behavior, where retries are throttled if there's still data remaining in
the tree, but not if all the data has finished loading.

The new behavior is still enabled in the public builds.
2023-04-20 14:23:22 -04:00
Andrew ClarkandGitHub ba3ca71328 Initial (client-only) async actions support (#26621)
Implements initial (client-only) support for async actions behind a
flag. This is an experimental feature and the design isn't completely
finalized but we're getting closer. It will be layered alongside other
features we're working on, so it may not feel complete when considered
in isolation.

The basic description is you can pass an async function to
`startTransition` and all the transition updates that are scheduled
inside that async function will be grouped together. The `isPending`
flag will be set to true immediately, and only set back to false once
the async action has completed (as well as all the updates that it
triggers).

The ideal behavior would be that all updates spawned by the async action
are automatically inferred and grouped together; however, doing this
properly requires the upcoming (stage 2) Async Context API, which is not
yet implemented by browsers. In the meantime, we will fake this by
grouping together all transition updates that occur until the async
function has terminated. This can lead to overgrouping between unrelated
actions, which is not wrong per se, just not ideal.

If the `useTransition` hook is removed from the UI before an async
action has completed — for example, if the user navigates to a new page
— subsequent transitions will no longer be grouped with together with
that action.

Another consequence of the lack of Async Context is that if you call
`setState` inside an action but after an `await`, it must be wrapped in
`startTransition` in order to be grouped properly. If we didn't require
this, then there would be no way to distinguish action updates from
urgent updates caused by user input, too. This is an unfortunate footgun
but we can likely detect the most common mistakes using a lint rule.

Once Async Context lands in browsers, we can start warning in dev if we
detect an update that hasn't been wrapped in `startTransition`. Then,
longer term, once the feature is ubiquitous, we can rely on it for real
and allow you to call `setState` without the additional wrapper.

Things that are _not_ yet implemented in this PR, but will be added as
follow ups:

- Support for non-hook form of `startTransition`
- Canceling the async action scope if the `useTransition` hook is
deleted from the UI
- Anything related to server actions
2023-04-19 13:33:11 -04:00
Tianyu YaoandGitHub b79a063752 Synchronously flush the transition lane scheduled in a popstate event (#26025)
<!--
  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 debug-test --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

Browsers restore state like forms and scroll position right after the
popstate event. To make sure the page work as expected on back or
forward button, we need to flush transitions scheduled in a popstate
synchronously, and only yields if it suspends.
This PR adds a new HostConfig method to check if `window.event ===
'popstate'`, and `scheduleMicrotask` if a transition is scheduled in a
`PopStateEvent`.

## How did you test this change?

yarn test
2023-04-13 15:21:19 -04:00
Samuel SuslaandGitHub 5ba0936f56 Remove revertRemovalOfSiblingPrerendering killswitch (#26549)
removal of sibling prerendering has been rolled out at Meta. We can
delete the flag now.
2023-04-12 21:05:17 -04:00