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.
[`tsup`](https://github.com/egoist/tsup) is esbuild based so build performance is comparable. It it is a little bit slower now because we also generate d.ts files, but it's still much faster than rollup which we used prior to esbuild.
```
# BEFORE: build all compiler packages
$ 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
$ time yarn build
✨ 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.
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.
[`tsup`](https://github.com/egoist/tsup) is esbuild based so build performance is comparable. It it is a little bit slower now because we also generate d.ts files, but it's still much faster than rollup which we used prior to esbuild.
```
# build all compiler packages
$ time yarn build
✨ 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.
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.
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.
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 #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.
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.
<!--
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.
In #32416, @michaelfaith discovered that https://github.com/facebook/react/blob/029e8bd618af23fbdd9efdac565ad81f7d4640d8/scripts/jest/setupTests.js#L82 would fail on the merged compiler eslint rule.
Upon further investigation I discovered that our CompilerError implementation doesn't play nicely with Reflect, so `Reflect.construct` would fail as `.toString` would error:
```
TypeError: Cannot read properties of undefined (reading 'map')
at _CompilerError.toString (/Users/.../code/react/node_modules/babel-plugin-react-compiler/src/CompilerError.ts:200:25)
at _CompilerError.get message [as message] (/Users/.../code/react/node_modules/babel-plugin-react-compiler/src/CompilerError.ts:194:17)
at Object.construct (/Users/.../code/react/scripts/jest/setupTests.js:153:52)
```
I tested this fix using my local copy of the built babel-plugin-react-compiler to verify that this fixes those tests.
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.
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.
laurenandGitHub
RickyandGitHub
Mathias StangandGitHub
Sebastian "Sebbie" SilbermannandGitHub
Sebastian MarkbågeandGitHub
Keith CirkelandGitHub
michael faithandGitHub
Jordan BrownandGitHub
