## Summary
Internal cleanup of ReactTestRenderer
## How did you test this change?
`yarn test packages/react/src/__tests__/ReactProfiler-test.internal.js`
## Summary
We need to unblock flipping the default for RTR to be concurrent
rendering. Update ReactDevToolsHooksIntegration-test to use
`unstable_isConcurrent` in place.
## How did you test this change?
`yarn test
packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js`
## Summary
Looks like this was added years ago for instrumentation at meta that
never ended up rolling out. Should be safe to clean up.
## How did you test this change?
`yarn test`
## Summary
This PR is a subset of https://github.com/facebook/react/pull/28425,
which only includes the feature flags that will be configured as
dynamic.
The following list summarizes the feature flag changes:
* RN FB
* Change to Dynamic
* consoleManagedByDevToolsDuringStrictMode
* enableAsyncActions
* enableDeferRootSchedulingToMicrotask
* enableRenderableContext
* useModernStrictMode
## How did you test this change?
Ran the following successfully:
```
$ yarn test
$ yarn flow native
$ yarn flow fabric
```
## Summary
This PR is a subset of https://github.com/facebook/react/pull/28425,
which only includes the feature flags that will be configured as "always
on" (i.e. not "dynamic").
The following list summarizes the feature flag changes:
* RN FB
* Always Enable
* enableCache
* enableCustomElementPropertySupport
* RN OSS
* Always Enable
* disableLegacyContext
* enableCache
* enableCustomElementPropertySupport
* RN Test
* Always Enable
* disableModulePatternComponents
* enableCustomElementPropertySupport
## How did you test this change?
Ran the following successfully:
```
$ yarn test
$ yarn flow native
$ yarn flow fabric
```
## Summary
Internal cleanup of ReactTestRenderer
## How did you test this change?
`yarn test
packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js`
## Summary
Currently, `ReactHooksInspection-test.js` fails because something is
polluting the resulting `Promise` with symbol properties. This changes
the unit test to be more resilient to such symbol properties.
## How did you test this change?
Ran the following successfully:
```
$ yarn test
```
In #23176 we added a special case in completeWork for SuspenseBoundaries
if they still have trailing children. However, that misses a case
because it doesn't log a recoverable error for the hydration mismatch.
So we get an error that we rerendered.
I think this special case was done to avoid contexts getting out of
sync. I don't know why we didn't just move where the pop happens though
so that's what I did here and let the regular pass throw instead. Seems
to be pass the tests.
Stacked on https://github.com/facebook/react/pull/28351, please review
only the last commit.
Top-level description of the approach:
1. Once user selects an element from the tree, frontend asks backend to
return the inspected element, this is where we simulate an error
happening in `render` function of the component and then we parse the
error stack. As an improvement, we should probably migrate from custom
implementation of error stack parser to `error-stack-parser` from npm.
2. When frontend receives the inspected element and this object is being
propagated, we create a Promise for symbolicated source, which is then
passed down to all components, which are using `source`.
3. These components use `use` hook for this promise and are wrapped in
Suspense.
Caching:
1. For browser extension, we cache Promises based on requested resource
+ key + column, also added use of
`chrome.devtools.inspectedWindow.getResource` API.
2. For standalone case (RN), we cache based on requested resource url,
we cache the content of it.
`_debugSource` was removed in
https://github.com/facebook/react/pull/28265.
This PR migrates DevTools to define `source` for Fiber based on
component stacks. This will be done lazily for inspected elements, once
user clicks on the element in the tree.
`DevToolsComponentStackFrame.js` was just copy-pasted from the
implementation in `ReactComponentStackFrame`.
Symbolication part is done in
https://github.com/facebook/react/pull/28471 and stacked on this commit.
There is a weird behaviour in all shells of RDT: when user opens
`Components` tab and scrolls down a tree (without any prior click or
focus event), and then clicks on some element, the `click` event will
not be fired. Because `click` event hasn't been fired, the `focus` event
is fired for the whole list and we pre-select the first (root) element
in the tree:
034130c02f/packages/react-devtools-shared/src/devtools/views/Components/Tree.js (L217-L226)
Check the demo (before) what is happening. I don't know exactly why
`click` event is not fired there, but it only happens:
1. For elements, which were not previously rendered (for virtualization
purposes).
2. When HTML-element (div), which represents the container for the tree
was not focused previously.
Unlike the `click` event, the `mousedown` event is fired consistently.
### Before
https://github.com/facebook/react/assets/28902667/9f3ad75d-55d0-4c99-b2d0-ead63a120ea0
### After
https://github.com/facebook/react/assets/28902667/e34816be-644c-444c-8e32-562a79494e44
Tested that it works in all shells, including the select / deselect
features (with `metaKey` param in event).
Updates the `flushSync` tests to use react-dom instead of ReactNoop.
flushSync is primarily a react-dom API and asserting the implementation
of ReactNoop is not really ideal especially since we are going to be
refactoring flushSync soon to not be implemented in the reconciler
internals.
ReactNoop still have a flushSync api and it can still be used in other
tests that are primarily about testing other functionlity and use
ReactNoop as the renderer.
The idea here is that host dispatchers are not bound to renders so we
need to be able to dispatch to them at any time. This updates the
implementation to chain these dispatchers so that each renderer can
respond to the dispatch. Semantically we don't always want every
renderer to do this for instance if Fizz handles a float method we don't
want Fiber to as well so each dispatcher implementation can decide if it
makes sense to forward the call or not. For float methods server
disaptchers will handle the call if they can resolve a Request otherwise
they will forward. For client dispatchers they will handle the call and
always forward. The choice needs to be made for each dispatcher method
and may have implications on correct renderer import order. For now we
just live with the restriction that if you want to use server and client
together (such as renderToString in the browser) you need to import the
server renderer after the client renderer.
Adds a flag to disable legacy mode. Currently this flag is used to cause
legacy mode apis like render and hydrate to throw. This change also
removes render, hydrate, unmountComponentAtNode, and
unstable_renderSubtreeIntoContainer from the experiemntal entrypoint.
Right now for Meta builds this flag is off (legacy mode is still
supported). In OSS builds this flag matches __NEXT_MAJOR__ which means
it currently is on in experiemental. This means that after merging
legacy mode is effectively removed from experimental builds. While this
is a breaking change, experimental builds are not stable and users can
pin to older versions or update their use of react-dom to no longer use
legacy mode APIs.
The runtime contains a type check to determine if a user-provided ref is
a valid type — a function or object (or a string, when
`disableStringRefs` is off). This currently happens during child
reconciliation. This changes it to happen only when the ref is passed to
the component that the ref is being attached to.
This is a continuation of the "ref as prop" change — until you actually
pass a ref to a HostComponent, class, etc, ref is a normal prop that has
no special behavior.
Depends on:
- https://github.com/facebook/react/pull/28398
---
This removes string refs, which has been deprecated in Strict Mode for
seven years.
I've left them behind a flag for Meta, but in open source this fully
removes the feature.
## Summary
`isInputPending` is not in use. This PR cleans up the flags controlling
its gating and parameters to simplify Scheduler.
Makes `frameYieldMs` feature flag static, set to 10ms in www, which we
found built on the wins provided by a broader yield interval via
`isInputPending`. Flag remains set to 5ms in OSS builds.
## How did you test this change?
`yarn test Scheduler`
If there's invalid dom nesting, there will be mismatches following but
the nesting is the most important cause of the problem.
Previously we would include the DOM nesting when rerendering thanks to
the new model of throw and recovery. However, the log would come during
the recovery phase which is after we've already logged that there was a
hydration mismatch.
People would consistently miss this log. Which is fair because you
should always look at the first log first as the most probable cause.
This ensures that we log in the hydration phase if there's a dom nesting
issue. This assumes that the consequence of nesting will appear such
that the won't have a mismatch before this. That's typically the case
because the node will move up and to be a later sibling. So as long as
that happens and we keep hydrating depth first, it should hold true.
There might be an issue if there's a suspense boundary between the nodes
we'll find discover the new child in the outer path since suspense
boundaries as breadth first.
Before:
<img width="996" alt="Screenshot 2024-02-23 at 7 34 01 PM"
src="https://github.com/facebook/react/assets/63648/af70cf7f-898b-477f-be39-13b01cfe585f">
After:
<img width="853" alt="Screenshot 2024-02-23 at 7 22 24 PM"
src="https://github.com/facebook/react/assets/63648/896c6348-1620-4f99-881d-b6069263925e">
Cameo: RSC stacks.
This pattern is a petpeeve of mine. I don't consider this best practice
and so most don't have these prefixes. Very inconsistent.
At best this is useless and noisey that you have to parse because the
information is also in the stack trace.
At worse these are misleading because they're highlighting something
internal (like validateDOMNesting) which even suggests an internal bug.
Even the ones public to React aren't necessarily what you called because
you might be calling a wrapper around it.
That would be properly reflected in a stack trace - which can also
properly ignore list so that the first stack you see is your callsite,
Which might be like `render()` in react-testing-library rather than
`createRoot()` for example.
I'm a bit ambivalent about this one because it's not the main strategy
that I plan on pursuing. I plan on replacing most DEV-only specific
stacks like `console.error` stacks with a new take on owner stacks and
native stacks. The future owner stacks may or may not be exposed to
error boundaries in DEV but if they are they'd be a new errorInfo
property since they're owner based and not available in prod.
The use case in `console.error` mostly goes away in the future so this
PR is mainly for error boundaries. It doesn't hurt to have it in there
while I'm working on the better stacks though.
The `componentStack` property exposed to error boundaries is more like
production behavior similar to `new Error().stack` (which even in DEV
won't ever expose owner stacks because `console.createTask` doesn't
affect these). I'm not sure it's worth adding server components in DEV
(this PR) because then you have forked behavior between dev and prod.
However, since even in the future there won't be any other place to get
the *parent* stack, maybe this can be useful information even if it's
only dev. We could expose a third property on errorInfo that's DEV only
and parent stack but including server components. That doesn't seem
worth it over just having the stack differ in dev and prod.
I don't plan on adding line/column number to these particular stacks.
A follow up could be to add this to Fizz prerender too but only in DEV.
