For Hookstate Proxies of class instances, `data.constructor.name`
returns `Proxy({})`, so use
`Object.getPrototypeOf(data).constructor.name` instead, which works
correctly from my testing.
<!--
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
React DevTools immediately bricks itself if you inspect any component
that has a prop that is a Hookstate that wraps a class instance ...
because these are proxies where `data.constructor.name` returns some
un-cloneable object, but `Object.getPrototypeOf(data)` doesn't return
`Object` (it returns the prototype of the class inside).
## How did you test this change?
This part of the code has no associated tests at all.
Technically,
`packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js`
exists, but I tried `yarn test` and these tests aren't even executed
anymore. I can't figure it out, so whatever.
If you run this code:
```js
class Class {}
const instance = new Class();
const instanceProxy = new Proxy(instance, {
get(target, key, receiver) {
if (key === 'constructor') {
return { name: new Proxy({}, {}) };
}
return Reflect.get(target, key, receiver);
},
});
```
then `instanceProxy.constructor.name` returns some proxy that cannot be
cloned, but `Object.getPrototypeOf(instanceProxy).constructor.name`
returns the correct value.
This PR fixes the devtools to use
`Object.getPrototypeOf(instanceProxy).constructor.name`.
I modified my local copy of devtools to use this method and it fixed the
bricking that I experienced.
Related #29954
<!--
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
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
When using React Devtools, calling `console.log('%s', null)` in userland
can cause it to throw an error:
```
TypeError: Cannot read properties of null (reading 'toString')
```
## How did you test this change?
Added a unit test.
<!--
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.
-->
See https://github.com/47ng/nuqs/issues/808.
## Summary
In React Native, public instances and internal host nodes are not
represented by the same object (ReactNativeElement & shadow nodes vs.
just DOM elements), and the only one that's required for rendering is
the shadow node. Public instances are generally only necessary when
accessed via refs or events, and that usually happens for a small amount
of components in the tree.
This implements an optimization to create the public instance on demand,
instead of eagerly creating it when creating the host node. We expect
this to improve performance by reducing the logic we do per node and the
number of object allocations.
## How did you test this change?
Manually synced the changes to React Native and run Fantom tests and
benchmarks, with the flag enabled and disabled. All tests pass in both
cases, and benchmarks show a slight but consistent performance
improvement.
## Summary
Fixes#32354.
Re-creation of #15197: adds a dev-only warning if `create == null` to
the three `use*Effect` functions:
* `useEffect`
* `useInsertionEffect`
* `useLayoutEffect`
Updates the warning to match the same text given in the
`react/exhaustive-deps` lint rule.
## How did you test this change?
I applied the changes manually within `node_modules/` on a local clone
of
https://github.com/JoshuaKGoldberg/repros/tree/react-use-effect-no-arguments.
Please pardon me for opening a PR addressing a not-accepted issue. I was
excited to get back to #15194 -> #15197 now that I have time. 🙂
---------
Co-authored-by: lauren <poteto@users.noreply.github.com>
Merges the useResourceEffect API into useEffect while keeping the
underlying implementation the same. useResourceEffect will be removed in
the next diff.
To fork between behavior we rely on a `typeof` check for the updater or
destroy function in addition to the CRUD feature flag. This does now
have to be checked every time (instead of inlined statically like before
due to them being different hooks) which will incur some non-zero amount
(possibly negligble) of overhead for every effect.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32205).
* #32206
* __->__ #32205
Small refactor to the `resource` type to narrow it to an arbitrary
object or void/null instead of the top type. This makes the overload on
useEffect simpler since the return type of create is no longer widened
to the top type when we merge their definitions.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32203).
* #32206
* #32205
* #32204
* __->__ #32203
## Summary
This PR attempts to make running the React DevTools a little friendlier
in projects that are not completely React.
At the moment, running the DevTools with `npx react-devtools` will
default to the port to use the `PORT` env variable otherwise it'll fall
back to `8097`. `PORT` is a common env variable, so we can get into this
strange situation where the a Rails server (eg Puma) is using `PORT`,
and then the React DevTools attempts to boot using the same `PORT`.
This PR introduces a dedicated env variable, `REACT_DEVTOOLS_PORT` to
assist in this scenario.
## How did you test this change?
I'm using fish shell, so I did the following, please let me know if
there's a better way:
```sh
cd packages/react-devtools
set -x PORT 1000
set -x REACT_DEVTOOLS_PORT 2000
node bin.js
```
We can see in the UI that it's listening on `2000`. Without this PR,
it'd listen on `1000`:
Follow-up for #32332. The Discord webhook seems to ignore draft PRs,
which is a good thing. But when a draft PR is then later set to "ready
for review" we do want to send another notification that should not be
filtered out.
Our internal build infra relies on a 1:1 mapping between `main` and the
2 build branches. Directly committing changes to those branches breaks
that infra.
Adds a simple workflow to leave a comment and decline the PR.
There's no real reason to have 2 jobs for sizebot. It's more of a
historical artifact from before the GH migration. Merging them should
require one less worker needing to be provisioned and some of the extra
overhead
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32333).
* __->__ #32333
* #32332
Adds a new Timing logger event to the compiler which currently only
records the walltime of running the compiler from the time the babel
plugin's Program visitor enters to the time it exits.
To enable, run the compiler with `ENABLE_REACT_COMPILER_TIMINGS=1 ...`
or `export ENABLE_REACT_COMPILER_TIMINGS=1` to set it by default.
<!--
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
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
Improve the error message, as the value is currently an object instead
of a string, which results in it being converted to '[object Object]'.
## How did you test this change?
Already tested locally.
<!--
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.
-->
followup to
* https://github.com/facebook/react/pull/32069
* https://github.com/facebook/react/pull/32163
* https://github.com/facebook/react/pull/32224
in react-dom in Dev we validate that the tag nesting is valid. This is
motivated primarily because while browsers are tolerant to poor HTML
there are many cases that if server rendered will be hydrated in a way
that will break hydration.
With the changes to singleton scoping where the document body is now the
implicit render/hydration context for arbitrary tags at the root we need
to adjust the validation logic to allow for valid programs such as
rendering divs as a child of a Document (since this div will actually
insert into the body).
While modern DOM implementations all support getRootNode if you are
running React in a runtime which does not the fallback logic which uses
`.ownerDocument` works everywhere except when the container is a
Document itself. This change corrects this by returning the container
intsance if it is a Document type.
We have this really old (5+ years) feature for inspecting native styles
of React Native Host components.
We also have a custom Cache implementation in React DevTools, which was
forked from React at some point. We know that this should be removed,
but it spans through critical parts of the application, like fetching
and caching inspected element.
Before this PR, this was also used for caching native style and layouts
of RN Host components. This approach is out of date, and was based on
the presence of Suspense boundary around inspected element View, which
we have removed to speed up element inspection -
https://github.com/facebook/react/pull/30555.
Looks like I've introduced a regression in
https://github.com/facebook/react/pull/31956:
- Custom Cache implementation will throw thenables and suspend.
- Because of this, some descendant Suspense boundaries will not resolve
for a long time, and React will throw an error
https://react.dev/errors/482.
I've switched from a usage of this custom Cache implementation to a
naive fetching in effect and keeping the layout and style in a local
state of a Context, which will be propagated downwards. The race should
be impossible, this is guaranteed by the mechanism for queueing messages
through microtasks queue.
The only downside is the UI. If you quickly switch between 2 elements,
and one of them has native style, while the other doesn't, UI will feel
jumpy. We can address this later with a Suspense boundary, if needed.
3 years ago we partially disabled comment nodes as valid containers.
Some unflagged support was left in due to legacy APIs like
`unmountComponentAtNode` and `unstable_renderSubtreeIntoContainer` but
these were since removed in React 19. This update flags the remaining
uses of comments as containers.
follow up to https://github.com/facebook/react/pull/32163
This continues the work of making Suspense workable anywhere in a
react-dom tree. See the prior PRs for how we handle server rendering and
client rendering. In this change we update the hydration implementation
to be able to locate expected nodes. In particular this means hydration
understands now that the default hydration context is the document body
when the container is above the body.
One case that is unique to hydration is clearing Suspense boundaries.
When hydration fails or when the server instructs the client to recover
an errored boundary it's possible that the html, head, and body tags in
the initial document were written from a fallback or a different primary
content on the server and need to be replaced by the client render.
However these tags (and in the case of head, their content) won't be
inside the comment nodes that identify the bounds of the Suspense
boundary. And when client rendering you may not even render the same
singletons that were server rendered. So when server rendering a
boudnary which contributes to the preamble (the html, head, and body tag
openings plus the head contents) we emit a special marker comment just
before closing the boundary out. This marker encodes which parts of the
preamble this boundary owned. If we need to clear the suspense boundary
on the client we read this marker and use it to reset the appropriate
singleton state.
This implements `findSourceMapURL` in react-server-dom-parcel, enabling
source maps for replayed server errors on the client. It utilizes a new
endpoint in the Parcel dev server that returns the source map for a
given bundle/file. The error overlay UI has also been updated to handle
these stacks. See https://github.com/parcel-bundler/parcel/pull/10082
Also updated the fixture to the latest Parcel canary. A few APIs have
changed. We do have a higher level library wrapper now (`@parcel/rsc`
added in https://github.com/parcel-bundler/parcel/pull/10074) but I left
the fixture using the lower level APIs directly here since it is easier
to see how react-server-dom-parcel is used.
<!--
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
Contributing to https://github.com/facebook/react/pull/32240, this
change adds the tsconfig, tsup config, and estree type declarations that
will be needed for that plugin's typescript migration.
Hacky retry pipeline for when transforming `fire(...)` calls encounters
validation, todo, or memoization invariant bailouts. Would love feedback
on how we implement this to be extensible to other compiler
non-memoization features (e.g. inlineJSX)
Some observations:
- Compiler "front-end" passes (e.g. lower, type, effect, and mutability
inferences) should be shared for all compiler features -- memo and
otherwise
- Many passes (anything dealing with reactive scope ranges, scope blocks
/ dependencies, and optimizations such as ReactiveIR #31974) can be left
out of the retry pipeline. This PR hackily skips memoization features by
removing reactive scope creation, but we probably should restructure the
pipeline to skip these entirely on a retry
- We should maintain a canonical set of "validation flags"
Note the newly added fixtures are prefixed with `bailout-...` when the
retry fire pipeline is used. These fixture outputs contain correctly
inserted `useFire` calls and no memoization.
## Summary
When lookup `Parent`, `HostRoot` and `HostPortal` should be merged,
because when creating a `Portal`, it will also include
`containerInfo`(So we can directly use this `containerInfo` to delete
the real DOM nodes.), so there is no need to handle them separately.
## How did you test this change?
No behavior changes, all existing tests pass.
<!--
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
Contributing to https://github.com/facebook/react/pull/32240, this
change adds the dev dependencies needed to support the migration of the
plugin to typescript.
## Summary
Fix typo in dangerfile.js which results in an unreachable code path
which ought to be hit when there is no matching base artifact during
DangerCI automated code review.
See:
221f3002ca/dangerfile.js (L73)
Compare:
221f3002ca/dangerfile.js (L171)
And the case which should hit this code path:
221f3002ca/dangerfile.js (L160)
Given the above context, the condition `Number === Infinity` is clearly
meant to be `decimal === Infinity`, which it will be if the `catch`
statement triggers when there is no matching base artifact. Without this
fix, the primitive value `Infinity` is passed to
`percentFormatter.format(decimal)`, resulting in the string `'+∞%'`.
With this fix, the resulting string will be the intended `'New file'`.
## [Resolves issue
32278](https://github.com/facebook/react/issues/32278)
Addresses https://github.com/facebook/react/issues/32244.
### Chromium
We will use
[chrome.permissions](https://developer.chrome.com/docs/extensions/reference/api/permissions)
for checking / requesting `clipboardWrite` permission before copying
something to the clipboard.
### Firefox
We will keep `clipboardWrite` as a required permission, because there is
no reliable and working API for requesting optional permissions for
extensions that are extending browser DevTools:
- `chrome.permissions` is unavailable for devtools pages -
https://bugzilla.mozilla.org/show_bug.cgi?id=1796933
- You can't call `chrome.permissions.request` from background, because
this instruction has to be executed inside user-event callback,
basically only initiated by user.
I don't really want to come up with solutions like opening a new tab
with a button that user has to click.
When a named ViewTransition component unmounts in one place and mounts
in a different place we need to match these up so we know a pair has
been created. Since the unmounts are tracked in the snapshot phase we
need some way to track the mounts before that.
Originally the way I did that is by reusing the render phase since there
was no other phase in the commit before that. However, that's not quite
correct. Just because something is visited in render doesn't mean it'll
commit. E.g. if that tree ends up suspending or erroring. Which would
lead to a false positive on match. The unmount shouldn't animate in that
case.
(Un)fortunately we have already added a traversal before the snapshot
phase for tracking suspensey CSS. The `accumulateSuspenseyCommit` phase.
This needs to find new mounts of Suspensey CSS or if there was a
reappearing Offscreen boundary it needs to find any Suspensey CSS
already inside that tree. This is exactly the same traversal we need to
find newly appearing View Transition components. So we can just reuse
that.
I just noticed that we don't actually need to let the devtools build
finish first because the e2e tests don't use those built files. We can
decouple them to allow them to run in paralllel.
Building DevTools is currently the long pole for the runtime CI job.
Let's see if we can get the overall runtime for runtime build and test
down by speeding this one step up.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32266).
* #32267
* __->__ #32266
## Summary
`fastAddProperties` has shown some perf benefits when used for creating
props payload for new components. In this PR we'll try to use it for
diffing props for existing components.
It would be good enough if it simply doesn't regress perf. We'll be able
to delete the old `addProperties`, and make `fastAddProperties` the
default behaviour.
## How did you test this change?
```
yarn lint
yarn flow native
yarn test packages/react-native-renderer -r=xplat --variant=false
yarn test packages/react-native-renderer -r=xplat --variant=true
```
