Commit Graph

15741 Commits

Author SHA1 Message Date
Jan Kassens 6d394e3d26
[actions] commit from special branches iff they exist (#26673)
This creates 2 special branches. If these special branches exist, we'll
commit build artifacts from these branches, main otherwise.
2023-04-19 12:11:40 -04:00
Sebastian Markbåge 1f248bdd71
Switching checked to null should leave the current value (#26667)
I accidentally made a behavior change in the refactor. It turns out that
when switching off `checked` to an uncontrolled component, we used to
revert to the concept of "initialChecked" which used to be stored on
state.

When there's a diff to this computed prop and the value of props.checked
is null, then we end up in a case where it sets `checked` to
`initialChecked`:


5cbe6258bc/packages/react-dom-bindings/src/client/ReactDOMInput.js (L69)

Since we never changed `initialChecked` and it's not relevant if
non-null `checked` changes value, the only way this "change" could
trigger was if we move from having `checked` to having null.

This wasn't really consistent with how `value` works, where we instead
leave the current value in place regardless. So this is a "bug fix" that
changes `checked` to be consistent with `value` and just leave the
current value in place. This case should already have a warning in it
regardless since it's going from controlled to uncontrolled.

Related to that, there was also another issue observed in
https://github.com/facebook/react/pull/26596#discussion_r1162295872 and
https://github.com/facebook/react/pull/26588

We need to atomically apply mutations on radio buttons. I fixed this by
setting the name to empty before doing mutations to value/checked/type
in updateInput, and then set the name to whatever it should be. Setting
the name is what ends up atomically applying the changes.

---------

Co-authored-by: Sophie Alpert <git@sophiebits.com>
2023-04-19 11:46:29 -04:00
Ruslan Lesiutin b90e8ebaa5
cleanup[devtools]: remove named hooks & profiler changed hook indices feature flags (#26635)
## Summary

Removing `enableNamedHooksFeature`, `enableProfilerChangedHookIndices`,
`enableProfilerComponentTree` feature flags, they are the same for all
configurations.
2023-04-19 10:05:31 +01:00
Ruslan Lesiutin a227bcd4f4
chore[devtools/release-scripts]: update messages / fixed npm view com… (#26660)
Some minor changes, observed while working on 24.7.5 release:
- Updated numeration of text instructions
- `reactjs.org` -> `react.dev`
- Fixed using `npm view` command for node 16+, `publish-release` script
currently fails if used with node 16+
2023-04-19 10:05:16 +01:00
Sophie Alpert c6db19f9cd
[Flight] Serialize Date (#26622)
This is kind of annoying because Date implements toJSON so
JSON.stringify turns it into a string before calling our replacer
function.
2023-04-18 20:52:03 -07:00
Mengdi Chen 96fd2fb726
(patch)[DevTools] bug fix: backend injection logic not working for undocked devtools window (#26665)
bugfix for #26492

This bug would cause users unable to use the devtools (component tree
empty).

The else-if logic is broken when user switch to undocked devtools mode
(separate window) because `sender.tab` would exist in that case.
<img width="314" alt="image"
src="https://user-images.githubusercontent.com/1001890/232930094-05a74445-9189-4d50-baf1-a0360b29ef7e.png">

Tested on Chrome with a local build
2023-04-18 20:39:22 -04:00
Sebastian Markbåge d8089f2cf2
[Flight Reply] Encode FormData (#26663)
Builds on top of https://github.com/facebook/react/pull/26661

This lets you pass FormData objects through the Flight Reply
serialization. It does that by prefixing each entry with the ID of the
reference and then the decoding side creates a new FormData object
containing only those fields (without the prefix).

Ideally this should be more generic. E.g. you should be able to pass
Blobs, Streams and Typed Arrays by reference inside plain objects too.
You should also be able to send Blobs and FormData in the regular Flight
serialization too so that they can go both directions. They should be
symmetrical. We'll get around to adding more of those features in the
Flight protocol as we go.

---------

Co-authored-by: Sophie Alpert <git@sophiebits.com>
2023-04-18 14:57:33 -04:00
Sophie Alpert 1b4a0daba8
Add assertions about <input> value dirty state (#26626)
Since this is an observable behavior and is hard to think about, seems
good to have tests for this.

The expected value included in each test is the behavior that existed
prior to #26546.
2023-04-18 11:03:36 -07:00
Sophie Alpert b433c379d5
Fix input tracking bug (#26627)
In
2019ddc75f,
we changed to set .defaultValue before .value on updates. In some cases,
setting .defaultValue causes .value to change, and since we only set
.value if it has the wrong value, this resulted in us not assigning to
.value, which resulted in inputValueTracking not knowing the right
value. See new test added.

My fix here is to (a) move the value setting back up first and (b)
narrowing the fix in the aforementioned PR to newly remove the value
attribute only if it defaultValue was previously present in props.

The second half is necessary because for types where the value property
and attribute are indelibly linked (hidden checkbox radio submit image
reset button, i.e. spec modes default or default/on from
https://html.spec.whatwg.org/multipage/input.html#dom-input-value-default),
we can't remove the value attribute after setting .value, because that
will undo the assignment we just did! That is, not having (b) makes all
of those types fail to handle updating props.value.

This code is incredibly hard to think about but I think this is right
(or at least, as right as the old code was) because we set .value here
only if the nextProps.value != null, and we now remove defaultValue only
if lastProps.defaultValue != null. These can't happen at the same time
because we have long warned if value and defaultValue are simultaneously
specified, and also if a component switches between controlled and
uncontrolled.

Also, it fixes the test in https://github.com/facebook/react/pull/26626.
2023-04-18 10:49:32 -07:00
Mengdi Chen d962f35cac
[DevTools] use backend manager to support multiple backends in extension (#26615)
In the extension, currently we do the following:
1. check whether there's at least one React renderer on the page
2. if yes, load the backend to the page
3. initialize the backend 

To support multiple versions of backends, we are changing it to:
1. check the versions of React renders on the page
2. load corresponding React DevTools backends that are shipped with the
extension; if they are not contained (usually prod builds of
prereleases), show a UI to allow users to load them from UI
3. initialize each of the backends

To enable this workflow, a backend will ignore React renderers that does
not match its version

This PR adds a new file "backendManager" in the extension for this
purpose.


------
I've tested it on Chrome, Edge and Firefox extensions
2023-04-18 12:02:42 -04:00
Ruslan Lesiutin 77d3b02e5c
React DevTools 4.27.4 -> 4.27.5 (#26637)
Full list of changes (not everything included in changelog):
* refactor[devtools]: copy to clipboard only on frontend side
([hoxyq](https://github.com/hoxyq) in
[#26604](https://github.com/facebook/react/pull/26604))
* Provide icon to edge devtools.
([harrygz889](https://github.com/harrygz889) in
[#26543](https://github.com/facebook/react/pull/26543))
* [BE] move shared types & constants to consolidated locations
([mondaychen](https://github.com/mondaychen) in
[#26572](https://github.com/facebook/react/pull/26572))
* remove backend dependency from the global hook
([mondaychen](https://github.com/mondaychen) in
[#26563](https://github.com/facebook/react/pull/26563))
* Replace deprecated `new-window` with
`webContents.setWindowOpenHandler()`
([Willie-Boy](https://github.com/Willie-Boy) in
[#26559](https://github.com/facebook/react/pull/26559))
* DevTools: Inline references to fiber flags
([acdlite](https://github.com/acdlite) in
[#26542](https://github.com/facebook/react/pull/26542))
* refactor[devtools]: forbid editing class instances in props
([hoxyq](https://github.com/hoxyq) in
[#26522](https://github.com/facebook/react/pull/26522))
* Move update scheduling to microtask
([acdlite](https://github.com/acdlite) in
[#26512](https://github.com/facebook/react/pull/26512))
* Remove unnecessary CIRCLE_CI_API_TOKEN checks
([mondaychen](https://github.com/mondaychen) in
[#26499](https://github.com/facebook/react/pull/26499))
* browser extension: improve script injection logic
([mondaychen](https://github.com/mondaychen) in
[#26492](https://github.com/facebook/react/pull/26492))
* [flow] make Flow suppressions explicit on the error
([kassens](https://github.com/kassens) in
[#26487](https://github.com/facebook/react/pull/26487))
2023-04-17 17:42:02 +01:00
Sebastian Markbåge b6006201b5
Add a way to create Server Reference Proxies on the client (#26632)
This lets the client bundle encode Server References without them first
being passed from an RSC payload. Like if you just import `"use server"`
from the client. A bundler could already emit these proxies to be called
on the client but the subtle difference is that those proxies couldn't
be passed back into the server by reference. They have to be registered
with React.

We don't currently implement importing `"use server"` from client
components in the reference implementation. It'd need to expand the
Webpack plugin with a loader that rewrites files with the `"use server"`
in the client bundle.

```
"use server";

export async function action() {
   ...
}
```
->
```
import {createServerReference} from "react-server-dom-webpack/client";
import {callServer} from "some-router/call-server";

export const action = createServerReference('1234#action', callServer);
```

The technique I use here is that the compiled output has to call
`createServerReference(id, callServer)` with the `$$id` and proxy
implementation. We then return a proxy function that is registered with
a WeakMap to the particular instance of the Flight Client.

This might be hard to implement because it requires emitting module
imports to a specific stateful runtime module in the compiler. A benefit
is that this ensures that this particular reference is locked to a
specific client if there are multiple - e.g. talking to different
servers.

It's fairly arbitrary whether we use a WeakMap technique (like we do on
the client) vs an `$$id` (like we do on the server). Not sure what's
best overall. The WeakMap is nice because it doesn't leak implementation
details that might be abused to consumers. We should probably pick one
and unify.
2023-04-14 23:20:35 -04:00
Sebastian Markbåge da6c23a45c
[Flight] Fallback to importing the whole module instead of encoding every name (#26624)
We currently don't just "require" a module by its module id/path. We
encode the pair of module id/path AND its export name. That's because
with module splitting, a single original module can end up in two or
more separate modules by name. Therefore the manifest files need to
encode how to require the whole module as well as how to require each
export name.

In practice, we don't currently use this because we end up forcing
Webpack to deopt and keep it together as a single module, and we don't
even have the code in the Webpack plugin to write separate values for
each export name.

The problem is with CJS we don't statically know what all the export
names will be. Since these cases will never be module split, we don't
really need to know.

This changes the Flight requires to first look for the specific name
we're loading and then if that name doesn't exist in the manifest we
fallback to looking for the `"*"` name containing the entire module and
look for the name in there at runtime.

We could probably optimize this a bit if we assume that CJS modules on
the server never get built with a name. That way we don't have to do the
failed lookup.

Additionally, since we've recently merged filepath + name into a single
string instead of two values, we now have to split those back out by
parsing the string. This is especially unfortunate for server references
since those should really not reveal what they are but be a hash or
something. The solution might just be to split them back out into two
separate fields again.

cc @shuding
2023-04-14 21:09:09 -04:00
Sophie Alpert 2bfe4b246f
[Flight] Fix style nit from #26623 (#26629)
Maybe this is faster.
https://github.com/facebook/react/pull/26623#discussion_r1167053174
2023-04-14 09:49:41 -07:00
Sophie Alpert ab2385fa38
[Flight] Serialize weird numbers (#26623) 2023-04-14 09:28:48 -07:00
Rubén Norte 39a3b72c6d
Post-process build files for React Native to add generated signature and @nolint (#26616)
## Summary

We're enabling a new mechanism to synchronize build files from `react`
to `react-native`. That new mechanism doesn't post-process files, so we
need to add that post-processing somewhere. This PR does that when
generating the files in the first place, so the generated files in the
`build` directory are ready to be committed in the `react-native`
repository directly.

This makes use of `signedsource` to avoid direct modifications of these
files in the `react-native` repository, as well as `@noformat` and
`@nolint` to prevent unactionable CI failures in that repository.

## How did you test this change?

Generated build files for `react-native` before and after this change:
```
node ./scripts/rollup/build-all-release-channels.js react-native
```

Checked new contents. Relevant changes:

```diff
diff --color -r build-before/react-native/implementations/ReactFabric-dev.fb.js build-after/react-native/implementations/ReactFabric-dev.fb.js
10c10
<  * @generated
---
>  * @generated SignedSource<<03cef14e77b8250b567dfdf3b066085e>>
diff --color -r build-before/react-native/implementations/ReactFabric-dev.js build-after/react-native/implementations/ReactFabric-dev.js
11c11
<  * @generated
---
>  * @generated SignedSource<<e39eed38a363846ca9ee9b59a225683c>>
diff --color -r build-before/react-native/implementations/ReactFabric-prod.fb.js build-after/react-native/implementations/ReactFabric-prod.fb.js
10c10
<  * @generated
---
>  * @generated SignedSource<<f65efcd6a469d5f6fef1ce647e5ec09a>>
diff --color -r build-before/react-native/implementations/ReactFabric-prod.js build-after/react-native/implementations/ReactFabric-prod.js
11c11
<  * @generated
---
>  * @generated SignedSource<<cdd582aa889b1054b2c5faf412622b18>>
diff --color -r build-before/react-native/implementations/ReactFabric-profiling.fb.js build-after/react-native/implementations/ReactFabric-profiling.fb.js
10c10
<  * @generated
---
>  * @generated SignedSource<<81e74849b24f104882bd298f062be0fa>>
diff --color -r build-before/react-native/implementations/ReactFabric-profiling.js build-after/react-native/implementations/ReactFabric-profiling.js
11c11
<  * @generated
---
>  * @generated SignedSource<<c050b7fa1453dc21ac1c5b98146210a8>>
diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.fb.js build-after/react-native/implementations/ReactNativeRenderer-dev.fb.js
10c10
<  * @generated
---
>  * @generated SignedSource<<9c03464b489b41c06a065aeba8619263>>
diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.js build-after/react-native/implementations/ReactNativeRenderer-dev.js
11c11
<  * @generated
---
>  * @generated SignedSource<<18b34c037544949dcf9b28f945921ba8>>
diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.fb.js build-after/react-native/implementations/ReactNativeRenderer-prod.fb.js
10c10
<  * @generated
---
>  * @generated SignedSource<<592e9654c584d1da523378b119bd8bd7>>
diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.js build-after/react-native/implementations/ReactNativeRenderer-prod.js
11c11
<  * @generated
---
>  * @generated SignedSource<<91c894db99e2d76f8a32708ad6ad1bde>>
diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.fb.js build-after/react-native/implementations/ReactNativeRenderer-profiling.fb.js
10c10
<  * @generated
---
>  * @generated SignedSource<<5ce378a9216ea747d91b208b9fd1ebd5>>
diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.js build-after/react-native/implementations/ReactNativeRenderer-profiling.js
11c11
<  * @generated
---
>  * @generated SignedSource<<1c7564f446ee83142976035b2884dcfd>>
diff --color -r build-before/react-native/shims/ReactFabric.js build-after/react-native/shims/ReactFabric.js
7c7
<  * @format
---
>  * @noformat
8a9,10
>  * @nolint
>  * @generated SignedSource<<cece19ddbec9f287c995721f49c68977>>
diff --color -r build-before/react-native/shims/ReactFeatureFlags.js build-after/react-native/shims/ReactFeatureFlags.js
7c7
<  * @format
---
>  * @noformat
8a9,10
>  * @nolint
>  * @generated SignedSource<<2881c8e89ef0f73f4cf6612cb518b197>>
diff --color -r build-before/react-native/shims/ReactNative.js build-after/react-native/shims/ReactNative.js
7c7
<  * @format
---
>  * @noformat
8a9,10
>  * @nolint
>  * @generated SignedSource<<0debd6e5a17dc037cb4661315a886de6>>
diff --color -r build-before/react-native/shims/ReactNativeTypes.js build-after/react-native/shims/ReactNativeTypes.js
7c7
<  * @format
---
>  * @noformat
8a9,10
>  * @nolint
>  * @generated SignedSource<<652b117c94307244bcf5e4af18928903>>
diff --color -r build-before/react-native/shims/ReactNativeViewConfigRegistry.js build-after/react-native/shims/ReactNativeViewConfigRegistry.js
7c7
<  * @format
---
>  * @noformat
8a9,10
>  * @nolint
>  * @generated SignedSource<<ce82e8957367bee7d11379ab88e3f7c5>>
diff --color -r build-before/react-native/shims/createReactNativeComponentClass.js build-after/react-native/shims/createReactNativeComponentClass.js
7c7
<  * @format
---
>  * @noformat
8a9,10
>  * @nolint
>  * @generated SignedSource<<ede54ac2fa1b9a09e234cdf098048989>>
```
2023-04-14 11:42:48 +02:00
Tianyu Yao d121c67004
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 Susla 7b0642bb98
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
Andrew Clark 8256781fdf
Throttle retries even if everything has loaded (#26611)
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)
2023-04-12 20:25:32 -04:00
Andrew Clark 72c890e312
Convert more Suspense tests to use act (2/n) (#26610)
Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they could
be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree.

I think this covers all the remaining tests that are affected.
2023-04-12 13:36:13 -04:00
Ruslan Lesiutin 21021fb0f0
refactor[devtools]: copy to clipboard only on frontend side (#26604)
Fixes https://github.com/facebook/react/issues/26500

## Summary
- No more using `clipboard-js` from the backend side, now emitting
custom `saveToClipboard` event, also adding corresponding listener in
`store.js`
- Not migrating to `navigator.clipboard` api yet, there were some issues
with using it on Chrome, will add more details to
https://github.com/facebook/react/pull/26539

## How did you test this change?
- Tested on Chrome, Firefox, Edge
- Tested on standalone electron app: seems like context menu is not
expected to work there (cannot right-click on value, the menu is not
appearing), other logic (pressing on copy icon) was not changed
2023-04-12 16:12:03 +01:00
Harry Zumwalt 5426af3d50
Provide icon to edge devtools. (#26543)
<!--
  Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.

Before submitting a pull request, please make sure the following is
done:

1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
  2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
  9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
  10. If you haven't already, complete the CLA.

Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->

## Summary
Addresses #26352.

This PR explicitly passes an icon to `chrome.devtools.panels.create()`,
so that edge devtools will display the icon when in [Focus
Mode](https://learn.microsoft.com/en-us/microsoft-edge/devtools-guide-chromium/experimental-features/focus-mode).

<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->

## How did you test this change?
Passing test suite (`yarn test` & `yarn test --prod`)  
Passing lint (`yarn linc`)  
Passing type checks (`yarn flow`)  

**Visual Testing**

Before Changes             | After Changes
:-------------------------:|:-------------------------:

![](https://user-images.githubusercontent.com/15645169/229591145-fe99df06-e2e3-4f21-ae31-f770d584ca6c.png)
|
![](https://user-images.githubusercontent.com/15645169/229591594-26c6cbaf-f345-4367-b234-8f3c8ab3ccb1.png)
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
  If you leave this empty, your PR will very likely be closed.
-->
2023-04-11 14:42:13 -04:00
Andrew Clark f9de24a26a
Convert more Suspense tests to use `act` (#26602)
Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they could
be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree. This isn't all the
affected tests, just some of them — I'll be sharding the changes across
multiple PRs.
2023-04-11 13:47:29 -04:00
Sebastian Markbåge 6b90976bc1
Use already extracted values instead of reading off props for controlled components (#26596)
Since `props.x` is a possibly megamorphic access, it can be slow to
access and trigger recompilation.

When we are looping over the props and pattern matching every key,
anyway, we've already done this work. We can just reuse the same value
by stashing it outside the loop in the stack.

This only makes sense for updates in diffInCommitPhase since otherwise
we don't have the full set of props in that loop.

We also have to be careful not to skip over equal values since we need
to extract them anyway.
2023-04-11 13:32:47 -04:00
Sebastian Markbåge 343a45ffa4
Remove initOption special case (#26595)
This traces back to https://github.com/facebook/react/pull/6449 and then
another before that.

I think that back then we favored the property over the attribute, and
setting the property wouldn't be enough. However, the default path for
these are now using attributes if we don't special case it. So we don't
need it.

The only difference is that we currently have a divergence for
symbol/function behavior between controlled values that use the
getToStringValue helpers which treat them as empty string, where as
everywhere else they're treated as null/missing.

Since this comes with a warning and is a weird error case, it's probably
fine to change.
2023-04-11 12:39:00 -04:00
Andrew Clark 58742c21b8
Delete unused `eventTimes` Fiber field (#26599) 2023-04-11 08:23:04 -04:00
Andrew Clark 0b931f90e8
Remove JND delay for non-transition updates (#26597)
Updates that are marked as part of a transition are allowed to block a
render from committing. Generally, other updates cannot — however,
there's one exception that's leftover from a previous iteration of our
Suspense architecture. If an update is not the result of a known urgent
event type — known as "Default" updates — then we allow it to suspend
briefly, as long as the delay is short enough that the user won't
notice. We refer to this delay as a "Just Noticable Difference" (JND)
delay. To illustrate, if the user has already waited 400ms for an update
to be reflected on the screen, the theory is that they won't notice if
you wait an additional 100ms. So React can suspend for a bit longer in
case more data comes in. The longer the user has already waited, the
longer the JND.

While we still believe this theory is sound from a UX perspective, we no
longer think the implementation complexity is worth it. The main thing
that's changed is how we handle Default updates. We used to render
Default updates concurrently (i.e. they were time sliced, and were
scheduled with postTask), but now they are blocking. Soon, they will
also be scheduled with rAF, too, which means by the end of the next rAF,
they will have either finished rendering or the main thread will be
blocked until they do. There are various motivations for this but part
of the rationale is that anything that can be made non-blocking should
be marked as a Transition, anyway, so it's not worth adding
implementation complexity to Default.

This commit removes the JND delay for Default updates. They will now
commit immediately once the render phase is complete, even if a
component suspends.
2023-04-11 00:19:49 -04:00
Sebastian Markbåge ac43bf6870
Move validation of text nesting into ReactDOMComponent (#26594)
Extract validateTextNesting from validateDOMNesting. We only need the
parent tag when validating text nodes. Then validate it in setProp.
2023-04-10 21:41:53 -04:00
Sebastian Markbåge ca41adb8c1
Diff properties in the commit phase instead of generating an update payload (#26583)
This removes the concept of `prepareUpdate()`, behind a flag.

React Native already does everything in the commit phase, but generates
a temporary update payload before applying it.

React Fabric does it both in the render phase. Now it just moves it to a
single host config.

For DOM I forked updateProperties into one that does diffing and
updating in one pass vs just applying a pre-diffed updatePayload.

There are a few downsides of this approach:

- If only "children" has changed, we end up scheduling an update to be
done in the commit phase. Since we traverse through it anyway, it's
probably not much extra.
- It does more work in the commit phase so for a large tree that is
mostly unchanged, it'll stall longer.
- It does some extra work for special cases since that work happens if
anything has changed. We no longer have a deep bailout.
- The special cases now have to each replicate the "clean up old props"
loop, leading to extra code.

The benefit is that this doesn't allocate temporary extra objects
(possibly multiple per element if the array has to resize). It's less
work overall. It also gives us an option to reuse this function for a
sync render optimization.

Another benefit is that if we do the loop in the commit phase I can do
further optimizations by reading all props that I need for special cases
in that loop instead of polymorphic reads from props. This is what I'd
like to do in future refactors that would be stacked on top of this
change.
2023-04-10 19:09:28 -04:00
Josh Story dd0619b2ee
rename $$$hostConfig to $$$config (#26593)
We have moved away from HostConfig since the name does not fully
describe the configs we customize per runtime like FlightClient,
FlightServer, Fizz, and Fiber. This commit generalizes $$$hostconfig to
$$$config
2023-04-10 15:01:04 -07:00
Josh Story b55d319559
Rename HostConfig files to FiberConfig to clarify they are configs fo… (#26592)
part of https://github.com/facebook/react/pull/26571

merging separately to improve tracking of files renames in git

Rename HostConfig files to FiberConfig to clarify they are configs for
Fiber and not Fizz/Flight. This better conforms to the naming used in
Flight and now Fizz of `ReactFlightServerConfig` and `ReactFizzConfig`
2023-04-10 14:58:44 -07:00
Josh Story ffb8eaca59
Rename ReactServerFormatConfig to ReactFizzConfig (#26591)
part of https://github.com/facebook/react/pull/26571

merging separately to improve tracking of file renames
2023-04-10 14:54:26 -07:00
Josh Story f4f873f628
Implements wiring for Flight to have it's own "HostConfig" (#26590)
Part of https://github.com/facebook/react/pull/26571

Implements wiring for Flight to have it's own "HostConfig" from Fizz.

Historically the ServerFormatConfigs were supposed to be generic enough
to be used by Fizz and Flight. However with the addition of features
like Float the configs have evolved to be more specific to the renderer.
We may want to get back to a place where there is a pure FormatConfig
which can be shared but for now we are embracing the fact that these
runtimes need very different things and DCE cannot adequately remove the
unused stuff for Fizz when pulling this dep into Flight so we are going
to fork the configs and just maintain separate ones.

At first the Flight config will be almost empty but once Float support
in Flight lands it will have a more complex implementation

Additionally this commit normalizes the component files which make up
FlightServerConfig and FlightClientConfig. Now each file that
participates starts with ReactFlightServerConfig... and
ReactFlightClientConfig...
2023-04-10 14:51:54 -07:00
Josh Story 44db16afc6
Normalize ReactFlightServerConfig and related files (#26589)
First part of https://github.com/facebook/react/pull/26571

merging separately to help with git history with a lot of file renames
2023-04-10 14:47:23 -07:00
Mengdi Chen 451736b557
[DevTools][BE] move shared types & constants to consolidated locations (#26572)
## Summary

This pull request aims to improve the maintainability of the codebase by
consolidating types and constants that are shared between the backend
and frontend. This consolidation will allow us to maintain backwards
compatibility in the frontend in the future.

To achieve this, we have moved the shared types and constants to the
following blessed files:

- react-devtools-shared/src/constants
- react-devtools-shared/src/types
- react-devtools-shared/src/backend/types
- react-devtools-shared/src/backend/NativeStyleEditor/types

Please note that the inclusion of NativeStyleEditor in this list is
temporary, and we plan to remove it once we have a better plugin system
in place.

## How did you test this change?

I have tested it by running `yarn flow dom-node`, which reports no
errors.
2023-04-10 17:07:05 -04:00
Andrew Clark fec97ecbc4
act: Move didScheduleLegacyUpdate to ensureRootIsScheduled (#26552)
`act` uses the `didScheduleLegacyUpdate` field to simulate the behavior
of batching in React <17 and below. It's a quirk leftover from a
previous implementation, not intentionally designed.

This sets `didScheduleLegacyUpdate` every time a legacy root receives an
update as opposed to only when the `executionContext` is empty. There's
no real reason to do it this way over some other way except that it's
how it used to work before #26512 and we should try our best to maintain
the existing behavior, quirks and all, since existing tests may have
come to accidentally rely on it.

This should fix some (though not all) of the internal Meta tests that
started failing after #26512 landed.

Will add a regression test before merging.
2023-04-10 14:40:18 -04:00
Sebastian Markbåge 9a9da7721e
Don't update textarea defaultValue and input checked unnecessarily (#26580)
In #26573 I changed it so that textareas get their defaultValue reset if
you don't specify one.

However, the way that was implemented, it always set it for any update
even if it hasn't changed.

We have a test for that, but that test only works if no properties
update at all so that no update was scheduled. This fixes the test so
that it updates some unrelated prop.

I also found a test for `checked` that needed a similar fix.

Interestingly, we don't do this deduping for `defaultValue` or
`defaultChecked` on inputs and there's no test for that.
2023-04-09 22:16:38 -04:00
Sebastian Markbåge e5146cb525
Refactor some controlled component stuff (#26573)
This is mainly renaming some stuff. The behavior change is
hasOwnProperty to nullish check.

I had a bigger refactor that was a dead-end but might as well land this
part and see if I can pick it up later.
2023-04-09 18:06:16 -04:00
Josh Story 657698e48d
[Tests] `waitForThrow` should diff strings (#26568)
Currently, `waitForThrow` tries to diff the expected value against the
thrown value if it doesn't match. However if the expectation is a
string, we are not diffing against the thrown message. This commit makes
it so if we are matching against message we also diff against message.
2023-04-07 14:14:08 -07:00
Mengdi Chen dd5365878d
[DevTools] remove backend dependency from the global hook (#26563)
## Summary

- #26234 is reverted and replaced with a better approach 
- introduce a new global devtools variable to decouple the global hook's
dependency on backend/console.js, and add it to react-devtools-inline
and react-devtools-standalone

With this PR, I want to introduce a new principle to hook.js: we should
always be alert when editing this file and avoid importing from other
files.
In the past, we try to inline a lot of the implementation because we use
`.toString()` to inject this function from the extension (we still have
some old comments left). Although it is no longer inlined that way, it
has became now more important to keep it clean as it is a de facto
global API people are using (9.9K files contains it on Github search as
of today).


**File size change for extension:**
Before:
379K installHook.js

After:
 21K installHook.js
363K renderer.js
2023-04-07 03:35:36 -04:00
Andrew Clark 85bb7b685b
Fix: Move `destroy` field to shared instance object (#26561)
This fixes the "double free" bug illustrated by the regression test
added in the previous commit.

The underlying issue is that `effect.destroy` field is a mutable field
but we read it during render. This is a concurrency bug — if we had a
borrow checker, it would not allow this.

It's rare in practice today because the field is updated during the
commit phase, which takes a lock on the fiber tree until all the effects
have fired. But it's still theoretically wrong because you can have
multiple Fiber copies each with their own reference to a single destroy
function, and indeed we discovered in production a scenario where this
happens via our current APIs.

In the future these types of scenarios will be much more common because
we will introduce features where effects may run concurrently with the
render phase — i.e. an imperative `hide` method that synchronously hides
a React tree and unmounts all its effects without entering the render
phase, and without interrupting a render phase that's already in
progress.

A future version of React may also be able to run the entire commit
phase concurrently with a subsequent render phase. We can't do this now
because our data structures are not fully thread safe (see: the Fiber
alternate model) but we should be able to do this in the future.

The fix I've introduced in this commit is to move the `destroy` field to
a separate object. The effect "instance" is a shared object that remains
the same for the entire lifetime of an effect. In Rust terms, a RefCell.
The field is `undefined` if the effect is unmounted, or if the effect
ran but is not stateful. We don't explicitly track whether the effect is
mounted or unmounted because that can be inferred by the hiddenness of
the fiber in the tree, i.e. whether there is a hidden Offscreen fiber
above it.

It's unfortunate that this is stored on a separate object, because it
adds more memory per effect instance, but it's conceptually sound. I
think there's likely a better data structure we could use for effects;
perhaps just one array of effect instances per fiber. But I think this
is OK for now despite the additional memory and we can follow up with
performance optimizations later.

---------

Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
Co-authored-by: Rick Hanlon <rickhanlonii@gmail.com>
Co-authored-by: Jan Kassens <jan@kassens.net>
2023-04-06 12:08:05 -04:00
Willie-Boy 60cfeeebe3
[DevTools] Replace deprecated `new-window` with `webContents.setWindowOpenHandler()` (#26559)
## Summary

The electron package was recently upgraded from ^11.1.0 to ^23.1.2
(#26337). However, the WebContents `new-window` event – that is used in
the react-devtools project – was deprecated in
[v12.0.0](https://releases.electronjs.org/release/v12.0.0) and removed
in [v22.2.0](https://releases.electronjs.org/release/v22.2.0). The event
was replaced by `webContents.setWindowOpenHandler()`. This PR replaces
the `new-window` event with `webContents.setWindowOpenHandler()`.

## How did you test this change?

I created a simple electron application with similar functionality:

```
const { app, BrowserWindow, shell } = require('electron')

const createWindow = () => {
  const mainWindow = new BrowserWindow({
    width: 800,
    height: 600
  })

  mainWindow.webContents.setWindowOpenHandler(({ url }) => {
    shell.openExternal(url)
    return { action: 'deny' }
  })

  mainWindow.loadFile('index.html')
}

app.whenReady().then(() => {
  createWindow()
})
```

---------

Co-authored-by: root <root@DESKTOP-KCGHLB8.localdomain>
2023-04-06 10:02:23 -04:00
Sebastian Markbåge 9cfba0f6ec
Clean up discrete event replaying (#26558)
We no longer replay and discrete events.

I might re-add a form of this but it'll look a little different.
2023-04-05 19:38:20 -04:00
Sophie Alpert 790ebc962d
Remove no-fallthrough lint suppressions (#26553)
The lint rule already understands a normal comment. Also a bunch of
these were unnecessary.
2023-04-04 20:08:33 -07:00
Sebastian Markbåge c15579631f
Put common aliases in Map/Set instead of switch over strings (#26551)
This is a follow up to https://github.com/facebook/react/pull/26546

This is strictly a perf optimization since we know that switches over
strings aren't optimally implemented in current engines. Basically
they're a sequence of ifs.

As a result, we're better off putting the unusual cases in a Map and the
very common cases in the beginning of the switch. We might be better off
putting very common cases in explicit ifs - just in case the engine does
optimize switches to a hash table which is potentially worse.

---------

Co-authored-by: Sophie Alpert <git@sophiebits.com>
2023-04-04 18:06:09 -04:00
Mohammad Ghorbani d5fd60f7e6
Remove findInstanceBlockingEvent unused parameters (#26534)
## Summary
First three parameters of `findInstanceBlockingEvent` are unused since I
think if we remove the unused parameters it makes it easier to know that
which parameters is need by `findInstanceBlockingEvent`.

## How did you test this change?
Existing tests.
2023-04-04 12:37:14 -04:00
Sebastian Markbåge eeabb7312f
Refactor DOM Bindings Completely Off of DOMProperty Meta Programming (#26546)
There are four places we have special cases based off the DOMProperty
config:

1) DEV-only: ReactDOMUnknownPropertyHook warns for passing booleans to
non-boolean attributes. We just need a simple list of all properties
that are affected by that. We could probably move this in under setProp
instead and have it covered by that list.
2) DEV-only: Hydration. This just needs to read the value from an
attribute and compare it to what we'd expect to see if it was rendered
on the client. This could use some simplification/unification of the
code but I decided to just keep it simple and duplicated since code size
isn't an issue.
3) DOMServerFormatConfig pushAttribute: This just maps the special case
to how to emit it as a HTML attribute.
4) ReactDOMComponent setProp: This just maps the special case to how to
emit it as setAttribute or removeAttribute.

Basically we just have to remember to keep pushAttribute and setProp
aligned. There's only one long switch in prod per environment.

This just turns it all to a giant simple switch statement with string
cases. This is in theory the most optimizable since syntactically all
the information for a hash table is there. However, unfortunately we
know that most VMs don't optimize this very well and instead just turn
them into a bunch of ifs. JSC is best. We can minimize the cost by just
moving common attribute to the beginning of the list.

If we shipped this, maybe VMs will get it together to start optimizing
this case but there's a chicken and egg problem here and the game theory
reality is that we probably don't want to regress. Therefore, I intend
to do a follow up after landing this which reintroduces an object
indirection for simple property aliases. That should be enough to make
the remaining cases palatable. I'll also extract the most common
attributes to the beginning or separate ifs.

Ran attribute-behavior fixture and the table is the same.
2023-04-04 11:05:56 -04:00
Andrew Clark 0ba4d7b0d8
DevTools: Inline references to fiber flags (#26542)
We shouldn't be referencing internal fields like fiber's `flag` directly
of DevTools. It's an implementation detail. However, over the years a
few of these have snuck in. Because of how DevTools is currently
shipped, where it's expected to be backwards compatible with older
versions of React, this prevents us from refactoring those fields inside
the reconciler.

The plan we have to address this is to fix how DevTools is shipped:
DevTools will be released in lockstep with each version of React.

Until then, though, I need a temporary solution because it's blocking a
feature I'm working on. So in meantime, I'm going to have to fork the
DevTool's code based on the React version, like we already do with the
fiber TypeOfWork enum.

As a first step, I've inlined all the references to fiber flags into the
specific call sites where they are used. Eventually we'll import these
functions from the reconciler so they stay in sync, rather than
maintaining duplicate copies of the logic.
2023-04-04 11:05:33 -04:00
Jan Kassens da94e8b24a
Revert "Cleanup enableSyncDefaultUpdate flag (#26236)" (#26528)
This reverts commit b2ae9ddb3b.

While the feature flag is fully rolled out, these tests are also testing
behavior set with an unstable flag on root, which for now we want to
preserve.

Not sure if there's a better way then adding a dynamic feature flag to
the www build?
2023-04-04 10:08:14 -04:00
Rubén Norte 0700dd50bd
Implement public instances for text nodes in Fabric (#26516)
## Summary

This adds the ability to create public instances for text nodes in
Fabric. The implementation for the public instances lives in React
Native (as it does for host components after #26437). The logic here
just handles their lazy instantiation when requested via
`getPublicInstanceFromInternalInstanceHandle`, which is called by Fabric
with information coming from the shadow tree.

It's important that the creation of public instances for text nodes is
done lazily to avoid regressing memory usage when unused. Instances for
text nodes are left intact if the public instance is never accessed.

This is necessary to implement access to text nodes in React Native as
explained in
https://github.com/react-native-community/discussions-and-proposals/pull/607

## How did you test this change?

Added unit tests (also fixed a test that was only testing the logic in a
mock :S).
2023-04-04 14:43:35 +01:00