Commit Graph

4899 Commits

Author SHA1 Message Date
Josh Story 1dba980e1f (cherrypick) [Fizz][Float] Do not write after closing the stream (#27541) (#31881)
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.
2024-12-20 14:44:43 -08:00
Josh Story 720de7f81a (cherrypick) [Flight Reply] Reject any new Chunks not yet discovered at the time of reportGlobalError (#31840) (#31872)
We might have already resolved models that are not pending and so are
not rejected by aborting the stream. When those later get parsed they
might discover new chunks which end up as pending. These should be
errored since they will never be able to resolve later.

This avoids infinitely hanging the stream.

This same fix needs to be ported to ReactFlightClient that has the same
issue.

Co-authored-by: Sebastian Markbåge <sebastian@calyptus.eu>
2024-12-20 09:01:12 -08:00
Andrew Clark d900fadbf9 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 Clark 13d0225c7d 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 Clark 62512bafc6 Test fix: Add missing warning assertion (#27434)
Adds a missing test assertion for Server Context deprecation.

The PR that added this warning was based on an older revision than the
PR that added the test.
2023-09-28 12:21:11 -04:00
Sebastian Markbåge 1ebedbec2b Add Server Context deprecation warning (#27424)
As agreed, we're removing Server Context. This was never official
documented.

We've found that it's not that useful in practice. Often the better
options are:

- Read things off the url or global scope like params or cookies.
- Use the module system for global dependency injection.
- Use `React.cache()` to dedupe multiple things instead of computing
once and passing down.

There are still legit use cases for Server Context but you have to be
very careful not to pass any large data, so in generally we recommend
against it anyway.

Yes, prop drilling is annoying but it's not impossible for the cases
this is needed. I would personally always pick it over Server Context
anyway.

Semantically, Server Context also blocks object deduping due to how it
plays out with Server Components that can't be deduped. This is much
more important feature.

Since it's already in canary along with the rest of RSC, we're adding a
warning for a few versions before removing completely to help migration.

---------

Co-authored-by: Josh Story <josh.c.story@gmail.com>
2023-09-28 11:03:19 -04:00
Josh Story f81c0f1ed9 [Flight] Implement react-server-dom-turbopack (#27315)
stacked on #27314 

Turbopack requires a different module loading strategy than Webpack and
as such this PR implements a new package `react-server-dom-turbopack`
which largely follows the `react-server-dom-webpack` but is implemented
for this new bundler
2023-09-27 10:03:57 -07:00
Josh Story 701ac2e572 [Flight][Float] Preinitialize module imports during SSR (#27314)
Currently when we SSR a Flight response we do not emit any resources for
module imports. This means that when the client hydrates it won't have
already loaded the necessary scripts to satisfy the Imports defined in
the Flight payload which will lead to a delay in hydration completing.

This change updates `react-server-dom-webpack` and
`react-server-dom-esm` to emit async script tags in the head when we
encounter a modules in the flight response.

To support this we need some additional server configuration. We need to
know the path prefix for chunk loading and whether the chunks will load
with CORS or not (and if so with what configuration).
2023-09-27 09:53:31 -07:00
Josh Story 49eba01930 [Fizz][Float] Refactor Resources (#27400)
Refactors Resources to have a more compact and memory efficient
struture. Resources generally are just an Array of chunks. A resource is
flushed when it's chunks is length zero. A resource does not have any
other state.

Stylesheets and Style tags are different and have been modeled as a unit
as a StyleQueue. This object stores the style rules to flush as part of
style tags using precedence as well as all the stylesheets associated
with the precedence. Stylesheets still need to track state because it
affects how we issue boundary completion instructions. Additionally
stylesheets encode chunks lazily because we may never write them as html
if they are discovered late.

The preload props transfer is now maximally compact (only stores the
props we would ever actually adopt) and only stores props for
stylesheets and scripts because other preloads have no resource
counterpart to adopt props into. The ResumableState maps that track
which keys have been observed are being overloaded. Previously if a key
was found it meant that a resource already exists (either in this render
or in a prior prerender). Now we discriminate between null and object
values. If map value is null we can assume the resource exists but if it
is an object that represents a prior preload for that resource and the
resource must still be constructed.
2023-09-26 09:59:39 -07:00
Sebastian Markbåge bff6be8eb1 [Fizz] Track postpones in fallbacks (#27421)
This fixes so that you can postpone in a fallback. This postpones the
parent boundary. I track the fallbacks in a separate replay node so that
when we resume, we can replay the fallback itself and finish the
fallback and then possibly later the content. By doing this we also
ensure we don't complete the parent too early since now it has a render
task on it.

There is one case that this surfaces that isn't limited to
prerender/resume but also render/hydrateRoot. I left todos in the tests
for this.

If you postpone in a fallback, and suspend in the content but eventually
don't postpone in the content then we should be able to just skip
postponing since the content rendered and we no longer need the
fallback. This is a bit of a weird edge case though since fallbacks are
supposed to be very minimal.

This happens because in both cases the fallback starts rendering early
as soon as the content suspends. This also ensures that the parent
doesn't complete early by increasing the blocking tasks. Unfortunately,
the fallback will irreversibly postpone its parent boundary as soon as
it hits a postpone.

When you suspend, the same thing happens but we typically deal with this
by doing a "soft" abort on the fallback since we don't need it anymore
which unblocks the parent boundary. We can't do that with postpone right
now though since it's considered a terminal state.

I think I'll just leave this as is for now since it's an edge case but
it's an annoying exception in the model. Makes me feel I haven't quite
nailed it just yet.
2023-09-25 19:02:25 -04:00
Ruslan Lesiutin 94d5b5b2bf React DevTools 4.28.3 -> 4.28.4 (#27419)
* refactor[devtools/extension]: refactored messaging logic across
different parts of the extension ([hoxyq](https://github.com/hoxyq) in
[#27417](https://github.com/facebook/react/pull/27417))
* fix[devtools/extension]: added a workaround for proxy content script
injection in firefox ([hoxyq](https://github.com/hoxyq) in
[#27375](https://github.com/facebook/react/pull/27375))
* fix[devtools/useTransition]: don't check for dispatch property when
determining if hook is stateful ([hoxyq](https://github.com/hoxyq) in
[#27365](https://github.com/facebook/react/pull/27365))
* feat[devtools/extension]: show disclaimer when page doesnt run react
and refactor react polling logic ([hoxyq](https://github.com/hoxyq) in
[#27373](https://github.com/facebook/react/pull/27373))
* feat:-Added a delete all filters action and added title to the add
filter a… ([Biki-das](https://github.com/Biki-das) in
[#27332](https://github.com/facebook/react/pull/27332))
* fix[devtools/extension]: unregister dynamically injected content
scripts instead of filtering ([hoxyq](https://github.com/hoxyq) in
[#27369](https://github.com/facebook/react/pull/27369))
* refactor[devtools/extension]: more stable element updates polling to
avoid timed out errors ([hoxyq](https://github.com/hoxyq) in
[#27357](https://github.com/facebook/react/pull/27357))
* feat[devtools/extension]: add dark theme for popup
([rakleed](https://github.com/rakleed) in
[#27330](https://github.com/facebook/react/pull/27330))
2023-09-25 13:24:52 -04:00
Sebastian Markbåge 430e712bbd [Fizz] Restrict types of keyPath when it is known (#27418)
When we render an element, we provide the newly created keypath because
that means we're never at the root.
2023-09-25 12:26:17 -04:00
Ruslan Lesiutin 09285d5a7f refactor[devtools/extension]: refactored messaging logic across different parts of the extension (#27417)
1.
https://github.com/bvaughn/react/commit/9fc04eaf3fb701cdc14f57d5aed48f3126af6c94#diff-2c5e1f5e80e74154e65b2813cf1c3638f85034530e99dae24809ab4ad70d0143
introduced a vulnerability: we listen to `'fetch-file-with-cache'` event
from `window` to fetch sources of the file, in which we want to parse
hook names. We send this event via `window`, which means any page can
also use this and manipulate the extension to perform some `fetch()`
calls. With these changes, instead of transporting message via `window`,
we have a distinct content script, which is responsible for fetching
sources. It is notified via `chrome.runtime.sendMessage` api, so it
can't be manipulated.
2. Consistent structure of messages `{source: string, payload: object}`
in different parts of the extension
3. Added some wrappers around `chrome.scripting.executeScript` API in
`packages/react-devtools-extensions/src/background/executeScript.js`,
which support custom flow for Firefox, to simulate support of
`ExecutionWorld.MAIN`.
2023-09-25 12:02:13 -04:00
Sebastian Markbåge 69728fde0a [Flight] Dedupe suspense boundaries when it has already been found earlier (#27408)
If we track a postponed SuspenseBoundary parent that we have to replay
through before it has postponed and it postpones itself later, we need
to upgrade it to a postponed replay boundary instead of adding two.
2023-09-23 12:33:48 -04:00
Sophie Alpert 7f6201889e Ship diffInCommitPhase (#27409)
Performance tests at Meta showed neutral results.
2023-09-22 20:24:42 -07:00
Sebastian Markbåge d9e00f795b Stop flowing and then abort if a stream is cancelled (#27405)
We currently abort a stream either it's explicitly told to abort (e.g.
by an abortsignal). In this case we still finish writing what we have as
well as instructions for the client about what happened so it can
trigger fallback cases and log appropriately.

We also abort a request if the stream itself cancels. E.g. if you can't
write anymore. In this case we should not write anything to the outgoing
stream since it's supposed to be closed already now. However, we should
still abort the request so that more work isn't performed and so that we
can log the reason for it to the onError callback.

We should also not do any work after aborting.

There we need to stop the "flow" of bytes - so I call stopFlowing in the
cancel case before aborting.

The tests were testing this case but we had changed the implementation
to only start flowing at initial read (pull) instead of start like we
used to. As a result, it was no longer covering this case. We have to
call reader.read() in the tests to start the flow so that we need to
cancel it.

We also were missing a final assertion on the error logs and since we
were tracking them explicitly the extra error was silenced.
2023-09-22 15:16:49 -04:00
Ruslan Lesiutin f9d75e32ba fix[devtools/extension]: added a workaround for proxy content script injection in firefox (#27375)
Changes:
1. [Firefox-only] For some reason, Firefox might try to inject
dynamically registered content script in pages like `about:blank`. I
couldn't find a way to change this behaviour, `about:` is not a valid
scheme, so we can't exclude it and `match_about_blank` flag is not
supported in `chrome.scripting.registerContentScripts`.
2. While navigating the history in Firefox, some content scripts might
not be re-injected and still be alive. To handle this, we are now
patching `window` with `__REACT_DEVTOOLS_PROXY_INJECTED__` flag, to make
sure that proxy is injected and only once. This flag is cleared on
`pagehide` event.
2023-09-22 19:36:03 +01:00
Ruslan Lesiutin 56b14477e9 fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful (#27365)
https://github.com/facebook/react/pull/26740 introduced regression:
React DevTools doesn't record updates for `useTransition` hook. I can
add more details about things on DevTools side, if needed.

The root cause is
https://github.com/facebook/react/blob/491aec5d6113ce5bae7c10966bc38a4a8fc091a8/packages/react-reconciler/src/ReactFiberHooks.js#L2728-L2730

React DevTools expects dispatch to be present for stateful hooks that
can schedule an update -
https://github.com/facebook/react/blob/2eed1328478e8c923fcb4e6abf5efbd9e1233402/packages/react-devtools-shared/src/backend/renderer.js#L1422-L1428

With these changes, we still call dispatch in `startTransition`, but
also patch `queue` object with it, so that React DevTools can recognise
`useTransition` as stateful hook that can schedule update.

I am not sure if this is the right approach to fix this, can we
distinguish if `startTransition` was called from `useTransition` hook or
as a standalone function?
2023-09-21 21:26:20 +01:00
Andrew Clark 68ac6dbcf8 Use MurmurHash3 when native hashing function is not available (#27399) 2023-09-21 14:59:51 -04:00
Sebastian Markbåge 47fed6961f [Fizz] Simplify ReplayNode data structure (#27395)
The key is that instead of storing different tags of resumable points,
we just store if a replay node has any resumable slots and if that's at
the root `number` or if it has resumable slots by index.

This is a simpler and more compact format because we don't have to
separate the three Resume forms.

This helps deal with Postpone in fallbacks because it doesn't just
double all the cases.
2023-09-21 13:49:57 -04:00
Sophie Alpert 3c27178a2f Update tracked value after resetting radio group (#27394)
Fixes #26876, I think. Review each commit separately (all assertions
pass in main already, except the last assertInputTrackingIsClean in
"should control radio buttons").

I'm actually a little confused on two things here:
* All the isCheckedDirty assertions are true. But I don't think we set
.checked unconditionally? So how does this happen?
* https://github.com/facebook/react/issues/26876#issuecomment-1611662862
claims that
https://github.com/facebook/react/compare/d962f35ca...1f248bdd7 contains
the faulty change, but it doesn't appear to change the restoration logic
that I've touched here. (One difference outside restoration is that
updateProperties did previously set `.checked` when `nextProp !==
lastProp` whereas the new logic in updateInput is to set it when
`node.checked !== !!checked`.)

But it seems to me like we need this call here anyway, and if it fixes
it then it fixes it? I think technically speaking we probably should do
all the updateInput() calls and then all the updateValueIfChanged()
calls—in particular I think if clicking A changed the checked radio
button from B to C then the code as I have it would be incorrect, but
that also seems unlikely so idk whether to care.

cc @zhengjitf @Luk-z who did some investigation on the original issue
2023-09-20 21:57:09 -07:00
Andrew Clark 2b3d582683 useFormState: Hash the component key path for more compact output (#27397)
To support MPA-style form submissions, useFormState sends down a key
that represents the identity of the hook on the page. It's based on the
key path of the component within the React tree; for deeply nested
hooks, this keypath can become very long. We can hash the key to make it
shorter.

Adds a method called createFastHash to the Stream Config interface.
We're not using this for security or obfuscation, only to generate a
more compact key without sacrificing too much collision resistance.

- In Node.js builds, createFastHash uses the built-in crypto module.
- In Bun builds, createFastHash uses Bun.hash. See:
https://bun.sh/docs/api/hashing#bun-hash

I have not yet implemented createFastHash in the Edge, Browser, or FB
(Hermes) stream configs because those environments do not have a
built-in hashing function that meets our requirements. (We can't use the
web standard `crypto` API because those methods are async, and yielding
to the main thread is too costly to be worth it for this particular use
case.) We'll likely use a pure JS implementation in those environments;
for now, they just return the original string without hashing it. I'll
address this in separate PRs.
2023-09-20 17:13:14 -04:00
Karim Piyar Ali 1f4936660d [Fizz] Allow passing a reason to abortStream (#26992)
## Summary

Currently `ReactFizzServer.abort` allows you to pass in the a `reason`
error, which then gets passed to the `onError` handler for each task
that ends up getting aborted. This adds in the ability to pass down that
same `reason` error to `ReactDOMServerFB.abortStream` as well.

## How did you test this change?

Added a test case to ReactDOMServerFB-test.internal.js
2023-09-20 16:41:12 -04:00
Sebastian Markbåge b775564d35 [Fizz] Ensure Resumable State is Serializable (#27388)
Moves writing queues to renderState.

We shouldn't need the resource tracking's value. We just need to know if
that resource has already been emitted. We can use a Set for this. To
ensure that set is directly serializable we can just use a
dictionary-like object with no value.

See individual commits for special cases.
2023-09-20 12:21:53 -04:00
Ruslan Lesiutin 1b1dcb8a40 feat[devtools/extension]: show disclaimer when page doesnt run react and refactor react polling logic (#27373)
Changes:
1. Refactored react polling logic, now each `.eval()` call is wrapped in
Promise, so we can chain them properly.
2. When user has browser DevTools opened and React DevTools panels were
mounted, user might navigate to the page, which doesn't have React
running. Previously, we would show just blank white page, now we will
show disclaimer. Disclaimer appears after 5 failed attempts to find
React. We will also show this disclaimer if it takes too long to load
the page, but once any React instance is loaded and registered, we will
update the panels.
3. Dark theme support for this disclaimer and popups in Firefox &
Chromium-based browsers

**Important**: this is only valid for case when React DevTools panels
were already created, like when user started debugging React app and
then switched to non-React page. If user starts to debug non-React app
(by opening browser DevTools for it), we will not create these panels,
just like before.

Q: "Why do we poll to get information about react?"
A: To handle case when react is loaded after the page has been loaded,
some sandboxes for example.

| Before | After |
| --- | --- |
| <img width="1840" alt="Screenshot 2023-09-14 at 15 37 37"
src="https://github.com/facebook/react/assets/28902667/2e6ffb39-5698-461d-bfd6-be2defb41aad">
| <img width="1840" alt="Screenshot 2023-09-14 at 15 26 16"
src="https://github.com/facebook/react/assets/28902667/1c8ad2b7-0955-41c5-b8cc-d0fdb03e13ca">
|
2023-09-20 13:30:50 +01:00
BIKI DAS a3743b2e44 feat:-Added a delete all filters action and added title to the add filter a… (#27332)
### changes

1. A small action added to delete all filters, it can be useful to
delete when someone has a bunch of them rather than clicking on them
selectively.

2.There was no title on the `add filter` button so added the same


https://github.com/facebook/react/assets/72331432/af6c0725-05b6-4f46-b8c9-8079bd933a8e



review @hoxyq

---------

Co-authored-by: BIKI DAS <bikidas@BIKIs-MacBook-Pro.local>
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
2023-09-20 10:26:44 +01:00
Sebastian Markbåge 2807d781a0 [Fizz] Reuse rootSegmentID as the SuspenseBoundaryID (#27387)
Originally the intension was to have React assign an ID to a user
rendered DOM node inside a `fallback` while it was loading. If there
already were an explicit `id` defined on the DOM element we would reuse
that one instead. That's why this was a DOM Config option and not just
built in to Fizz.

This became tricky since it can load late and so we'd have to transfer
it down and detect it only once it finished rendering and if there is no
DOM element it doesn't work anyway. So instead, what we do in practice
is to always use a `<template>` tag with the ID. This has the downside
of an extra useless node and shifting child CSS selectors.

Maybe we'll get around to fixing this properly but it might not be worth
it.

This PR just gets rid of the SuspenseBoundaryID concept and instead we
just use the same ID number as the root segment ID of the boundary to
refer to the boundary to simplify the implementation.

This also solves the problem that SuspenseBoundaryID isn't currently
serializable (although that's easily fixable by itself if necessary).
2023-09-18 11:56:47 -04:00
Sebastian Markbåge 925c66a647 [Fizz] Client render the nearest child or parent suspense boundary if replay errors or is aborted (#27386)
Based on #27385.

When we error or abort during replay, that doesn't actually error the
component that errored because that has already rendered. The error only
affects any child that is not yet completed. Therefore the error kind of
gets thrown at the resumable point.

The resumable point might be a hole in the replay path, in which case
throwing there errors the parent boundary just the same as if the replay
component errored. If the hole is inside a deeper Suspense boundary
though, then it's that Suspense boundary that gets client rendered. I.e.
the child boundary. We can still finish any siblings.

In the shell all resumable points are inside a boundary since we must
have finished the shell. Therefore if you error in the root, we just
simply just turn all incomplete boundaries into client renders.
2023-09-18 11:25:56 -04:00
Sebastian Markbåge df061b3966 [Fizz] Don't bail out of flushing if we still have pending root tasks (#27385)
The idea for this check is that we shouldn't flush anything before we
flush the shell. That may or may not hold true in future formats like
RN.

It is a problem for resuming because with resuming it's possible to have
root tasks that are used for resuming but the shell was already flushed
so we can have completed boundaries before the shell has fully resumed.
What matters is whether the parent has already flushed or not.

It's not technically necessary to bail early because there won't be
anything in partialBoundaries or completedBoundaries because nothing
gets added there unless the parent has already flushed.

It's not exactly slow to have to check the length of three arrays so
it's probably not a big deal.

Flush partials in an early preamble needs further consideration
regardless.
2023-09-18 11:25:21 -04:00
Sebastian Markbåge a5fc797db1 [Fizz] Replay Postponed Paths (#27379)
This forks Task into ReplayTask and RenderTask.

A RenderTask is the normal mode and it has a segment to write into.

A ReplayTask doesn't have a segment to write into because that has
already been written but instead it has a ReplayState which keeps track
of the next set of paths to follow. Once we hit a "Resume" node we
convert it into a RenderTask and continue rendering from there.

We can resume at either an Element position or a Slot position. An
Element pointing to a component doesn't mean we resume that component,
it means we resume in the child position directly below that component.
Slots are slots inside arrays.

Instead of statically forking most paths, I kept using the same path and
checked for the existence of a segment or replay state dynamically at
runtime.

However, there's still quite a bit of forking here like retryRenderTask
and retryReplayTask. Even in the new forks there's a lot of duplication
like resumeSuspenseBoundary, replaySuspenseBoundary and
renderSuspenseBoundary. There's opportunity to simplify this a bit.
2023-09-15 15:24:04 -04:00
Andrew Clark 2d2f2af29b Restrict React DOM imports from Server Components (#27382)
Adds a separate entry point for the react-dom package when it's accessed
from a Server Component environment, using the "react-server" export
condition.

When you're inside a Server Component module, you won't be able to
import client-only APIs like useState. This applies to almost all React
DOM exports, except for Float ones like preload.
2023-09-15 14:53:19 -04:00
Andrew Clark d6dcad6a8b useFormState: Only emit markers if postback state is provided (#27374)
This is an optimization where useFormState will only emit extra comment
markers if a form state is passed at the root. If no state is passed, we
don't need to emit anything because none of the hooks will match.
2023-09-14 11:46:16 -04:00
Ruslan Lesiutin 54c2f2a346 fix[devtools/extension]: unregister dynamically injected content scripts instead of filtering (#27369)
Same as https://github.com/facebook/react/pull/26765.
Related bug report -
https://bugs.chromium.org/p/chromium/issues/detail?id=1393762.

This was changed in https://github.com/facebook/react/pull/27215, when I
have refactored background logic in the extension. I've missed this
case, because the extension was working in incognito mode.

Turns out, it stops working after the first reload, and it stops only in
incognito mode, so I am rolling out back the previous workaround.

Tested on Chrome that it fixes the extension in incognito mode and that
it doesn't affect default mode.
2023-09-14 15:21:28 +01:00
Andrew Clark caa716d50b useFormState: MPA submissions to a different page (#27372)
The permalink option of useFormState controls which page the form is
submitted to during an MPA form submission (i.e. a submission that
happens before hydration, or when JS is disabled). If the same
useFormState appears on the resulting page, and the permalink option
matches, it should receive the form state from the submission despite
the fact that the keypaths do not match.

So the logic for whether a form state instance is considered a match is:
- Both instances must be passed the same action signature
- If a permalink is provided, the permalinks must match.
- If a permalink is not provided, the keypaths must match.

Currently, if there are multiple matching useFormStates, they will all
match and receive the form state. We should probably only match the
first one, and/or warn when this happens. I've left this as a TODO for
now, pending further discussion.
2023-09-14 01:07:00 -04:00
Sebastian Markbåge a6e4791b11 [Fizz] Fix root segment IDs (#27371)
Typically we assign IDs lazily when flushing to minimize the ids we have
to assign and we try to maximize inlining.

When we prerender we could always flush into a buffer before returning
but that's not actually what we do right now. We complete rendering
before returning but we don't actually run the flush path until someone
reads the resulting stream. We assign IDs eagerly when something
postpone so that we can refer to those ids in the holes without flushing
first.

This leads to some interesting conditions that needs to consider this.

This PR also deals with rootSegmentId which is the ID of the segment
that contains the body of a Suspense boundary. The boundary needs to
know this in addition to the Suspense Boundary's ID to know how to
inject the root segment into the boundary.

Why is the suspense boundary ID and the rootSegmentID just not the same
ID? Why don't segments and suspense boundary not share ID namespace?
That's a good question and I don't really remember. It might not be a
good reason and maybe they should just be the same.
2023-09-13 23:18:03 -04:00
Andrew Clark 95c9554bc7 useFormState: Compare action signatures when reusing form state (#27370)
During an MPA form submission, useFormState should only reuse the form
state if same action is passed both times. (We also compare the key
paths.)

We compare the identity of the inner closure function, disregarding the
value of the bound arguments. That way you can pass an inline Server
Action closure:

```js
function FormContainer({maxLength}) {
  function submitAction(prevState, formData) {
    'use server'
    if (formData.get('field').length > maxLength) {
      return { errorMsg: 'Too many characters' };
    }
    // ...
  }
  return <Form submitAction={submitAction} />
}
```
2023-09-13 20:46:22 -04:00
Andrew Clark 612b2b6601 useFormState: Reuse state from previous form submission (#27321)
If a Server Action is passed to useFormState, the action may be
submitted before it has hydrated. This will trigger a full page
(MPA-style) navigation. We can transfer the form state to the next page
by comparing the key path of the hook instance.

`ReactServerDOMServer.decodeFormState` is used by the server to extract
the form state from the submitted action. This value can then be passed
as an option when rendering the new page. It must be passed during both
SSR and hydration.

```js
const boundAction = await decodeAction(formData, serverManifest);
const result = await boundAction();
const formState = decodeFormState(result, formData, serverManifest);

// SSR
const response = createFromReadableStream(<App />);
const ssrStream = await renderToReadableStream(response, { formState })

// Hydration
hydrateRoot(container, <App />, { formState });
```

If the `formState` option is omitted, then the state won't be
transferred to the next page. However, it must be passed in both places,
or in neither; misconfiguring will result in a hydration mismatch.

(The `formState` option is currently prefixed with `experimental_`)
2023-09-13 18:30:40 -04:00
Sebastian Markbåge e5205658f4 [Fizz] Various smaller refactors (#27368)
Back ported from a larger PR.
2023-09-13 00:16:44 -04:00
Andrew Clark 69be472c11 Fix: Initialize childIndex in Task constructor (#27367)
This field was not being initialized. Although the property is part of
the Flow type, the type error wasn't caught because the constructor
itself is not covered by Flow, which is unfortunate. (I assume this is
related to the dev-only componentStack property.)
2023-09-12 22:17:03 -04:00
Andrew Clark d07921eeda Don't modify keyPath until right before recursive renderNode call (#27366)
Currently, if a component suspends, the keyPath has already been
modified to include the identity of the component itself; the path is
set before the component body is called (akin to the begin phase in
Fiber). An accidental consequence is that when the promise resolves and
component is retried, the identity gets appended to the keyPath again,
leading to a duplicate node in the path.

To address this, we should only modify contexts after any code that may
suspend. For maximum safety, this should occur as late as possible:
right before the recursive renderNode call, before the children are
rendered.

I did not add a test yet because there's no feature that currently
observes it, but I do have tests in my other WIP PR for useFormState:
#27321
2023-09-12 21:27:23 -04:00
Sebastian Markbåge 7a3cb8f9cf Track the key path difference between right before the first array (#27360)
There's a subtle difference if you suspend before the first array or
after. In Fiber, we don't deal with this because we just suspend the
parent and replay it if lazy() or Usable are used in its child slots. In
Fizz we try to optimize this a bit more and enable resuming inside the
component.

Semantically, it's different if you suspend/postpone before the first
child array or inside that child array. Because when you resume the
inner result might be another array and either that's part of the parent
path or part of the inner slot.

There might be more clever way of structuring this but I just use -1 to
indicate that we're not yet inside the array and is in the root child
position. If that renders an element, then that's just the same as the 0
slot.

We need to also encode this in the resuming. I called that resuming the
element or resuming the slot.
2023-09-12 12:30:42 -04:00
Josh Story bbc8530ed7 [Float] Refactor public interface and internal HostDispatcher implementation (#27361)
When Float was first developed the internal implementation and external
interface were the same. This is problematic for a few reasons. One, the
public interface is typed but it is also untrusted and we should not
assume that it is actually respected. Two, the internal implementations
can get called from places other than the the public interface and
having to construct an options argument that ends up being destructured
to process the request is computationally wasteful and may limit JIT
optimizations to some degree. Lastly, the wire format was not as
compressed as it could be and it was untyped.

This refactor aims to address that by separating the public interface
from the internal implementations so we can solve these challenges and
also make it easier to change Float in the future

* The internal dispatcher method preinit is now preinitStyle and
preinitScript.
* The internal dispatcher method preinitModule is now
preinitModuleScript in anticipation of different implementations for
other module types in the future.
* The wire format is explicitly typed and only includes options if they
are actually used omitting undefined and nulls.
* Some function arguments are not options even if they are optional. For
instance precedence can be null/undefined because we deafult it to
'default' however we don't cosnider this an option because it is not
something we transparently apply as props to the underlying instance.
* Fixes a problem with keying images in flight where srcset and sizes
were not being taken into account.
* Moves argument validation into the ReactDOMFloat file where it is
shared with all runtimes that expose these methods
* Fixes crossOrigin serialization to use empty string except when
'use-credentials'
2023-09-12 08:09:10 -07:00
Ruslan Lesiutin 2eed132847 refactor[devtools/extension]: more stable element updates polling to avoid timed out errors (#27357)
Some context:
- When user selects an element in tree inspector, we display current
state of the component. In order to display really current state, we
start polling the backend to get available updates for the element.

Previously:
- Straight-forward sending an event to get element updates each second.
Potential race condition is not handled in any form.
- If user navigates from the page, timeout wouldn't be cleared and we
would potentially throw "Timed out ..." error.
- Bridge disconnection is not handled in any form, if it was shut down,
we could spam with "Timed out ..." errors.

With these changes:
- Requests are now chained, so there can be a single request at a time.
- Handling both navigation and shut down events.

This should reduce the number of "Timed out ..." errors that we see in
our logs for the extension. Other surfaces will also benefit from it,
but not to the full extent, as long as they utilize
"resumeElementPolling" and "pauseElementPolling" events.

Tested this on Chrome, running React DevTools on multiple tabs,
explicitly checked the case when service worker is in idle state and we
return back to the tab.
2023-09-12 15:05:39 +01:00
Sebastian Markbåge bb1d8d1667 Encode lazy Node slots properly in key path and resumable paths (#27359)
It's possible to postpone a specific node and not using a wrapper
component. Therefore we encode the resumable slot as the index slot.
When it's a plain client component that postpones, it's encoded as the
child slot inside that component which is the one that's postponed
rather than the component itself.

Since it's possible for a child slot to suspend (e.g. React.lazy's
microtask in this case) retryTask might need to keep its index around
when it resolves.
2023-09-11 19:25:24 -04:00
Andrew Clark a4aceafc63 Fix: Skip hidden inputs before text instance (#27358)
Found a hydration bug that happens when you pass a Server Action to
`formAction` and the next node is a text instance.

The HTML generated by Fizz is something like this:

```html
<button name="$ACTION_REF_5" formAction="" formEncType="multipart/form-data" formMethod="POST">
  <input type="hidden" name="$ACTION_5:0" value="..."/>
  <input type="hidden" name="$ACTION_5:1" value="..."/>
  <input type="hidden" name="$ACTION_KEY" value="..."/>Count: <!-- -->0
</button>
```

Fiber is supposed to skip over the extra hidden inputs, but it doesn't
handle this correctly if the next expected node isn't a host instance.
In this case, it's a text instance.

Not sure if the proper fix is to change the HTML that is generated, or
to change the hydration logic, but in this PR I've done the latter.
2023-09-11 18:22:37 -04:00
Pavel 627b7abd62 feat[devtools/extension]: add dark theme for popup (#27330)
## Summary

If the system/browser uses a dark theme, then the extension popup still
remains with a light background. And so that the eyes do not hurt in a
dark room, you need a dark theme.

## How did you test this change?

Make sure the dark theme is enabled on your system/browser.

**Before:**

![Screenshot 2023-09-02 at 22 20
56](https://github.com/facebook/react/assets/19418601/f7166ea7-f562-4d11-8851-be08fa9629a7)

**After:**

![Screenshot 2023-09-02 at 22 21
30](https://github.com/facebook/react/assets/19418601/e51ecd5f-3e71-4193-83ff-a548bce76bd4)
2023-09-11 14:02:54 +01:00
Josh Story 41f0e9dae3 [Float][Fizz] include preloadModule and preinitModule on react-dom/server-rendering-stub (#27347)
When implementing `preloadModule` and `preinitModule` these methods were
not exposed on the server rendering stub when they should have been.
This PR corrects that omission.
2023-09-07 14:48:22 -07:00
Andrew Clark 8b26f07a88 useFormState: Emit comment to mark whether state matches (#27307)
A planned feature of useFormState is that if the page load is the result
of an MPA-style form submission — i.e. a form was submitted before it
was hydrated, using Server Actions — the state of the hook should
transfer to the next page.

I haven't implemented that part yet, but as a prerequisite, we need some
way for Fizz to indicate whether a useFormState hook was rendered using
the "postback" state. That way we can do all state matching logic on the
server without having to replicate it on the client, too.

The approach here is to emit a comment node for each useFormState hook.
We use one of two comment types: `<!--F-->` for a normal useFormState
hook, and `<!--F!-->` for a hook that was rendered using the postback
state. React will read these markers during hydration. This is similar
to how we encode Suspense boundaries.

Again, the actual matching algorithm is not yet implemented — for now,
the "not matching" marker is always emitted.

We can optimize this further by not emitting any markers for a render
that is not the result of a form postback, which I'll do in subsequent
PRs.
2023-09-07 16:05:44 -04:00
Josh Story 3566de59e2 [Fizz][Float] <img> inside <picture> should not preload during SSR (#27346)
img tags inside picture tags should not automatically be preloaded
because usually the img is a fallback. We will consider a more
comprehensive way of preloading picture tags which may require a
technique like using an inline script to construct the image in the
browser but for now we simply omit the preloads to avoid harming load
times by loading fallbacks.
2023-09-07 12:48:40 -07:00
Sebastian Markbåge 953cb02f6d [Fizz] Split createRequest into createRequest, createPrerenderRequest and resumeRequest (#27342)
Just moving some internal code around again.

I originally encoded what type of work using startRender vs
startPrerender. I had intended to do more forking of the work loop but
we've decided not to go with that strategy. It also turns out that
forking when we start working is actually too late because of a subtle
thing where you can call abort before work begins. Therefore it's
important that starting the work comes later.
2023-09-07 00:18:30 -04:00