This adds a page to the DOM fixture to test Fragment Refs. The first
test case is for `addEventListener`/`removeEventListener`.
Setting `enableFragmentRefs` to `__EXPERIMENTAL__` and building is
required to run the fixture.
<img width="872" alt="Screenshot 2025-03-05 at 12 58 57 PM"
src="https://github.com/user-attachments/assets/fee498b7-fd96-4178-9e82-c46d4cb55c9b"
/>
I'm not sure what exactly is causing the flakiness in the playground e2e
tests but I suspect it's some kind of timing issue.
Let's try waiting for Monaco to be fully initialized before running
tests.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32584).
* __->__ #32584
* #32583
Extracting portions of #32416 for easier review. This PR dedupes
@babel/types to resolve to 7.26.3, for compatibility in the root
workspace where eslint-plugin-react-hooks resides.
I also needed to update @babel/preset-typescript in snap.
The compiler changes in HIR and ReactiveScopes were needed due to types
changing. Notably, Babel [added support for optional chaining
assignment](https://github.com/babel/babel/pull/15751) (currently [Stage
1](https://github.com/tc39/proposal-optional-chaining-assignment)), so
in the latest versions of @babel/types, AssignmentExpression.left can
now also be of t.OptionalMemberExpression.
Given that this is in Stage 1, the compiler probably shouldn't support
this syntax, so this PR updates HIR to bailout with a TODO if there is a
non LVal on the lhs of an Assignment Expression.
There was also a small superficial SourceLocation change needed in
`InferReactiveScopeVariables` as Babel 8 changes were [accidentally
released in
7](https://github.com/babel/babel/issues/10746#issuecomment-2699146670).
It doesn't affect our analysis so it seems fine to just update with the
new properties.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32581).
* #32582
* __->__ #32581
Co-authored-by: michael faith <michaelfaith@users.noreply.github.com>
Co-authored-by: michael faith <michaelfaith@users.noreply.github.com>
Follow up to #32540.
We do allow gestures to be cancelled early (we call skipTransition) if
the gesture stops before it has even started.
This happens in the fixture when we auto-scroll.
*This API is experimental and subject to change or removal.*
This PR is an alternative to
https://github.com/facebook/react/pull/32421 based on feedback:
https://github.com/facebook/react/pull/32421#pullrequestreview-2625382015
. The difference here is that we traverse from the Fragment's fiber at
operation time instead of keeping a set of children on the
`FragmentInstance`. We still need to handle newly added or removed child
nodes to apply event listeners and observers, so we treat those updates
as effects.
**Fragment Refs**
This PR extends React's Fragment component to accept a `ref` prop. The
Fragment's ref will attach to a custom host instance, which will provide
an Element-like API for working with the Fragment's host parent and host
children.
Here I've implemented `addEventListener`, `removeEventListener`, and
`focus` to get started but we'll be iterating on this by adding
additional APIs in future PRs. This sets up the mechanism to attach refs
and perform operations on children. The FragmentInstance is implemented
in `react-dom` here but is planned for Fabric as well.
The API works by targeting the first level of host children and proxying
Element-like APIs to allow developers to manage groups of elements or
elements that cannot be easily accessed such as from a third-party
library or deep in a tree of Functional Component wrappers.
```javascript
import {Fragment, useRef} from 'react';
const fragmentRef = useRef(null);
<Fragment ref={fragmentRef}>
<div id="A" />
<Wrapper>
<div id="B">
<div id="C" />
</div>
</Wrapper>
<div id="D" />
</Fragment>
```
In this case, calling `fragmentRef.current.addEventListener()` would
apply an event listener to `A`, `B`, and `D`. `C` is skipped because it
is nested under the first level of Host Component. If another Host
Component was appended as a sibling to `A`, `B`, or `D`, the event
listener would be applied to that element as well and any other APIs
would also affect the newly added child.
This is an implementation of the basic feature as a starting point for
feedback and further iteration.
This fixes a critical issue with moveBefore. I was told that the
disconnected -> connected case was going to be relaxed and not be an
error but apparently that is not the case.
This means that we can't use this for initial insertions. Only moves.
Unfortunately React's internals doesn't distinguish these cases. This
adds a hack that checks each nodes but this is pretty bad for
performance. We should only call this in one or the other case.
Given that we still need feature detection. Both of which means that
these calls are no longer inlined and this extra code. I wonder if it's
even worth it given that you can't even rely on it working anyway since
not all browsers have it. Kind of don't want to ship this until all
browsers have it.
Even then we'd ideally refactor React to use separate code paths for
initial insertion vs moves. Which leads to some unfortunate code
duplication.
Enabling feature detection of early DOM features in a framework is
reckless. I'm not judging other frameworks (but also a little bit).
Because if you do something like `if (moveBefore) moveBefore(a, b) else
insertBefore(a, b)` like we do and then the implementation has to change
there are still too many websites out there that it becomes impossible
to change it. It would break the web. It would instead have to change to
a different name. That's what happened with `contains` -> `includes`.
Counter to popular belief it didn't have anything to do with patching
prototypes. Therefore, ideally frameworks shouldn't start rely on it
until there's two implementations so that there's time for feedback.
That's why we didn't immediately enable this even in experimental.
However, at this point there's probably enough feature detection and it
has shipped long enough in Chrome that it's unlikely to be able to
change at this point.
We can enable it now. For now just in `@experimental` to see if we can
flush out issues with it before bringing it to stable.
Otherwise these can survive into the next View Transition and cause
havoc to that transition.
This was appearing as a flash in Safari in the fixture when going from
A->B. This triggers a View Transition and at the same time the scroll
position updates in an effect. That fires a scroll event which starts a
gesture. This shouldn't really happen and the SwipeRecognizer should
ideally ignore those but it's good to surface edge cases. That gesture
is blocked on the View Transition finishing and then immediately after
it starts a gesture View Transition. That gesture then picked up the
former Animation from the previous transition which caused issues. This
PR fixes that flash.
Currently in the `compiler` workspace, we invoke esbuild directly to
build most packages (with the exception of `snap`). This has been mostly
fine, but does not allow us to do things like generate type declaration
files.
I would like #32416 to be able to consume the merged
eslint-plugin-react-compiler from source rather than via npm, and one of
the things that has come up from my exploration in that stack using the
compiler from source is that babel-plugin-react-compiler is missing type
declarations. This is primarily because React's build process uses
rollup + rollup-plugin-typescript, which runs tsc. So the merged plugin
needs to typecheck properly in order to build. An alternative might be
to migrate to something like babel with rollup instead to simply strip
types rather than typecheck before building. The minor downside of that
approach is that we would need to manually maintain a d.ts file for
eslint-plugin-react-hooks. For now I would like to see if this PR helps
us make progress rather than go for the slightly worse alternative.
[`tsup`](https://github.com/egoist/tsup) is esbuild based so build
performance is comparable. It is slower when generating d.ts files, but
it's still much faster than rollup which we used prior to esbuild. For
now, I have turned off `dts` by default, and it is only passed when
publishing on npm.
If you want to also generate d.ts files you can run `yarn build --dts`.
```
# BEFORE: build all compiler packages (esbuild)
$ time yarn build
✨ Done in 15.61s.
yarn build 13.82s user 1.54s system 96% cpu 15.842 total
# ---
# AFTER: build all compiler packages (tsup)
$ time yarn build
✨ Done in 12.39s.
yarn build 12.58s user 1.68s system 106% cpu 13.350 total
# ---
# AFTER: build all compiler packages and type declarations (tsup)
$ time yarn build --dts
✨ Done in 30.69s.
yarn build 43.57s user 3.20s system 150% cpu 31.061 total
```
I still need to test if this unblocks #32416 but this stack can be
landed independently though as we could probably just release type
declarations on npm. No one should be using the compiler directly, but
if they really wanted to, lack of type declarations would not stop them
(cf React secret internals).
Note that I still kept esbuild as we still use it directly for forgive.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32550).
* #32551
* __->__ #32550
<!--
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?
-->
This PR fixes asserts when `passChildrenWhenCloningPersistedNodes` is
enabled for React Native and OffscreenComponent child rendering unhides
host components.
Discussions around possible fixes for the asserts seen in React Native
suggested changing the way we handle hiding/unhiding host components by
updating the fiber state with the hidden host component instead of
submitting a hidden clone Fabric and keeping the original as the current
fiber.
Implementing this fix would require holding onto the original styling of
the hidden host component. The reconciler updates the styling by adding
`display: none` to hide the contents. If the original host component was
already hidden, the renderer would lose that information and remove the
styling when showing the contents again.
To reduce the changes required to make
`passChildrenWhenCloningPersistedNodes` work, this PR falls back to the
original cloning method when OffscreenComponents are part of the
children needed to be added back. This effectively resolve the asserts
triggered by the feature in RN and improves overall performance.
## How did you test this change?
<!--
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.
-->
This fix was tested by enabling `passChildrenWhenCloningPersistedNodes`
in an app built with React Native that had a repro for triggering the
asserts. The asserts do not occur anymore when using the changes in this
PR.
---------
Co-authored-by: Nick <lefever@meta.com>
<!--
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?
-->
Adds changelog entries for the last two minor releases of
`eslint-plugin-react-hooks`. Fixes#31717.
I chose to not include #31208 (8382581446)
and #32115 (fd2d279984) in the changelog
as they only changed internals that do not affect consumers of the
plugin, and it doesn't seem like the changelog previously included such
changes.
Changes are sorted by importance (rather than by commit date), with the
most important changes first.
## How did you test this change?
<!--
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.
-->
Docs only, nothing to test.
This is used to register Server References that exist in the current
environment but also exists in the server it might call into. Such as a
remote server.
If the value comes from the remote server in the first place then this
is called automatically to ensure that you can pass a reference back to
where it came from - even if the `serverModuleMap` option is used. This
was already the case when `serverModuleMap` wasn't passed. This is how
you can pass server references back to the server. However, when we
added `serverModuleMap` that pass was skipped because we were getting
real functions instead of proxies.
For functions that wasn't yet passed from the remote server to the
current server, we can register them eagerly just like we do for
`import('/server').registerServerReference()`. You can now also do this
with `import('/client').registerServerReference()`. We could make them
shared so you only have to do this once but it might not be possible to
pass to the remote server and the remote server might not even be the
same RSC renderer. Therefore I split them. It's up to the compiler
whether it should do that or not. It has to know that any function you
might call might be able to receive it. This is currently global to a
specific RSC renderer.
Setting the animation's currentTime causes a quirk where the transition
can end up off by a bit and the end state can be slightly off the end
time.
However, I realized that we don't have to because if we just set the
direction in the `animate()` call directly the Safari bug goes away.
This is really the essence mechanism of the `useSwipeTransition`
feature.
We don't want to immediately switch to the destination state when
starting a gesture. The effects remain mounted on the current state. We
want the current state to be "live". This is important to for example
allow a video to keeping playing while starting a swipe (think
TikTok/Reels) and not stop until you've committed the action. The only
thing that can be live is the "new" state. Therefore we treat the
destination as the "old" state and perform a reverse animation from
there.
Ideally we could apply the old state to the DOM tree, take a snapshot
and then revert it back in the mutation of `startViewTransition`.
Unfortunately, the way `startViewTransition` was designed it always
paints one frame of the "old" state which would lead this to cause a
flicker.
To work around this, we need to create a clone of any View Transition
boundary that might be mutated and then render that offscreen. That way
we can render the "current" state on screen and the "destination" state
offscreen for the screenshots. Being mutated can be either due to React
doing a DOM mutation or if a child boundary resizes that causes the
parent to relayout. We don't have to do this for insertions or deletions
since they only appear on one side.
The worst case scenario is that we have to clone the whole root. That's
what this first PR implements. We clone the container and if it's not
absolutely positioned, we position it on top of the current one. If the
container is `document` or `<html>` we instead clone the `<body>` tag
since it's the only one we can insert a duplicate of. If the container
is deep in the tree we clone just that even though technically we should
probably clone the whole document in that case. We just keep the impact
smaller. Ideally though we'd never hit this case. In fact, if we clone
the document we issue a warning (always for now) since you probably
should optimize this. In the future I intend to add optimizations when
affected View Transition boundaries are absolutely positioned since they
cannot possibly relayout the parent. This would be the ideal way to use
this feature most efficiently but it still works without it.
Since we render the "old" state outside the viewport, we need to then
adjust the animation to put it back into the viewport. This is the
trickiest part to get right while still preserving any customization of
the View Transitions done using CSS. This current approach reapplies all
the animations with adjusted keyframes.
In the case of an "exit" the pseudo-element itself is positioned outside
the viewport but since we can't programmatically update the style of the
pseudo-element itself we instead adjust all the keyframes to put it back
into the viewport. If there is no animation on the group we add one.
In the case of an "update" the pseudo-element is positioned on the new
state which is already inside the viewport. However, the auto-generated
animation of the group has a starting keyframe that starts outside the
viewport. In this case we need to adjust that keyframe.
In the future I might explore a technique that inserts stylesheets
instead of mutating the animations. It might be simpler. But whatever
hacks work to maximize the compatibility is best.
This change adds more details about prior versions of the plugin's
config, to help people as they migrate from legacy to flat configs
across multiple versions of this plugin. At some point in the 6.0 or 7.0
cycle, it would probably make sense to re-consolidate this into a single
version.
Closes#32494
Summary: Correctly supports React.useEffect when React is
imported as `import * as React from 'react'`
(as well as other namespaces as specified in the config).
We added support for `onScrollEnd` in #26789 but it only works in Chrome
and Firefox. Safari still doesn't support `scrollend` and there's no
indication that they will anytime soon so this polyfills it.
While I don't particularly love our synthetic event system this tries to
stay within the realm of how our other polyfills work. This implements
all `onScrollEnd` events as a plugin.
The basic principle is to first feature detect the `onscrollend` DOM
property to see if there's native support and otherwise just use the
native event.
Then we listen to `scroll` events and set a timeout. If we don't get any
more scroll events before the timeout we fire `onScrollEnd`. Basically
debouncing it. If we're currently pressing down on touch or a mouse then
we wait until it is lifted such as if you're scrolling with a finger or
using the scrollbars on desktop but isn't currently moving.
If we do get any native events even though we're in polyfilling mode, we
use that as an indication to fire the `onScrollEnd` early.
Part of the motivation is that this becomes extra useful pair for
https://github.com/facebook/react/pull/32422. We also probably need
these events to coincide with other gesture related internals so you're
better off using our polyfill so they're synced.
I end up rebuilding for testing the view-transition fixture a lot. It
doesn't need everything that flight needs so this just adds a short hand
that's a little faster to rebuild.
---------
Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
Randomly noticed this when I looked at a recent [DevTools regression
test run](https://github.com/facebook/react/actions/runs/13578385011).
I don't recall why we added `continue-on-error` previously, but I
believe it was to keep all jobs in the matrix running even if one were
to fail, in order to fully identify any failures from code changes like
build or test failures.
There is now a `fail-fast` option which does this.
[`continue-on-error`](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idcontinue-on-error)
now means:
> Prevents a workflow run from failing when a job fails. Set to true to
allow a workflow run to pass when this job fails.
so it's not correct to use it.
This change swaps which config `recommended` is aliasing. In 5.2.0, the
new flat config was introduced as `recommended-latest`, while
`recommended` still pointed at the legacy rc-based config, with a note
that in the next major version `recommended` would be updated to point
at `recommend-latest`. This change makes that swap, and make the default
`recommended` experience the flat config. To continue using the legacy
rc recommended config, please make the following change in your config
```diff
- extends: ['plugin:react-hooks/recommended']
+ extends: ['plugin:react-hooks/recommended-legacy']
```
This change also deprecates `recommended-latest` in favor of
`recommended`. `recommended-latest` will be removed in a future major
version.
The README has been updated to reflect the new usage, and to put the
flat config sections before the legacy config sections.
I also took the opportunity to change the v9 fixture to use a typescript
config, serving as a demonstration for usage as well as a way to
validate the types are correct.
BREAKING CHANGE
---------
Co-authored-by: lauren <poteto@users.noreply.github.com>
Since the compiler plugin is going to be merged into the hooks plugin,
and ultimately decomposed into several more rules, it would be good to
start creating a more traditional folder structure for the plugin. This
change just moves the rules into a `rules` folder.
Co-authored-by: lauren <poteto@users.noreply.github.com>
In preparation for the merging of the compiler plugin into this one
(#32416), this change proactively updates the plugin's `engines`
declaration to require Node versions greater than or equal to 18
BREAKING CHANGE
Co-authored-by: lauren <poteto@users.noreply.github.com>
This adds a `ReactFiberApplyGesture` which is basically intended to be a
fork of the phases in `ReactFiberCommitWork` except for the fake commit
that `useSwipeTransition` does. So far none of the phases are actually
implemented yet. This is just the scaffolding around them so I can fill
them in later.
The important bit is that we call `startViewTransition` (via the
`startGestureTransition` Config) when a gesture starts. We add a paused
animation to prevent the transition from committing (even if the
ScrollTimeline goes to 100%). This also locks the documents so that we
can't commit any other Transitions until it completes.
When the gesture completes (scroll end) then we stop the gesture View
Transition. If there's no new work scheduled we do that immediately but
if there was any new work already scheduled, then we assume that this
will potentially commit the new state. So we wait for that to finish.
This lets us lock the animation in its state instead of snapping back
and then applying the real update.
Using this technique we can't actually run a View Transition from the
current state to the actual committed state because it would snap back
to the beginning and then run the View Transition from there. Therefore
any new commit needs to skip View Transitions even if it should've
technically animated to that state. We assume that the new state is the
same as the optimistic state you already swiped to. An alternative to
this technique could be to commit the optimistic state when we cancel
and then apply any new updates o top of that. I might explore that in
the future.
Regardless it's important that the `action` associated with the swipe
schedules some work before we cancel. Otherwise it risks reverting
first. So I had to update this in the fixture.
It's getting unwieldy to list every single package to skip in these
commands when you only want to publish one, ie
eslint-plugin-react-hooks.
This adds a new `onlyPackages` and `publishVersion` option to the
publish commands to make that easier.
This doesn't change anything. It just moves some functions.
This moves the view transitions helper functions into its own file. This
is similar to how I already moved ReactFiberCommitEffects and
ReactFiberCommitHostEffects out of ReactFiberCommitWork.
This makes it a bit easier to navigate and get an overview of
ReactFiberCommitWork but another motivation is also so that I can refer
to these helpers from
[ReactFiberApplyGesture](https://github.com/facebook/react/pull/32451/files#diff-42297cf327dee8e01d83c85314b8965953b9674e7c4615ce6c430464dcc8550b).
For the `useId` algorithm we used colon `:` before and after.
https://github.com/facebook/react/pull/23360
This avoids collisions in general by using an unusual characters. It
also avoids collisions when concatenated with some other ID.
Unfortunately, `:` is not a valid character in `view-transition-name`.
This PR swaps the format from:
```
:r123:
```
To the unicode:
```
«r123»
```
Which is valid CSS selectors. This also allows them being used for
`querySelector()` which we didn't really find a legit use for but seems
ok-ish.
That way you can get a view-transition-name that you can manually
reference. E.g. to generate styles:
```js
const id = useId();
return <>
<style>{`
::view-transition-group(${id}) { ... }
::view-transition-old(${id}) { ... }
::view-transition-new(${id}) { ... }
`}</style>
<ViewTransition name={id}>...</ViewTransition>
</>;
```
Lauren Tan
Jack Pope
mofeiZ
Sebastian Markbåge
Hendrik Liebau
Nick Lefever
Ricky
Mathias Stang
Sebastian "Sebbie" Silbermann
Keith Cirkel
michael faith
Jordan Brown