Accidentally broke this when migrating our test runner to use the bundled build https://github.com/facebook/react/pull/32758
The fix is pretty simple. File watcher should listen for changes in `packages/babel-plugin-react-compiler` instead of `cwd`, which is now `packages/snap`.
Currently, inferred effect dependencies are considered a
"compiler-required" feature. This means that untransformed callsites
should escalate to a build error.
`ValidateNoUntransformedReferences` iterates 'special effect' callsites
and checks that the compiler was able to successfully transform them.
Prior to this PR, this relied on checking the number of arguments passed
to this special effect.
This obviously doesn't work with `noEmit: true`, which is used for our
eslint plugin (this avoids mutating the babel program as other linters
run with the same ast). This PR adds a set of `babel.SourceLocation`s to
do best effort matching in this mode.
Typically we mark the name of things that might animate in the snapshot
phase. At the same time we track that should call startViewTransition
too. However, we don't do this for "enter" since they're only marked
later. Leading to having just an "enter" not to animate unless there's
at least another update too.
This tracks if there's a ViewTransitionComponent in the tree that
enters. Luckily we know that from the static flag so we don't have to
traverse it.
Stacked on https://github.com/facebook/react/pull/32734
In React a ViewTransition class of `"none"` doesn't just mean that it
has no class but also that it has no ViewTransition name. The default
(`null | undefined`) means that it has no specific class but should run
with the default built-in animation. This adds this as an explicit
string called `"auto"` as well.
That way you can do `<ViewTransition default="foo" enter="auto">` to
override the "foo" just for the "enter" trigger to be the default
built-in animation. Where as if you just specified `null` it would be
like not specifying enter at all which would trigger "foo".
It was always confusing that this is not a CSS class but a
view-transition-class.
The `className` sticks out a bit among its siblings `enter`, `exit`,
`update` and `share`. The idea is that the most specific definition
override is the class name that gets applied and this prop is really
just the fallback, catch-all or "any" that is applied if you didn't
specify a more specific one.
It has also since evolved not just to take a string but also a map of
Transition Type to strings.
The "class" is really the type of the value. We could add a suffix to
all of them like `defaultClass`, `enterClass`, `exitClass`,
`updateClass` and `shareClass`. However, this doesn't necessarily make
sense with the mapping of Transition Type to string. It also makes it a
bit too DOM centric. In React Native this might still be called a
"class" but it might be represented by an object definition. We might
even allow some kind of inline style form for the DOM too. Really this
is about picking which "animation" that runs which can be a string or
instance. "Animation" is too broad because there's also a concept of a
CSS Animation and these are really sets of CSS animations (group,
image-pair, old, new). It could maybe be `defaultTransition`,
`enterTransition`, etc but that seems unnecessarily repetitive and still
doesn't say anything about it being a class.
We also already have the name "default" in the map of Transition Types.
In fact you can now specify a default for default:
```
<ViewTransition default={{"navigation-back": "slide-out", "default": "fade-in"}}>
```
One thing I don't like about the name `"default"` is that it might be
common to just apply a named class that does it matching to
enter/exit/update in the CSS selectors (such as the `:only-child` rule)
instead of doing that mapping to each one using React. In that can you
end up specifying only `default={...}` a lot and then what is it the
"default" for? It's more like "all". I think it's likely that you end up
with either "default" or the specific forms instead of both at once.
Starting a View Transition is an async sequence. Since React can get a
sync update in the middle of sequence we sometimes interrupt that
sequence.
Currently, we don't actually cancel the View Transition so it can just
run as a partial. This ensures that we fully skip it when that happens,
as well as warn.
However, it's very easy to trigger this with just a setState in
useLayoutEffect right now. Therefore if we're inside the preparing
sequence of a startViewTransition, this delays work that would've
normally flushed in a microtask. ~Maybe we want to do the same for
Default work already scheduled through a scheduler Task.~ Edit: This was
already done.
`flushSync` currently will still lead to an interrupted View Transition
(with a warning). There's a tradeoff here whether we want to try our
best to preserve the guarantees of `flushSync` or favor the animation.
It's already possible to suspend at the root with `flushSync` which
means it's not always 100% guaranteed to commit anyway. We could treat
it as suspended. But let's see how much this is a problem in practice.
Currently, `babel-plugin-react-compiler` is bundled with (almost) all
external dependencies. This is because babel traversal and ast logic is
not forward-compatible. Since `babel-plugin-react-compiler` needs to be
compatible with babel pipelines across a wide semvar range, we (1) set
this package's babel dependency to an early version and (2) inline babel
libraries into our bundle.
A few other packages in `react/compiler` depend on the compiler. This PR
moves `snap`, our test fixture compiler and evaluator, to use the
bundled version of `babel-plugin-react-compiler`. This decouples the
babel version used by `snap` with the version used by
`babel-plugin-react-compiler`, which means that `snap` now can test
features from newer babel versions (see
https://github.com/facebook/react/pull/32742).
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32758).
* #32759
* __->__ #32758
https://github.com/facebook/react/pull/32529 added a dynamic flag for
this, but that breaks tests since the flags are not defined everywhere.
However, this is a static value and the flag is only for supporting
existing tests. So we can override it in the test config, and make it
static at built time instead.
This adds early logging when two ViewTransitions with the same name are
mounted at the same time. Whether they're part of a View Transition or
not.
This lets us include the owner stack of each one. I do two logs so that
you can get the stack trace of each one of the duplicates.
It currently only logs once for each name which also avoids the scenario
when you have many hits for the same name in one commit. However, we
could also possibly log a stack for each of them but seems noisy.
Currently we don't log if a SwipeTransition is the first time the pair
gets mounted which could lead to a View Transition error before we've
warned. That could be a separate improvement.
This got moved into the functional component and class component case
statements here:
https://github.com/facebook/react/commit/0de1233fd180969f7ffdfc98151922f2466ceb1f.
So that we could separate the error case for class components.
However, due to a faulty rebase this got restored at the top as well.
Leading to double component renders being logged.
In the other offscreen reconnect passes we don't do this in each case
statement but still once at the top. The reason this doesn't matter is
because use the PerformedWork flag and that is only set for function and
class components. Although maybe it should be set for expensive DOM
components too and then we have to remember this.
Casing was incorrect.
Tested by running locally with a PAT.
```
$ scripts/release/download-experimental-build.js --commit=2d40460cf768071d3a70b4cdc16075d23ca1ff25
Command failed: gh attestation verify artifacts_combined.zip --repo=facebook/react
Error: failed to fetch attestations from facebook/react: HTTP 404: Not Found (https://api.github.com/repos/facebook/react/attestations/sha256:23d05644f9e49e02cbb441e3932cc4366b261826e58ce222ea249a6b786f0b5f?per_page=30)
`gh attestation verify artifacts_combined.zip --repo=facebook/react` (exited with error code 1)
$ scripts/release/download-experimental-build.js --commit=2d40460cf768071d3a70b4cdc16075d23ca1ff25 --noVerify
⠼ Downloading artifacts from GitHub for commit 2d40460cf7) 5% 0.1m, estimated 1.6m
✓ Downloading artifacts from GitHub for commit 2d40460cf7) 9.5 secs
An experimental build has been downloaded!
You can download this build again by running:
scripts/download-experimental-build.js --commit=2d40460cf768071d3a70b4cdc16075d23ca1ff25
```
We now generate attestations in `process_artifacts_combined` so we can
verify the provenance of the build later in other workflows. However,
this requires `write` permissions for `id-token` and `attestations` so
PRs from forks cannot generate this attestation.
To get around this, I added a `--no-verify` flag to
scripts/release/download-experimental-build.js. This flag is only passed
in `runtime_build_and_test.yml` for the sizebot job, since 1) the
workflow runs in the `pull_request` trigger which has read-only
permissions, and 2) the downloaded artifact is only used for sizebot
calculation, and not actually used.
The flag is explicitly not passed in `runtime_commit_artifacts.yml`
since there we actually use the artifact internally. This is fine as
once a PR lands on main, it will then run the build on that new commit
and generate an attestation.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32738).
* #32739
* __->__ #32738
<!--
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
1. Having a development build for FB will be convenient for fb internal
feature development
2. Add a new checkbox to toggle new internal features added to React
Devtools.
## How did you test this change?
1. yarn test
2. set extra env variables in bash profile and build an internal version
with the new script.
3. toggle on/off the new checkbox, the value is stored in local storage
correctly.
---------
Co-authored-by: Aohua Mu <muaohua@fb.com>
Uses https://cli.github.com/manual/gh_attestation_verify to verify that
the downloaded artifact matches the attestation generated during the
build process in runtime_commit_artifacts.
Example:
On a workflow run of runtime_build_and_test.yml with no attestations:
```
$ scripts/release/download-experimental-build.js --commit=ea5f065745b777cb41cc9e54a3b29ed8c727a574
Command failed: gh attestation verify artifacts_combined.zip --repo=facebook/react
Error: failed to fetch attestations from facebook/react: HTTP 404: Not Found (https://api.github.com/repos/facebook/react/attestations/sha256:7adba0992ba477a927aad5a07f95ee2deb7d18427c84279d33fc40a3bc28ebaa?per_page=30)
`gh attestation verify artifacts_combined.zip --repo=facebook/react` (exited with error code 1)
```
On one which does:
```
$ scripts/release/download-experimental-build.js --commit=12e85d74c1c233cdc2f3228a97473a4435d50c3b
✓ Downloading artifacts from GitHub for commit 12e85d74c1) 10.5 secs
An experimental build has been downloaded!
You can download this build again by running:
scripts/download-experimental-build.js --commit=12e85d74c1c233cdc2f3228a97473a4435d50c3b
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32728).
* #32729
* __->__ #32728
Need this to run against target for forks to get the notification.
This job does not checkout the code in the PR, so it's safe to run from
the target.
Also fixes failing checks on PRs:
<img width="870" alt="Screenshot 2025-03-24 at 3 28 30 PM"
src="https://github.com/user-attachments/assets/add78287-6449-4e48-9376-f3b360d2607c"
/>
(Found when compiling Meta React code)
Let variable declarations and reassignments are currently rewritten to
`StoreLocal <varName>` instructions, which each translates to a new
`const varName` declaration in codegen.
```js
// Example input
function useHook() {
const getX = () => x;
let x = CONSTANT1;
if (cond) {
x += CONSTANT2;
}
return <Stringify getX={getX} />
}
// Compiled output, prior to this PR
import { c as _c } from "react/compiler-runtime";
function useHook() {
const $ = _c(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const getX = () => x;
let x = CONSTANT1;
if (cond) {
let x = x + CONSTANT2;
x;
}
t0 = <Stringify getX={getX} />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
```
This also manifests as a babel internal error when replacing the
original function declaration with the compiler output. The below
compilation output fails with `Duplicate declaration "x" (This is an
error on an internal node. Probably an internal error.)`.
```js
// example input
let x = CONSTANT1;
if (cond) {
x += CONSTANT2;
x = CONSTANT3;
}
// current output
let x = CONSTANT1;
if (playheadDragState) {
let x = x + CONSTANT2
x;
let x = CONSTANT3;
}
```
We currently have the ability to have a separate animation for a
ViewTransition that relayouts but doesn't actually have any internal
mutations. This can be useful if you want to separate just a move from
for example flashing an update.
However, we're concerned that this might be more confusion than its
worth because subtle differences in mutations can cause it to trigger
the other case. The existence of the property name might also make you
start looking for it to solve something that it's not meant for.
We already fallback to using the "update" property if it exists but
layout doesn't. So if we ever decide to add this back it would backwards
compatible. We've also shown in implementation that it can work.
This implements `getRootNode(options)` on fragment instances as the
equivalent of calling `getRootNode` on the fragment's parent host node.
The parent host instance will also be used to proxy dispatchEvent in an
upcoming PR.
Avoid failing builds when imported function specifiers conflict by using
babel's `generateUid`. Failing a build is very disruptive, as it usually
presents to developers similar to a javascript parse error.
```js
import {logRender as _logRender} from 'instrument-runtime';
const logRender = () => { /* local conflicting implementation */ }
function Component_optimized() {
_logRender(); // inserted by compiler
}
```
Currently, we fail builds (even in `panicThreshold:none` cases) when
import specifiers are detected to conflict with existing local
variables. The reason we destructively throw (instead of bailing out) is
because (1) we first generate identifier references to the conflicting
name in compiled functions, (2) replaced original functions with
compiled functions, and then (3) finally check for conflicts.
When we finally check for conflicts, it's too late to bail out.
```js
// import {logRender} from 'instrument-runtime';
const logRender = () => { /* local conflicting implementation */ }
function Component_optimized() {
logRender(); // inserted by compiler
}
```
Adds Effect.ConditionallyMutateIterator, which has the following
effects:
- capture for known array, map, and sets
- mutate for all other values
An alternative to this approach could be to add polymorphic shape
definitions
* Adds `isConstructor: boolean` to `FunctionType`. With this PR, each
typed function can either be a constructor (currently only known
globals) or non constructor. Alternatively, we prefer to encode
polymorphic types / effects (and match the closest subtype)
* Add Map and Set globals + built-ins
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32697).
* #32698
* __->__ #32697
`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.
Mofei Zhang
mofeiZ
Sebastian Markbåge
Ricky
lauren
MU AOHUA
Sam Zhou
Jack Pope
Sebastian "Sebbie" Silbermann
Dimitri POSTOLOV