We did a bunch of refactors to ReactFlightClient in PRs like #29823 and #33664.
This brings a bunch of those related refactors to the equivalent FlightReplyServer. Such as deep resolution of cycles and deferred error handling.
`react-stack-bottom-frame` -> `react_stack_bottom_frame`.
This survives `@babel/plugin-transform-function-name`, but now frames
will be displayed as `at Object.react_stack_bottom_frame (...)` in V8.
Checks that were relying on exact function name match were updated to
use either `.indexOf()` or `.includes()`
For backwards compatibility, both React DevTools and Flight Client will
look for both options. I am not so sure about the latter and if React
version is locked.
When we get the source location for "View source for this element" we
should be using the enclosing function of the callsite of the child. So
that we don't just point to some random line within the component.
This is similar to the technique in #33136.
This technique is now really better than the fake throw technique, when
available. So I now favor the owner technique. The only problem it's
only available in DEV and only if it has a child that's owned (and not
filtered).
We could implement this same technique for the error that's thrown in
the fake throwing solution. However, we really shouldn't need that at
all because for client components we should be able to call
`inspect(fn)` at least in Chrome which is even better.
Enables the `enableEagerAlternateStateNodeCleanup` feature flag for all
variants, while maintaining the `__VARIANT__` for the internal React
Native flavor for backtesting reasons.
```
$ yarn test
```
`pull_request_target` gives access to repository secrets and permissions
for use from forks, for example to add a comment.
> Due to the dangers inherent to automatic processing of PRs, GitHub’s
standard pull_request workflow trigger by default prevents write
permissions and secrets access to the target repository. However, in
some scenarios such access is needed to properly process the PR. To this
end the pull_request_target workflow trigger was introduced.
> The reason to introduce the pull_request_target trigger was to enable
workflows to label PRs (e.g. needs review) or to comment on the PR.
(via
https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/)
In this case there is no reason for us to allow this, so let's just use
the normal `pull_request` trigger which is less permissive.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32708).
* __->__ #32708
* #32709
Followup from https://github.com/facebook/react/pull/32499
Manual mode is unused and has some bugs such as revealing hidden
boundaries when manually toggling. We also want to change how manual
mode works, and do some refactors to Activity to make it easier to
support. For now we'll remove it, then add it back after the other
changes we have planned.
This works around this Safari bug.
https://bugs.webkit.org/show_bug.cgi?id=290146
This unfortunate because it may cause additional layouts if there's more
updates to the tree coming by manual mutation before it gets painted
naturally. However, we might end up wanting to read layout early anyway.
This affects the fixture because we clone the `<link>` from the `<head>`
which is itself another bug. However, it should be possible to have
`<link>` tags inserted into the new tree so this is still relevant.
Partially reverts #32686.
PR caches inherit from caches generated in `main`. If it cannot find
that cache, it will create one scoped to just that PR (and PRs that
inherit from it).
There is an edge case where cache eviction can happen in the middle of a
test run. If cache eviction removes a `main` cache, child jobs that
depend on it will start failing because of the `fail-on-cache-miss`
setting.
This PR reverts the default behavior. If this happens, the workflow will
still continue in slow mode where it will `yarn install` child jobs
instead of reusing from cache. This is slower but will at least allow
workflows to continue.
Additionally I added restore keys so that we can fallback to other
caches if present so `yarn install` doesn't need to start over from
scratch.
Updates ~all of our validations to return a Result, and then updates callers to either unwrap() if they should bailout or else just log.
ghstack-source-id: 418b5f5aa2b7dd49ca76b3f98a48a35150691d7e
Pull Request resolved: https://github.com/facebook/react/pull/32688
React uses function identity to determine whether a given JSX expression represents the same type of component and should reconcile (keep state, update props) or replace (teardown state, create a new instance). This PR adds off-by-default validation to check that developers are not dynamically creating components during render.
The check is local and intentionally conservative. We specifically look for the results of call expressions, new expressions, or function expressions that are then used directly (or aliased) as a JSX tag. This allows common sketchy but fine-in-practice cases like passing a reference to a component from a parent as props, but catches very obvious mistakes such as:
```js
function Example() {
const Component = createComponent();
return <Component />;
}
```
We could expand this to catch more cases, but this seems like a reasonable starting point. Note that I tried enabling the validation by default and the only fixtures that error are the new ones added here. I'll also test this internally. What i'm imagining is that we enable this in the linter but not the compiler.
ghstack-source-id: e7408c0a55478b40d65489703d209e8fa7205e45
Pull Request resolved: https://github.com/facebook/react/pull/32683
Since we use a centralized cache we should fail subsequent steps if the
child jobs are unable to restore the cache from the first 2 jobs.
Also fix some incorrect hashes used for the fixture tests.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32686).
* __->__ #32686
* #32685
There was a bug previously in our commit artifacts step where the
emitted REVISION hash would reference the commit on the builds branch
rather than from `main`.
Given that our internal manual sync script also does this, let's align
them both to always reference the commit from `main` instead.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32680).
* __->__ #32680
* #32679
* #32678
Defaults to warn, but since some steps require these artifacts to be
uploaded we specify an error if its not found. Some other steps like
playwright test-results are only uploaded on failure so it's okay to
ignore.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32679).
* #32680
* __->__ #32679
* #32678
> Caches have branch scope restriction in place. This means that if
caches for a specific branch are using a lot of storage quota, it may
result into more frequently used caches from default branch getting
thrashed. For example, if there are many pull requests happening on a
repo and are creating caches, these cannot be used in default branch
scope but will still occupy a lot of space till they get cleaned up by
eviction policy. But sometime we want to clean them up on a faster
cadence so as to ensure default branch is not thrashing.
https://github.com/actions/cache/blob/main/tips-and-workarounds.md#force-deletion-of-caches-overriding-default-cache-eviction-policy
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32675).
* __->__ #32675
* #32674
To avoid race conditions where multiple jobs try to write to the same
cache, we now centralize saving the cache and then reusing it in every
subsequent job.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32672).
* #32675
* #32674
* __->__ #32672
## Summary
In the recommended configuration for `eslint-plugin-react-compiler`,
i.e. `reactCompiler.configs.recommended`, the rule is typed as `string`
rather than `eslint.Linter.RuleEntry` or anything assignable thereto,
which results in the following type error if you type check your eslint
configuration:
```
Property ''react-compiler/react-compiler'' is incompatible with index signature.
Type 'string' is not assignable to type 'RuleEntry | undefined'.
```
Simply adding a const assertion fixes the error.
## How did you test this change?
I emitted declarations for the module and confirmed that the rule is now
typed as the string literal `'error'`
I'm seeing a lot of instances of
> Failed to save: Unable to reserve cache with key
runtime-and-compiler-node_modules-v5-X64-Linux-e454609794aae66da9909c77dd6efa073eceff7f44d6527611f8465e102578b4,
another job may be creating this cache.
which is adding ~20 seconds to every step. Let's try to bust the cache
following this
[comment](https://github.com/actions/cache/issues/485#issuecomment-744145040)
and see if that helps.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32671).
* #32672
* __->__ #32671
Using the github variable for the commit message replaces the variable
inline. If the commit message contains quotes or other characters that
need to be escaped, this breaks the workflow.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32668).
* #32669
* __->__ #32668
Follow up to #32656.
Remove touchAction from SwipeRecognizer. I was under the wrong
impression that this was only the touch-action applied to this
particular element, but that parents would still win but in fact this
blocks the parent from scrolling in the other direction. By specifying a
fixed direction it also blocked rage-swiping in the other direction
early on.
Disable pointer-events on view-transition so that the scroll can be hit.
This means that touches hit below the items animating above. This allows
swiping to happen again before momentum scroll has finished. Previously
they were ignored. This only works as long as the SwipeRecognizer is
itself not animating. This means you can now rage-swipe in both
directions quickly.
Alternative to facebook/react#31584 which sets
enableTreatFunctionDepsAsConditional:true` by default.
This PR changes dependency hoisting to be more conservative while trying
to preserve an optimal "happy path". We assume that a function "is
likely called" if we observe the following in the react function body.
- a direct callsite
- passed directly as a jsx attribute or child
- passed directly to a hook
- a direct return
A function is also "likely called" if it is directly called, passed to
jsx / hooks, or returned from another function that "is likely called".
Note that this approach marks the function definition site with its
hoistable properties (not its use site). I tried implementing use-site
hoisting semantics, but it felt both unpredictable (i.e. as a developer,
I can't trust that callbacks are well memoized) and not helpful (type +
null checks of a value are usually colocated with their use site)
In this fixture (copied here for easy reference), it should be safe to
use `a.value` and `b.value` as dependencies, even though these functions
are conditionally called.
```js
// inner-function/nullable-objects/assume-invoked/conditional-call-chain.tsx
function Component({a, b}) {
const logA = () => {
console.log(a.value);
};
const logB = () => {
console.log(b.value);
};
const hasLogged = useRef(false);
const log = () => {
if (!hasLogged.current) {
logA();
logB();
hasLogged.current = true;
}
};
return <Stringify log={log} shouldInvokeFns={true} />;
}
```
On the other hand, this means that we produce invalid output for code
like manually implementing `Array.map`
```js
// inner-function/nullable-objects/bug-invalid-array-map-manual.js
function useFoo({arr1, arr2}) {
const cb = e => arr2[0].value + e.value;
const y = [];
for (let i = 0; i < arr1.length; i++) {
y.push(cb(arr1[i]));
}
return y;
}
```
Reverts facebook/react#30660
I don’t feel confident in the approach. This part of code is supposed to
rely on the module bundler behaving as expected. _Maybe_ this is correct
but I need to review it closer — it was intentionally _not_ implemented
this way originally.
I’ll try to take a closer look some time this week. We don’t have to
merge this revert right now but just flagging that I don’t understand
the thinking behind the new approach and don’t have confidence in it.
Adds `getClientRects()` to fragment instances with a fixture test case.
`Element.getClientRect` returns a collection of `DOMRect`s (see example
of multiline span returning two `DOMRect` boxes).
`fragmentInstance.getClientRects` here flattens those collections into
an array of rects.
`focus()` was added in https://github.com/facebook/react/pull/32465.
Here we add `focusLast()` and `blur()`. I also extended `focus` to take
options.
`focus` will focus the first focusable element. `focusLast` will focus
the last focusable element. We could consider a `focusFirst` naming or
even the `focusWithin` used by test selector APIs as well.
`blur` will only have an effect if the current `document.activeElement`
is one of the fragment children.
I made the button a bit bigger and moved the swipe recognizer around the
whole screen. Typically these are used around the whole content without
any affordances and not as a standalone scrubber. Ideally the swipe
would be able to be inside the animating content but it can't yet due to
[this Safari bug](https://bugs.webkit.org/show_bug.cgi?id=288795).
Added back some paragraphs so that scrolling can be tested properly. It
appears it's possible to get the swipe to be a bit misaligned if you
scroll enough on iOS.
<img width="437" alt="Screenshot 2025-03-17 at 10 27 42 PM"
src="https://github.com/user-attachments/assets/589dc828-717e-420c-83dc-94ae6ad59791"
/>
This does the same thing for `measureUpdateViewTransition` that we did
for `measureNestedViewTransitions` in
e3cbaffef0.
If a boundary hasn't mutated and didn't change in size, we mark it for
cancellation. Otherwise we add names to it. The different from the
CommitViewTransition path is that the "old" names are added to the
clones so this is the first time the "new" names.
Now we also cancel any boundaries that were unchanged. So now the root
no longer animates. We still have to clone them. There are other
optimizations that can avoid cloning but once we've done all the layouts
we can still cancel the running animation and let them just be the
regular content if they didn't change. Just like the regular
fire-and-forget path.
This also fixes the measurement so that we measure clones by adjusting
their position back into the viewport.
This actually surfaces a bug in Safari that was already in #32612. It
turns out that the old names aren't picked up for some reason and so in
Safari they looked more like a cross-fade than what #32612 was supposed
to fix. However, now that bug is even more apparent because they
actually just disappear in Safari. I'm not sure what that bug is but
it's unrelated to this PR so will fix that separately.
ViewTransition uses the `useId` algorithm to auto-assign names. This
ensures that we could animate between SSR content and client content by
ensuring that the names line up.
However, I missed that we need to bump the id (materialize it) when we
do that. This is what function components do if they use one or more
`useId()`. This caused duplicate names when two ViewTransitions were
nested without any siblings since they would share name.
In light of recent third party actions being compromised, let's just
push the commit ourselves rather than use a third party action. We
already detect if changes are needed, so the step will only run if so.
I also added a `dry_run` option to the manual runs of this workflow for
testing.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32648).
* #32650
* #32649
* __->__ #32648
Follow-up to #31850. We want to build using the original commit SHA, not
the merge commit that GitHub Actions creates behind the scenes. We were
already checking out the correct commit object, but the COMMIT_SHA
artifact was still pointing to the merge commit.
This should fix the sizebot links to point to working URLs, too.
We don't have an experimental-only build of devtools, but we can at
least add these filters to the internal build.
A better way would be to use feature detection, but I'm not sure how and
this isn't a very heavily used feautre.
This implements `observeUsing(observer)` and `unobserverUsing(observer)`
on fragment instances. IntersectionObservers and ResizeObservers can be
passed to observe each host child of the fragment. This is the
equivalent to calling `observer.observe(child)` or
`observer.unobserve(child)` for each child target.
Just like the addEventListener, the observer is held on the fragment
instance and applied to any newly mounted child. So you can do things
like wrap a paginated list in a fragment and have each child
automatically observed as they commit in.
Unlike, the event listeners though, we don't `unobserve` when a child is
removed. If a removed child is currently intersecting, the observer
callback will be called when it is removed with an empty rect. This lets
you track all the currently intersecting elements by setting state from
the observer callback and either adding or removing them from your list
depending on the intersecting state. If you want to track the removal of
items offscreen, you'd have to maintain that state separately and append
intersecting data to it in the observer callback. This is what the
fixture example does.
There could be more convenient ways of managing the state of multiple
child intersections, but basic examples are able to be modeled with the
simple implementation. Let's see how the usage goes as we integrate this
with more advanced loggers and other features.
For now you can only attach one observer to an instance. This could
change based on usage but the fragments are composable and could be
stacked as one way to apply multiple observers to the same elements.
In practice, one pattern we expect to enable is more composable logging
such as
```javascript
function Feed({ items }) {
return (
<ImpressionLogger>
{items.map((item) => (
<FeedItem />
))}
</ImpressionLogger>
);
}
```
where `ImpressionLogger` would set up the IntersectionObserver using a
fragment ref with the required business logic and various components
could layer it wherever the logging is needed. Currently most callsites
use a hook form, which can require wiring up refs through the tree and
merging refs for multiple loggers.
Based off: https://github.com/facebook/react/pull/32499
While looking into `React.lazy` issues for built-ins, I noticed we
already error for `lazy` with build-ins, but we don't have any tests for
`getComponentNameFromType` using all the built-ins. This may be
something we should handle, but for now we should at least have tests.
Here's why: while writing tests, I noticed we check `type` instead of
`$$typeof` for portals:
9cdf8a99ed/packages/react-reconciler/src/ReactPortal.js (L25-L32)
This PR adds tests for all the built-ins and fixes the portal bug.
[Commit to
review](e068c167d4)
This PR separates Activity to it's own element type separate from
Offscreen. The goal is to allow us to add Activity element boundary
semantics during hydration similar to Suspense semantics, without
impacting the Offscreen behavior in suspended children.
There's two ways to find updated View Transitions.
One is the "commit/measureNestedViewTransitions" pass which is used to
find things in unchanged subtrees. This can only lead to the relayout
case since there's can't possibly be any mutations in the subtree. This
is only triggered when none of the direct siblings have any mutations at
all.
The other case is "commit/measureUpdateViewTransition" which is for a
ViewTransition that itself has mutations scheduled inside of it which
leads to the "update" case.
However, there's a case between these two cases. When a direct sibling
has a mutation but there's also a ViewTransition exactly at the same
level. In that case we can't bail out on the whole set of children so we
won't trigger the "nested" case. Previously we also didn't trigger the
"commit/measureUpdateViewTransition" case because we first checked if
that had any mutations inside of it at all. This leads to neither case
picking up this boundary.
We could check if the ViewTransition itself has any mutations inside and
if not trigger the nested path.
There's a simpler way though. Because
`commit/measureUpdateViewTransition` is actually just optimistic. The
flags are pessimistic and we don't know for sure if there will actually
be a mutation until we've traversed the tree. It can sometimes lead to
the "relayout" case. So we can just use that same path, knowing that
it'll just lead to the layout pass. Therefore it's safe to just remove
this check.
This is a nit but a Config should not have to know anything about the
internals of Fibers. Ideally it shouldn't even access them but we have
some cases where we need pointers back in like for this fragment.
The way we've typically abstracted this is using the
`ReactFiberTreeReflection` helper that's in the `react-reconciler`. Such
as in the event system.
f3c956006a/packages/react-dom-bindings/src/events/ReactDOMEventListener.js (L22-L26)
We sometimes cheat but we really should clean this up such that a
`Fiber` is actually an opaque type to the Configs and it can never dot
into it without using a helper.
So this just moves `traverseFragmentInstanceChildren` to
ReactFiberTreeReflection so that the ConfigDOM doesn't ever dot into its
fields itself. It just passes the Fiber through back into the
react-reconciler. I had to add a wrapper to read the `.child` to avoid
that being assumed too. I also noticed that FragmentInstanceType is not
actually passed through so that argument is unnecessary.
Stacked on #32599 and #32611.
This is able to reuse the code from CommitViewTransitions for "enter",
"shared" and "layout". The difference is that for "enter"/"shared" in
the "new" phase we pass in the deletions.
For "layout" of nested boundaries we just need to measure the clones at
the same time we measure the original nodes since we haven't measured
them in a previous phase in the current approach.
With these updates, things move around more like expected in the fixture
because we're now applying the appropriate pairs to trigger individual
animations instead of just the full document cross-fade.
The "update" phase is a little more complicated and is coming soon.
Stacked on #32578.
We need to apply view-transition-names to the clones that we create in
the "old" phase for the ViewTransition boundaries that should activate.
Finding pairs is a little trickier than in
ReactFiberCommitViewTransitions. Normally we collect all name
"insertions" in the `accumulateSuspenseyCommit` phase before we even
commit. Then in the snapshot do we visit all "deletions" and since we
already collected all the insertions we know immediately if the deletion
had a pair and should therefore get a "name" assigned to activate the
boundary. For ReactFiberApplyGesture we need to assign names to
"insertions" since it's in reverse but we don't already have a map of
deletions. Therefore we need to first visit all deletions.
Instead of doing that in a completely separate pass, we instead visit
deletions in the same pass to find pairs. Since this is in the same pass
we might visit insertions before deletions or vice versa depending on
document order. However, we can deal with this by applying the name to
the insertion when we find the deletion if we've already made the clones
at that point.
Applying names to pure exits, updates or nested (relayout) is a bit more
straight-forward.
Stacked on #32585 and #32605.
This adds more loops for the phases of "Apply Gesture". It doesn't
implement the interesting bit yet like adding view-transition-names and
measurements. I'll do that in a separate PR to keep reviewing easier.
The three phases of this approach is roughly:
- Clone and apply names to the "old" state.
- Inside startViewTransition: Apply names to the "new" state. Measure
both the "old" and "new" state to know whether to cancel some of them.
Delete the clones which will include all the "old" names.
- After startViewTransition: Restore "new" names back to no
view-transition-name.
Since we don't have any other Effects in these phases we have a bit more
flexibility and we can avoid extra phases that traverse the tree. I've
tried to avoid any additional passes.
An interesting consequence of this approach is that we could measure
both the "old" and "new" state before `startViewTransition`. This would
be more efficient because we wouldn't need to take View Transition
snapshots of parts of the tree that won't actually animate. However,
that would require an extra pass and force layout earlier. It would also
have different semantics from the fire-and-forget View Transitions
because we could optimize better which can be visible. It would also not
account for any late mutations. So I decided to instead let the layout
be computed by painting as usual and then measure both "old" and "new"
inside the startViewTransition instead. Then canceling anything that
doesn't animate to keep it consistent.
Unfortunately, though there's not a lot of code sharing possible in
these phases because the strategy is so different with the cloning and
because the animation is performed in reverse. The "finishedWork" Fiber
represents the "old" state and the "current" Fiber represents the "new"
state.
The most complicated phase is the cloning. I actually ended up having to
make a very different pattern from the other phases and CommitWork in
general. Because we have to clone as we go and also do other things like
apply names and finding pairs, it has more phases. I ended up with an
approach that uses three different loops. The outer one for updated
trees, one for inserted trees that don't need cloning (doesn't include
reappearing offscreen) and one for not updated trees that still need
cloning. Inside each loop it can also be in different phases which I
track with the `visitPhase` enum - this pattern is kind of new.
Additionally, we need to measure the cloned nodes after we've applied
mutations to them and we have to wait until the whole tree is inserted.
We don't have a reference to these DOM elements in the Fiber tree since
that still refers to the original ones. We need to store the cloned
elements somewhere. So I added a temporary field on the
ViewTransitionState to keep track of any clones owned by that
ViewTransition.
When we deep clone an unchanged subtree we don't have DOM element
instances. It wouldn't be quite safe to try to find them from the tree
structure. So we need to avoid the deep clones if we might need DOM
elements. Therefore we keep traversing in the case where we need to find
nested ViewTransition boundaries that are either potentially affected by
layout or a "pair".
For the other two phases the pattern there's a lot of code duplication
since it's slightly different from the commit ones but they at least
follow the same pattern. For the restore phase I was actually able to
reuse most of the code.
I don't love how much code this is.
This prepares from being able to reuse some this in ApplyGesture.
These all start with resetting a counter but it's tricky to have to
remember to do this and tricky to do from the outside of this module. So
we make an exported helper that does the resetting. Ideally it gets
inlined.
We also stop passing "current" to measureViewTransitionHostInstances.
Same thing for cancelViewTransitionHostInstances. This doesn't make
sense for "nested" which has not updated and so might not have an
alternate. Instead we pass in the old and new name if they might be
different.
Normally these are gated by the whole commitGestureOnRoot path but in
the case of an early commit these phases may need to be invoked.
Earlier. Those paths weren't gated which I noticed when I started adding
code to them.
This is the exact same code in both cases. It's just general clean up.
By unifying them it becomes less confusing to reuse these helpers in the
Apply Gesture path where the naming is reversed.
Traverse program after running compiler transform to find untransformed
references to compiler features (e.g. `inferEffectDeps`, `fire`).
Hard error to fail the babel pipeline when the compiler fails to
transform these features to give predictable runtime semantics.
Untransformed calls to functions like `fire` will throw at runtime
anyways, so let's fail the build to catch these earlier.
Note that with this fails the build *regardless of panicThreshold*
We only need the compiler built for `yarn test` in the root directory.
Rather than always cache both for every step, let's just do it where
it's needed explicitly.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32608).
* #32609
* __->__ #32608
Now that the compiler lint rule is merged into
eslint-plugin-react-hooks, we also need to update our caches so compiler
dependencies are also cached. This should fix the CI walltime regression
we are now seeing.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32603).
* #32604
* __->__ #32603
Removes `EnvironmentConfig.enableMinimalTransformsForRetry` in favor of
`run` parameters. This is a minimal difference but lets us explicitly
opt out certain compiler passes based on mode parameters, instead of
environment configurations
Retry flags don't really make sense to have in `EnvironmentConfig`
anyways as the config is user-facing API, while retrying is a compiler
implementation detail.
(per @josephsavona's feedback
https://github.com/facebook/react/pull/32164#issuecomment-2608616479)
> Re the "hacky" framing of this in the PR title: I think this is fine.
I can see having something like a compilation or output mode that we use
when running the pipeline. Rather than changing environment settings
when we re-run, various passes could take effect based on the
combination of the mode + env flags. The modes might be:
>
> * Full: transform, validate, memoize. This is the default today.
> * Transform: Along the lines of the backup mode in this PR. Only
applies transforms that do not require following the rules of React,
like `fire()`.
> * Validate: This could be used for ESLint.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32511).
* #32512
* __->__ #32511
This change fixes a coverage hole in rolling out with `gating`. Prior to
this PR, configuring `gating` causes React Compiler to bail out of
optimizing some functions.
This means that it's not entirely safe to cutover from `gating` enabled
for all users (i.e. rolled out 100%) to removing the `gating` config
altogether, as new functions may be opted into compilation when they
stop bailing out due to gating-specific logic.
This is technically slightly slower due to the additional function
indirection. An alternative approach is to recommend running a codemod
to insert `use no memo`s on currently-bailing out functions before
removing the`gating` config.
---
Tested [internally](
https://fburl.com/diff/q982ovua) by enabling on a page that previously
had a few hundred bailouts due to gating + hoisted function declarations
and (1) clicking around locally and (2) running a bunch of e2e tests
Reduce false positive bailouts by using the same
`isReferencedIdentifier` logic that the compiler also uses for
determining context variables and a function's own hoisted declarations.
Details:
Previously, we counted every babel identifier as a reference. This is
problematic because babel counts most string symbols as an identifier.
```js
print(x); // x is an identifier as expected
obj.x // x is.. also an identifier here
{x: 2} // x is also an identifier here
```
This PR adds a check for `isReferencedIdentifier`. Note that only
non-lval
references pass this check. This should be fine as we don't need to
hoist function declarations before writes to the same lvalue (which
should error in strict mode anyways)
```js
print(x); // isReferencedIdentifier(x) -> true
obj.x // isReferencedIdentifier(x) -> false
{x: 2} // isReferencedIdentifier(x) -> false
x = 2 // isReferencedIdentifier(x) -> false
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32596).
* __->__ #32596
* #32595
* #32594
* #32593
* #32522
* #32521
- Add `at`, `indexOf`, and `includes`
- Optimize MixedReadOnly which is currently only used by hook return
values. Hook return values are typed as Frozen, this change propagates
that to return values of aliasing function calls (such as `at`). One
potential issue is that developers may pass
`enableAssumeHooksFollowRulesOfReact:false` and set
`transitiveMixedData`, expecting their transitive mixed data to be
mutable. This is a bit of an edge case and already doesn't have clear
semantics.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32594).
* #32596
* #32595
* __->__ #32594
* #32593
* #32522
* #32521
Expand type inference to infer mixedReadOnly types for numeric and
computed property accesses.
```js
function Component({idx})
const data = useFragment(...)
// we want to type `posts` correctly as Array
const posts = data.viewers[idx].posts.slice(0, 5);
// ...
}
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32593).
* #32596
* #32595
* #32594
* __->__ #32593
* #32522
* #32521
## Summary
Right now, `react-compiler-healthcheck` flags `mobx` as a "known
incompatible library". But it's not precisely *MobX* that's
incompatible. It's the observer HOC that comes from `mobx-react` and
`mobx-react-lite`.
I've been working on
[mst-use-observable](https://github.com/coolsoftwaretyler/mst-use-observable),
which makes MobX-State-Tree compatible with the compiler. However,
projects that use `mobx-state-tree` and `mst-use-observable` will still
depend on `mobx` as a dependency.
And there [have been efforts in the past to write a hook for
observability](https://github.com/mobxjs/mobx/discussions/2566). So it's
possible that MobX could become compatible, so long as authors access it
with a hook, rather than the HOC.
I would like to propose updating the health check to be a little more
precise and flag the HOC dependencies, rather than MobX itself.
Thanks in advance for your consideration!
## How did you test this change?
`npx react-compiler-healthcheck` shouldn't flag on `mobx` in
dependencies, but will for `mobx-react-lite` and `mobx-react`.
Test suites, formatting, linting, all passed.
---------
Co-authored-by: lauren <poteto@users.noreply.github.com>
fix: update CONTRIBUTING.md link path
Updated the relative path to CONTRIBUTING.md from `../CONTRIBUTING.md`
to `./../../CONTRIBUTING.md` to ensure the correct file is referenced.
<!--
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?
-->
## 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 change merges the `react-compiler` rule from
`eslint-plugin-react-compiler` into the `eslint-plugin-react-hooks`
plugin. In order to do the move in a way that keeps commit history with
the moved files, but also no remove them from their origin until a
future cleanup change can be done, I did the `git mv` first, and then
recreated the files that were moved in their original places, as a
separate commit. Unfortunately GH shows the moved files as new instead
of the ones that are truly new. But in the IDE and `git blame`, commit
history is intact with the moved files.
Since this change adds new dependencies, and one of those dependencies
has a higher `engines` declaration for `node` than what the plugin
currently has, this is technically a breaking change and will have to go
out as part of a major release.
### Related Changes
- https://github.com/facebook/react/pull/32458
---------
Co-authored-by: Lauren Tan <poteto@users.noreply.github.com>
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>
it('effect autodeps config must have at least 1 required argument',()=>{
expect(()=>{
validateEnvironmentConfig({
inferEffectDependencies:[
{
function:{
source:'react',
importSpecifierName:'useEffect',
},
numRequiredArgs: 0,
},
],
}asany);
}).toThrowErrorMatchingInlineSnapshot(
`"InvalidConfig: Could not validate environment config. Update React Compiler config to fix the error. Validation error: numRequiredArgs must be > 0 at "inferEffectDependencies[0].numRequiredArgs""`,
| ^^^ Invariant: Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting. Rewrite the reference to Foo_withRef to not rely on hoisting to fix this issue (2:2)
| ^^^^^^^^^^^ Invariant: Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting. Rewrite the reference to Foo_withRef to not rely on hoisting to fix this issue (3:3)
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
