Commit Graph

4914 Commits

Author SHA1 Message Date
Ruslan Lesiutin 77ec61885f fix[devtools/inspectElement]: dont pause initial inspectElement call when user switches tabs (#27488)
There are not so many changes, most of them are changing imports,
because I've moved types for UI in a single file.

In https://github.com/facebook/react/pull/27357 I've added support for
pausing polling events: when user inspects an element, we start polling
React DevTools backend for updates in props / state. If user switches
tabs, extension's service worker can be killed by browser and this
polling will start spamming errors.

What I've missed is that we also have a separate call for this API, but
which is executed only once when user selects an element. We don't
handle promise rejection here and this can lead to some errors when user
selects an element and switches tabs right after it.

The only change here is that this API now has
`shouldListenToPauseEvents` param, which is `true` for polling, so we
will pause polling once user switches tabs. It is `false` by default, so
we won't pause initial call by accident.


https://github.com/hoxyq/react/blob/af8beeebf63b5824497fcd0bb35b7c0ac8fe60a0/packages/react-devtools-shared/src/backendAPI.js#L96
2023-10-10 18:10:17 +01:00
Pieter De Baets 151e75a128 [Fabric] Pass children when cloning (#27458)
## Summary

Currently when cloning nodes in Fabric, we reset a node's children on
each clone, and then repeatedly call appendChild to restore the previous
list of children (even if it was quasi-identical to before). This causes
unnecessary invalidation of the layout state in Fabric's ShadowNode data
(which in turn may require additional yoga clones) and extra JSI calls.

This PR adds a feature flag to pass in the children as part of the clone
call, so Fabric always has a complete view of the node that's being
mutated.

This feature flag requires matching changes in the react-native repo:
https://github.com/facebook/react-native/pull/39817

## How did you test this change?

Unit test added demonstrates the new behaviour 

```
yarn test -r www-modern ReactFabric-test
yarn test ReactFabric-test.internal
```

Tested a manual sync into React Native and verified core surfaces render
correctly.
2023-10-10 15:11:26 +01:00
Josh Story dddfe68820 pull implementations from the right react-dom (#27471)
should have imported from the stub implementation. this blew up the
server rendering stub size (effectively pulling in the entire client
bundle)
2023-10-05 09:14:57 -07:00
Josh Story 546178f910 react-dom/server-rendering-stub: restore experimental prefix for useFormState and useFormStatus (#27470)
in #27461 the experimental prefix was added back for `useFormState` and
`useFormStatus` in react-dom. However these functions are also exported
from the server rendering stub too and when using the stub with
experimental prefixes their absence causes unexpected errors.

This change adds back the experimental prefix for these two hooks to
match the experimental build of react-dom.
2023-10-05 08:53:14 -07:00
Sebastian Markbåge 0fba3ecf73 [Fizz] Reset error component stack and fix error messages (#27456)
The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.
2023-10-04 16:48:12 -04:00
Josh Story 6f13243957 Move ReactCurrentDispatcher back to shared internals (#27462)
The jsx-runtime uses the ReactCurrentDispatcher from shared internals.
Recently this was moved to ReactServerSharedInternals which broke
jsx-runtime. This change moves it back to ReactSharedInternals until we
can come up with a new forking mechanism.
2023-10-04 13:16:21 -07:00
Andrew Clark ca237d6f0a Add back temporary experimental_ aliases for Server Actions APIs (#27461)
This adds back the `experimental_`-prefixed Server Actions APIs to the
experimental builds only, so that apps that use those don't immediately
break when upgrading. The APIs will log a warning to encourage people to
move to the unprefixed version, or to switch to the canary release
channel.

We can remove these in a few weeks after we've given people a chance to
upgrade.

This does not affect the canary builds at all, since they never had the
prefixed versions to begin with.
2023-10-04 15:58:08 -04:00
Andrew Clark 44d40a077a Remove prefix from formState option (#27460)
`useFormState` is now in canary.
2023-10-04 15:17:37 -04:00
Andrew Clark bfefb22842 Upgrade Server Actions to canary (#27459)
Upgrades the stability of Server Actions from experimental to canary.

- Turns on enableAsyncActions and enableFormActions
- Removes "experimental_" prefix from useOptimistic, useFormStatus, and
useFormState
2023-10-04 14:51:36 -04:00
Andrew Clark 88d56b8e81 Warn if optimistic state is updated outside of a transition (#27454)
### Based on #27453 

If optimistic state is updated, and there's no startTransition on the
stack, there are two likely scenarios.

One possibility is that the optimistic update is triggered by a regular
event handler (e.g. `onSubmit`) instead of an action. This is a mistake
and we will warn.

The other possibility is the optimistic update is inside an async
action, but after an `await`. In this case, we can make it "just work"
by associating the optimistic update with the pending async action.

Technically it's possible that the optimistic update is unrelated to the
pending action, but we don't have a way of knowing this for sure because
browsers currently do not provide a way to track async scope. (The
AsyncContext proposal, if it lands, will solve this in the future.)
However, this is no different than the problem of unrelated transitions
being grouped together — it's not wrong per se, but it's not ideal.

Once AsyncContext starts landing in browsers, we will provide better
warnings in development for these cases.
2023-10-04 10:57:55 -04:00
Andrew Clark 85c2b519b5 Fix: Optimistic update does not get reset (#27453)
I found a bug where if an optimistic update causes a component to
rerender, and there are no other state updates during that render, React
bails out without applying the update.

Whenever a hook detects a change, we must mark the component as dirty to
prevent a bailout. We check for changes by comparing the new state to
`hook.memoizedState`. However, when implementing optimistic state
rebasing, I incorrectly reset `hook.memoizedState` to the incoming base
state, even though I only needed to reset `hook.baseState`. This was
just a mistake on my part.

This wasn't caught by the existing tests because usually when the
optimistic state changes, there's also some other state that marks the
component as dirty in the same render.

I fixed the bug and added a regression test.
2023-10-03 15:01:40 -04:00
Sophie Alpert db69f95e48 Fix checkbox and radio hydration (#27401)
Fixes whatever part of https://github.com/facebook/react/issues/26876
and https://github.com/vercel/next.js/issues/49499 that
https://github.com/facebook/react/pull/27394 didn't fix, probably.

From manual tests I believe this behavior brings us back to parity with
latest stable release (18.2.0). It's awkward that we keep the user's
state even for controlled inputs, so the DOM is out of sync with React
state.

Previously the .defaultChecked assignment done in updateInput() was
changing the actual checkedness because the dirty flag wasn't getting
set, meaning that hydrating could change which radio button is checked,
even in the absence of user interaction! Now we go back to always
detaching again.
2023-10-02 11:38:44 -07:00
Sophie Alpert 4f4c52a3c8 Fix controlled radios, maybe for real this time (#27443)
Fixes #26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:


https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness
from (false, true) to (true, false), we need to make sure that
input2.checked is explicitly set to false, even though setting
`input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all
the inputs rerender at the same time? Hence the TODO. But in practice
they usually would and I _think_ this is comparable to what we had
before.

Also treating function and symbol as false like we used to and like we
do on initial mount.
2023-10-02 11:38:13 -07:00
Sebastian Markbåge 843ec07021 [Flight] Taint APIs (#27445)
This lets a registered object or value be "tainted", which we block from
crossing the serialization boundary. It's only allowed to stay
in-memory.

This is an extra layer of protection against mistakes of transferring
data from a data access layer to a client. It doesn't provide perfect
protection, because it doesn't trace through derived values and
substrings. So it shouldn't be used as the only security layer but more
layers are better.

`taintObjectReference` is for specific object instances, not any nested
objects or values inside that object. It's useful to avoid specific
objects from getting passed as is. It ensures that you don't
accidentally leak values in a specific context. It can be for security
reasons like tokens, privacy reasons like personal data or performance
reasons like avoiding passing large objects over the wire.

It might be privacy violation to leak the age of a specific user, but
the number itself isn't blocked in any other context. As soon as the
value is extracted and passed specifically without the object, it can
therefore leak.

`taintUniqueValue` is useful for high entropy values such as hashes,
tokens or crypto keys that are very unique values. In that case it can
be useful to taint the actual primitive values themselves. These can be
encoded as a string, bigint or typed array. We don't currently check for
this value in a substring or inside other typed arrays.

Since values can be created from different sources they don't just
follow garbage collection. In this case an additional object must be
provided that defines the life time of this value for how long it should
be blocked. It can be `globalThis` for essentially forever, but that
risks leaking memory for ever when you're dealing with dynamic values
like reading a token from a database. So in that case the idea is that
you pass the object that might end up in cache.

A request is the only thing that is expected to do any work. The
principle is that you can derive values from out of a tainted
entry during a request. Including stashing it in a per request cache.
What you can't do is store a derived value in a global module level
cache. At least not without also tainting the object.
2023-10-02 13:55:39 -04:00
Rubén Norte 54baa7997c Add feature flag to use microtasks in the React Native Fabric renderer (#27364)
## Summary

This is part of an effort to align the event loop in React Native with
its behavior on the Web. In this case, we're going to test enabling
microtasks in React Native (Fabric) and we need React to schedule work
using microtasks if available there. This just adds a feature flag to
configure that behavior at runtime.

## How did you test this change?

* Reviewed the generated code, which looks ok.
* Did a manual sync of this PR to Meta's internal infra and tested it
with my changes to enable microtasks in RN/Hermes.
2023-10-02 17:12:35 +01:00
Sebastian Markbåge a6ed60a8eb [Fizz] Don't double replay elements when it's the postpone point (#27440)
The `resumeElement` function wasn't actually doing the correct thing
because it was resuming the element itself but what the child slot means
is that we're supposed to resume in the direct child of the element.
This is difficult to check for since it's all the possible branches that
the element can render into, so instead we just check this in
renderNode. It means the hottest path always checks the task which is
unfortunate.

And also, resuming using the correct nextSegmentId.

Fixes two bugs surfaced by this test.

---------

Co-authored-by: Josh Story <josh.c.story@gmail.com>
2023-09-29 18:24:38 -04:00
Sebastian Markbåge c7ba8c0988 Enforce that the "react-server" build of "react" is used (#27436)
I do this by simply renaming the secret export name in the "subset"
bundle and this renamed version is what the FlightServer uses.

This requires us to be more diligent about always using the correct
instance of "react" in our tests so there's a bunch of clean up for
that.
2023-09-29 18:24:05 -04: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