A few small improvements:
* Use `<root>/.worktrees` as the directory for worktrees so its hidden
by default in finder/ls
* Generate names with a timestamp, and allow auto-generating a name so
that you can just call eg `./scripts/worktree.sh --compiler --claude`
and get a random name
A few times an agent has constructed fixtures that are silently skipped
because the component has no jsx or hook calls. This PR updates snap to
ensure that for each fixture either:
1) There are at least one compile success/failure *and* the
`@expectNothingCompiled` pragma is missing
2) OR there are zero success/failures *and* the `@expectNothingCompiled`
pragma is present
This ensures we are intentional about fixtures that are expected not to
have compilation, and know if that expectation breaks.
A whole bunch of changes to snap aimed at making it more usable for
humans and agents. Here's the new CLI interface:
```
node dist/main.js --help
Options:
--version Show version number [boolean]
--sync Run compiler in main thread (instead of using worker
threads or subprocesses). Defaults to false.
[boolean] [default: false]
--worker-threads Run compiler in worker threads (instead of
subprocesses). Defaults to true.
[boolean] [default: true]
--help Show help [boolean]
-w, --watch Run compiler in watch mode, re-running after changes
[boolean]
-u, --update Update fixtures [boolean]
-p, --pattern Optional glob pattern to filter fixtures (e.g.,
"error.*", "use-memo") [string]
-d, --debug Enable debug logging to print HIR for each pass[boolean]
```
Key changes:
* Added abbreviations for common arguments
* No more testfilter.txt! Filtering/debugging works more like Jest, see
below.
* The `--debug` flag (`-d`) controls whether to emit debug information.
In watch mode, this flag sets the initial debug value, and it can be
toggled by pressing the 'd' key while watching.
* The `--pattern` flag (`-p`) sets a filter pattern. In watch mode, this
flag sets the initial filter. It can be changed by pressing 'p' and
typing a new pattern, or pressing 'a' to switch to running all tests.
* As before, we only actually enable debugging if debug mode is enabled
_and_ there is only one test selected.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35537).
* #35607
* #35298
* #35596
* #35573
* #35595
* #35539
* __->__ #35537
* #35523
Intended to be used directly and/or from skills in an agent.
Usage is `./scripts/worktree.sh [--compiler] [--claude] <name>`. The
script:
* Checks that ./worktrees is in gitignore
* Checks the named worktree does not exist yet
* Creates the named worktree in ./worktrees/
* Installs deps
* cds into the worktree (optionally the compiler dir if `--compiler`)
* optionally runs claude in the worktree if `--claude`
Add a fast heuristic to detect whether a file may contain React
components or hooks before running the full compiler. This avoids the
overhead of Babel AST parsing and compilation for utility files, config
files, and other non-React code.
The heuristic uses ESLint's already-parsed AST to check for functions
with React-like names at module scope:
- Capitalized functions: MyComponent, Button, App
- Hook pattern functions: useEffect, useState, useMyCustomHook
Files without matching function names are skipped and return an empty
result, which is cached to avoid re-checking for subsequent rules.
Also adds test coverage for the heuristic edge cases.
Follow up to #35559.
The clean up function of the custom timeline doesn't necessarily clean
up the animation. Just the timeline's internal state.
This affects Firefox which doesn't support ScrollTimeline so uses the
polyfill's custom timeline.
Currently we always clone the root when a gesture transition happens.
The was to add an optimization where if a Transition could be isolated
to an absolutely positioned subtree then we could just clone that
subtree or just do a plain insertion if it was simple an Enter. That way
when switching between two absolutely positioned pages the shell
wouldn't need to be cloned. In that case `detectMutationOrInsertClones`
would return false. However, currently it always return true because we
don't yet have that optimization.
The idea was to warn when the root required cloning to ensure that you
optimize it intentionally since it's easy to accidentally update more
than necessary. However, since this is not yet actionable I'm removing
this warning for now.
Instead, I add a warning for particularly bad cases where you really
shouldn't clone like iframe and video. They may not be very actionable
without the optimization since you can't scope it down to a subtree
without the optimization. So if they're above the gesture then they're
always cloned atm. However, it might also be that it's unnecessary to
keep them mounted if they could be removed or hidden with Activity.
`useInsertionEffect` is meant to be used to insert `<style>` tags that
affect the layout. It allows precomputing a layout before it mounts.
Since we're not normally firing any effects during the "apply gesture"
phase where we create the clones, it's possible for the target snapshot
to be missing styles. This makes it so that `useInsertionEffect` for a
new tree are mounted before the snapshot is taken and then unmounted
before the animation starts.
Note that because we are mounting a clone of the DOM tree and the
previous DOM tree remains mounted during the snapshot, we can't unmount
any previous insertion effects. This can lead to conflicts but that is
similar to what can happen with conflicts for two mounted Activity
boundaries since insertion effects can remain mounted inside those.
A revealed Activity will have already had their insertion effects fired
while offscreen.
However, one thing this doesn't yet do is handle the case where a
`useInsertionEffect` is *updated* as part of a gesture being applied.
This means it's still possible for it to miss some styles in that case.
The interesting thing there is that since the old state and the new
state will both be applicable to the global DOM in this phase, what
should really happen is that we should mount the new updated state
without unmounting the old state and then unmount the updated state.
Meaning you can have the same hook in the mounted state twice at the
same time.
Stacked on #35556 and #35559.
Given that we don't automatically clean up all view transition
animations since #35337 and browsers are buggy, it's important that you
clean up any `Animation` started manually from the events. However,
there was no clean up function for when the View Transition is forced to
stop. This also makes it harder to clean up custom timers etc too.
This lets you return a clean up function from all the events on
`<ViewTransition>`.
Follow up to #35337.
During a gesture, we always cancel the original animation and create a
new one that we control. That's the one we need to add to the set that
needs to be cancelled. Otherwise future gestures hang.
An unfortunate consequence is that any custom ones that you start e.g.
with #35556 or through other means aren't automatically cleaned up (in
fact there's not even a clean up callback yet). This can lead these to
freeze the whole UI afterwards. It would be really good to get this
fixed in browsers instead so we can revert #35337.
Flights tests are failing locally and in CI non-deterministically
because we're not disabling async hooks after tests, and GC can clear
WeakRefs non-deterministically.
This PR fixes the issue by adding an afterEach to disable installed
hooks, and normalizing the `value` to `value: {value: undefined}}` when
snapshotting.
I was experimenting with animations in SuspenseList and hit a crash
using ViewTransition as a direct child with `revealOrder="together"`
```
TypeError: Cannot read properties of null (reading 'autoName')
33 | return props.name;
34 | }
> 35 | if (instance.autoName !== null) {
| ^
36 | return instance.autoName;
37 | }
```
When ViewTransition is direct child of SuspenseList, the second render
pass calls resetChildFibers, setting stateNode to null. Other fibers
create stateNode in completeWork. ViewTransition does not, so stateNode
is lost.
Followed the pattern used for Offscreen to update stateNode in beginWork
if it is null.
Also added a regression test.
When `renderModelDestructive` unwraps a lazy element and subsequently
calls `renderModelDestructive` again with the resolved model, we should
preserve the parent connection so that cyclic references can be
serialized properly. This can occur in an advanced scenario where the
result from the Flight Client is serialized again with the Flight
Server, e.g. for slicing a precomputed payload into multiple parts.
Note: The added test only fails when run with `--prod`. In dev mode, the
component info outlining prevents the issue from occurring.
When a lazy element or component is initialized a thenable is returned
which was only be conditionally instrumented in dev when asyncDebugInfo
was enabled. When instrumented these thenables can be used in
conjunction with the SuspendOnImmediate optimization where if a thenable
resolves before the stack unwinds we can continue rendering from the
last suspended fiber. Without this change a recent fix to the useId
implementation cannot be easily tested in production because this
optimization pathway isn't available to regular React.lazy thenables. To
land the prior PR I changed the thenables to a custom type so I could
instrument manually in the test. WIth this change we can just use a
regular Promise since ReactLazy will instrument in all
environments/flags now
Stacked on #35487.
This is slightly different because the first suspended commit is on
blockers that prevent us from committing which still needs to be
resolved first.
If a gesture lane has to be rerendered while the gesture is happening
then it reenters this state with a new tree. (Currently this doesn't
happen for a ping I think which is not really how it usually works but
better in this case.)
If an initial value is specified, then it's always used regardless as
part of the gesture render.
If a gesture render causes an update, then previously that was not
treated as deferred and could therefore be blocking the render. However,
a gesture is supposed to flush synchronously ideally. Therefore we
should consider these as urgent.
The effect is that useDeferredValue renders the previous state.
Stacked on #35486.
When a Gesture commits, it leaves behind work on a Transition lane
(`revertLane`). This entangles that lane with whatever lane we're using
in the event that cancels the Gesture. This ensures that the revert and
the result of any resulting Action commits as one batch. Typically the
Action would apply a new state that is similar or the same as the revert
of the Gesture.
This makes it resilient to unbatching in #35392.
Stacked on #35485.
Before this PR, the `startGestureTransition` API would itself never
commit its state. After the gesture releases it stops the animation in
the next commit which just leaves the DOM tree in the original state. If
there's an actual state change from the Action then that's committed as
the new DOM tree. To avoid animating from the original state to the new
state again, this is DOM without an animation. However, this means that
you can't have the actual action committing be in a slightly different
state and animate between the final gesture state and into the new
action.
Instead, we now actually keep the render tree around and commit it in
the end. Basically we assume that if the Timeline was closer to the end
then visually you're already there and we can commit into that state.
Most of the time this will be at the actual end state when you release
but if you have something else cancelling the gesture (e.g.
`touchcancel`) it can still commit this state even though your gesture
recognizer might not consider this an Action. I think this is ok and
keeps it simple.
When the gesture lane commits, it'll leave a Transition behind as work
from the revert lanes on the Optimistic updates. This means that if you
don't do anything in the Action this will cause another commit right
after which reverts. This revert can animate the snap back.
There's a few fixes needed in follow up PRs:
- Fixed in #35487. ~To support unentangled Transitions we need to
explicitly entangle the revert lane with the Action to avoid committing
a revert followed by a forward instead of committing the forward
entangled with the revert. This just works now since everything is
entangled but won't work with #35392.~
- Fixed in #35510. ~This currently rerenders the gesture lane once
before committing if it was already completed but blocked. We should be
able to commit the already completed tree as is.~
When Fiber replays work after suspending and resolving in a microtask it
stripped the Forked flag from Fibers because this flag type was not
considered a Static flag. The Forked nature of a Fiber is not render
dependent and should persist after unwinding work. By making this change
the replay correctly generates the necessary tree context.
When a View Transition might not need to update we add it to a queue. If
the parent are able to be reverted, we then cancel the already started
view transitions. We do this by adding an animation that hides the "old"
state and remove the view transition name from the old state.
There was a bug where if you have more than one child in a
`<ViewTransition>` we didn't add the right suffix to the name we added
in the queue so it wasn't adding an animation that hides the old state.
The effect was that it playing an exit animation instead of being
cancelled.
Requires full error message in assert helpers.
Some of the error messages we asset on add a native javascript stack
trace, which would be a pain to add to the messages and maintain. This
PR allows you to just add `\n in <stack>` placeholder to the error
message to denote a native stack trace is present in the message.
---
Note: i vibe coded this so it was a pain to backtrack this to break this
into a stack, I tried and gave up, sorry.
When hydrating if something suspends and then resolves in a microtask it
is possible that React will resume the render without fully unwinding
work in progress. This can cause hydration cursors to be offset and lead
to hydration errors. This change adds a restore step when replaying
HostComponent to ensure the hydration cursor is in the appropriate
position when replaying.
fixes: #35210
Jest's default test sequencer sorts alphabetically, causing large test
files
(eg ReactDOMFloat-test.js at 9k lines,
ReactHooksWithNoopRenderer-test.js at 4k
lines) to cluster in shard 3/5. This made shard 3/5 average 117s vs 77s
for
other shards, a 52% slowdown. I'm using filesize as a rough proxy for
number of tests.
This custom sequencer sorts tests by file size and distributes large
files evenly across all shards
instead of clustering them together.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35458).
* __->__ #35458
* #35459
DevTools has ~45 test files which don't distribute well across 10
shards,
causing shard 3 to run 2x slower than others (104s vs ~50s). This moves
DevTools build tests to a separate job with 3 shards for better load
balancing.
When the Fizz runtime runs a view-transition we apply
`view-transition-name` and `view-transition-class` to the `style`. These
can be observed by Fiber when hydrating which incorrectly leads to
hydration errors.
More over, even after we remove them, the `style` attribute has now been
normalized which we are unable to diff because we diff against the SSR
generated `style` attribute string and not the normalized form. So if
there are other inline styles defined, we have to skip diffing them in
this scenario.
## Summary
This PR improves cyclic thenable detection in
`ReactFlightReplyServer.js`. Fixes#35368.
The previous fix only detected direct self-references (`inspectedValue
=== chunk`) and relied on the `cycleProtection` counter to eventually
bail out of longer cycles. This change keeps the existing
MAX_THENABLE_CYCLE_DEPTH ($1000$) `cycleProtection` cap as a hard
guardrail and adds a visited set so that we can detect self-cycles and
multi-node cycles as soon as any `ReactPromise` is revisited and while
still bounding the amount of work we do for deep acyclic chains via
`cycleProtection`.
## How did you test this change?
- Ran the existing test suite for the server renderer:
```bash
yarn test react-server
yarn test --prod react-server
yarn flow dom-node
yarn linc
```
---------
Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
