* A few new minimization strategies, removing function params and
array/object pattern elements
* Ensure that we preserve the same set of errors based on not just
category+reason but also description.
More snap improvements for use with agents:
* `yarn snap compile [--debug] <path>` for compiling any file,
optionally with debug logs
* `yarn snap minimize <path>` now accepts path as a positional param for
consistency w 'compile' command
* Both compile/minimize commands properly handle paths relative to the
compiler/ directory. When using `yarn snap` the current working
directory is compiler/packages/snap, but you're generally running it
from the compiler directory so this matches expectations of callers
better.
This is a combination of a) a subagent for investigating compiler errors
and b) testing that agent by fixing bugs with for loops within
try/catch. My recent diffs to support maybe-throw within value blocks
was incomplete and handled many cases, like optionals/logicals/etc
within try/catch. However, the handling for for loops was making more
assumptions and needed additional fixes.
Key changes:
* `maybe-throw` terminal `handler` is now nullable. PruneMaybeThrows
nulls the handler for blocks that cannot throw, rather than changing to
a `goto`. This preserves more information, and makes it easier for
BuildReactiveFunction's visitValueBlock() to reconstruct the value
blocks
* Updates BuildReactiveFunction's handling of `for` init/test/update
(and similar for `for..of` and `for..in`) to correctly extract value
blocks. The previous logic made assumptions about the shape of the
SequenceExpression which were incorrect in some cases within try/catch.
The new helper extracts a flattened SequenceExpression.
Supporting changes:
* The agent itself (tested via this diff)
* Updated the script for invoking snap to keep `compiler/` as the
working directory, allowing relative paths to work more easily
* Add an `--update` (`-u`) flag to `yarn snap minimize`, which updates
the fixture in place w the minimized version
Allows Server Components to import Context from a `"use client'` module
and render its Provider.
Only tricky part was that I needed to add `REACT_CONTEXT_TYPE` handling
in mountLazyComponent so lazy-resolved Context types can be rendered.
Previously only functions, REACT_FORWARD_REF_TYPE, and REACT_MEMO_TYPE
were handled.
Tested in the Flight fixture.
ty bb claude
Closes https://github.com/facebook/react/issues/35340
---------
Co-authored-by: Sophie Alpert <git@sophiebits.com>
Currently, IO that finished before the request started is not considered
IO:
6a0ab4d2dd/packages/react-server/src/ReactFlightServer.js (L5338-L5343)
This leads to loss of debug info when a flight stream is deserialized
and serialized again.
We can solve this by allowing "when the the request started" to be set
to a point in the past, when the original stream started by doing
```js
const startTime = performance.now() + performance.timeOrigin
// ... stuff happens and time passes...
ReactServer.renderToReadableStream(..., { startTime })
```
Fixes https://github.com/facebook/react/issues/31463,
https://github.com/facebook/react/issues/30114.
When switching between roots in the profiler flamegraph, the commit
index was preserved from the previous root. This caused an error
"Invalid commit X. There are only Y commits." when the new root had
fewer commits than the selected index.
This fix resets the commit index to 0 (or null if no commits) when the
commitData changes, which happens when switching roots.
Snap now supports subcommands 'test' (default) and 'minimize`. The
minimize subcommand attempts to minimize a single failing input fixture
by incrementally simplifying the ast so long as the same error occurs. I
spot-checked it and it seemed to work pretty well. This is intended for
use in a new subagent designed for investigating bugs — fixture
simplification is an important part of the process and we can automate
this rather than light tokens on fire.
Example Input:
```js
function Component(props) {
const x = [];
let result;
for (let i = 0; i < 10; i++) {
if (cond) {
try {
result = {key: bar([props.cond && props.foo])};
} catch (e) {
console.log(e);
}
}
}
x.push(result);
return <Stringify x={x} />;
}
```
Command output:
```
$ yarn snap minimize --path .../input.js
Minimizing: .../input.js
Minimizing................
--- Minimized Code ---
function Component(props) {
try {
props && props;
} catch (e) {}
}
Reduced from 16 lines to 5 lines
```
This demonstrates things like:
* Removing one statement at at time
* Replacing if/else with the test, consequent, or alternate. Similar for
other control-flow statements including try/catch
* Removing individual array/object expression properties
* Replacing single-value array/object with the value
* Replacing control-flow expression (logical, consequent) w the test or
left/right values
* Removing call arguments
* Replacing calls with a single argument with the argument
* Replacing calls with multiple arguments with an array of the arguments
* Replacing optional member/call with non-optional versions
* Replacing member expression with the object. If computed, also try
replacing w the key
* And a bunch more strategies, see the code
Adds support for value terminals (optional/logical/ternary/sequence)
within try/catch clauses.
Try/catch expressions insert maybe-throw terminals after each
instruction, but BuildReactiveFunction's value block extraction was not
expecting these terminals. The fix is to roughly treat maybe-throw
similarly to goto, falling through to the continuation block, but there
are a few edge cases to handle.
I've also added extensive tests, including testing that errors correctly
flow to the catch handler.
Fixes missing source locations for ReturnStatement nodes in generated
ast. Simple change using existing pattern, only required changes to the
codegen step, no other pipeline changes.
**Most file changes are new lines in generated code.** [First
commit](d15e90ebe0)
has the relevant changes, second commit has the noisy snap updates.
I added an exception to the validator to not report an error when a
return statement will be optimized to an implicit return by codegen, as
there's no separate return statement to instrument anyways in the final
ast. An edge case when it comes to preserving source locations for
instrumentation that is likely not as common for most babel transforms
since they are not doing optimizations.
In https://github.com/facebook/react/pull/35646 I thought there was a
bug in trusted types, but the bug is in jsdom.
For trusted types we still want to check the coersion and throw for a
good dev warning, but prod will also throw becuase the browser will
implicitly coerce to a string. This ensures there's no behavior
difference between dev and prod.
So the right fix is to add in the JSDOM hack that's used in
`ReactDOMSelect-test.js`.
Stacked on https://github.com/facebook/react/pull/35630
- Adds test case for compareDocumentPosition, missing before and also
extending to text nodes
- Adds event handling fixture case for text
- Adds getRootNode fixture case for text
This PR adds text node support to FragmentInstance operations, allowing
fragment refs to properly handle fragments that contain text nodes
(either mixed with elements or text-only).
Not currently adding/removing new text nodes as we don't need to track
them for events or observers in DOM. Will follow up on this and with
Fabric support.
## Support through parent element
- `dispatchEvent`
- `compareDocumentPosition`
- `getRootNode`
## Support through Range API
- `getClientRects`: Uses Range to calculate bounding rects for text
nodes
- `scrollIntoView`: Uses Range to scroll to text node positions directly
## No support
- `focus`/`focusLast`/`blur`: Noop for text-only fragments
- `observeUsing`: Warns for text-only fragments in DEV
- `addEventListener`/`removeEventListener`: Ignores text nodes, but
still works on Fragment level through `dispatchEvent`
Follow-up to https://github.com/facebook/react/pull/34665.
Already gated on `enableProfilerTimer` everywhere, which is only enabled
for `__PROFILE__`, except for Flight should be unified in a future.
## Overview
Adds a claude setup that works with the nested /compiler setup.
The constraints are:
- when working in the root repo, don't use the compiler configs (easy)
- when working in the compiler/ don't use the parent contigs (hard)
The first one is easy: there's a claude.md and .claude directory in
/compiler that is only loaded when you start a session from /compuler.
The second one is hard, because if you start a session from /compiler,
the parent claude files and skills are loaded.
I was able to deny the permissions to the parent skills in
settings.json, but the descriptions are still loaded into context and I
don't think that's avoidable.
To keep the parent claude file out of context, I created a hook hack: I
moved all the non-compiler claude file context to instructions.md and
added a SessionStart hook to cat the file into context if the cwd isn't
the /compiler. Works well, but won't show it as part of the `/context`
slash command.
## Skills
I also added a number of skills specific to the React repo:
| Skill | Description |
|-------|-------------|
| `/extract-errors` | `yarn extract-errors` |
| `/feature-flags` | how feature flags work and `@gate` |
| `/fix` | linc and prettier |
| `/flags` | `yarn flags` |
| `/flow` | `yarn flow <variant>` |
| `/test` | `yarn test-*` |
| `/verify` | `run all the lints/tests/flow to verify` |
### Example: Flow
| before | after |
|-----|-----|
| <img width="1076" height="442" alt="flow-before"
src="https://github.com/user-attachments/assets/73eec143-d0af-4771-b501-c9dc29cc09ac"
/> | <img width="1076" height="273" alt="flow-after"
src="https://github.com/user-attachments/assets/292d33af-1d98-4252-9c08-744b33e88b86"
/> |
### Example: Tests
| before | after |
|-----|-----|
| <img width="1048" height="607" alt="test-before"
src="https://github.com/user-attachments/assets/aa558ccf-2cee-4d22-b1f1-e4221c5a59dd"
/> | <img width="1075" height="359" alt="test-after"
src="https://github.com/user-attachments/assets/eb795392-6f46-403f-b9bb-8851ed790165"
/> |
Autogenerated summaries of each of the compiler passes which allow
agents to get the key ideas of a compiler pass, including key
input/output invariants, without having to reprocess the file each time.
In the subsequent diff this seemed to help.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35595).
* #35607
* #35298
* #35596
* #35573
* __->__ #35595
* #35539
Much nicer workflow for working through errors in the compiler:
* Run `yarn snap -w`, oops there are are errors
* Hit 'p' to select a fixture => the suggestions populate with recent
failures, sorted alphabetically. No need to copy/paste the name of the
fixture you want to focus on!
* tab/shift-tab to pick one, hit enter to select that one
* ...Focus on fixing that test...
* 'p' to re-enter the picker. Snap tracks the last state of each fixture
and continues to show all tests that failed on their last run, so you
can easily move on to the next one. The currently selected test is
highlighted, making it easy to move to the next one.
* 'a' at any time to run all tests
* 'd' at any time to toggle debug output on/off (while focusing on a
single test)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35539).
* #35607
* #35298
* #35596
* #35573
* #35595
* __->__ #35539
`invariant()` was a pain to use - we always record a single location,
but the API required passing a compiler detail. This PR replaces
`invariant()` (keeping the name) with `simpleInvariant()`s signature,
and updates call sites accordingly. I've noticed that agents
consistently get invariant() wrong, which aligns with it being tedious
to call when you're writing code by hand. The simplified API should help
a bit.
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.
