Commit Graph

274 Commits

Author SHA1 Message Date
lauren
603e6108f3
[compiler] Update react deps to experimental instead of beta (#31385)
Some tests rely on experimental APIs so let's just use
react@experimental instead of beta
2024-10-29 21:51:41 -04:00
lauren
4abe4b5821
[compiler] Check if local identifier is a hook when resolving globals (#31384)
When resolving import specifiers from the react namespace (`import
{imported as local} from 'react'`), we were previously only checking if
the `imported` identifier was a hook if we didn't already have its
definition in the global registry. We also need to check if `local` is a
hook in the case of aliasing since there may be hook-like APIs in react
that don't start with `use` (eg they are experimental or unstable).

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31384).
* #31385
* __->__ #31384
* #31383
2024-10-29 21:36:48 -04:00
lauren
3928cb00db
[compiler] Ref validation repro for ImportSpecifier with renamed local (#31383)
This was originally reported in
https://github.com/reactwg/react-compiler/discussions/27.

Adding a failing repro to capture this case.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31383).
* #31385
* #31384
* __->__ #31383
2024-10-29 21:36:36 -04:00
Sathya Gunasekaran
02c0e824e4
[compiler][ez] Remove unused param (#31376) 2024-10-28 15:08:27 +00:00
Sathya Gunasekaran
aded0ef831
[compiler] Handle member expr as computed property (#31344)
This PR loosens the restriction on the types of computed properties we
can handle.

Previously, we would disallow anything that is not an identifier because
non-identifiers could be mutating. But member expressions are not
mutating so we can treat them similar to identifiers.
2024-10-28 13:10:01 +00:00
Mike Vitousek
fe04dbcbc4 [compiler] Fix to ref access check to ban ref?.current
ghstack-source-id: ea417a468e
Pull Request resolved: https://github.com/facebook/react/pull/31360
2024-10-25 16:51:36 -07:00
Simen Bekkhus
ae90522bc6
chore: remove unsued deps from React Compiler Babel plugin (#31315)
<!--
  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

Since the Babel plugin is bundled into a single file (except for
`@babel/types`
45804af18d/compiler/packages/babel-plugin-react-compiler/rollup.config.js (L18))
we can move these deps to `devDependencies`.

Main motivation is e.g. not installing ancient version of
`pretty-format` (asked in https://github.com/facebook/react/issues/29062
without getting a reason, but if consumers can just skip the deps
entirely that's even better).

<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->

## How did you test this change?

I tested by installing the plugin into an empty project, deleting
everything in `node_modules` _except_ for `babel-plugin-react-compiler`
and doing `require('babel-plugin-react-compiler')`. It still worked
fine, so it should work in other cases as well 😀

<!--
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.
-->
2024-10-22 13:51:58 -04:00
Joseph Savona
cdde15efe1
[compiler] InlineJSXTransform transforms jsx inside function expressions (#31282)
InlineJSXTransform wasn't traversing into function expressions or object
methods, so any JSX inside such functions wouldn't have gotten inlined.
This PR updates to traverse nested functions to transform all JSX within
a hook or component.

Note that this still doesn't transform JSX outside of components or
hooks, ie in standalone render helpers.
2024-10-18 11:27:48 -07:00
Sathya Gunasekaran
c91b3b090a
JSX Outlining (#30956)
Currently, the react compiler can not compile within callbacks which can
potentially cause over rendering. Consider this example:
```jsx
function Component(countries, onDelete) {
  const name = useFoo();
  return countries.map(() => {
    return (
      <Foo>
        <Bar name={name}/>
        <Baz onclick={onDelete} />
      </Foo>
    );
  });
}
```

In this case, there's no memoization of the nested jsx elements. But
instead if we were to manually refactor the nested jsx into separate
component like this:
```jsx
function Component(countries, onDelete) {
  const name = useFoo();
  return countries.map(() => {
    return <Temp name={name} onDelete={onDelete} />;
  });
}

function Temp({ name, onDelete }) {
  return (
    <Foo>
      <Bar name={name} />
      <Baz onclick={onDelete} />
    </Foo>
  );
}

```

The compiler can now optimise both these components:
```jsx
function Component(countries, onDelete) {
  const $ = _c(4);
  const name = useFoo();
  let t0;
  if ($[0] !== name || $[1] !== onDelete || $[2] !== countries) {
    t0 = countries.map(() => <Temp name={name} onDelete={onDelete} />);
    $[0] = name;
    $[1] = onDelete;
    $[2] = countries;
    $[3] = t0;
  } else {
    t0 = $[3];
  }
  return t0;
}

function Temp(t0) {
  const $ = _c(7);
  const { name, onDelete } = t0;
  let t1;
  if ($[0] !== name) {
    t1 = <Bar name={name} />;
    $[0] = name;
    $[1] = t1;
  } else {
    t1 = $[1];
  }
  let t2;
  if ($[2] !== onDelete) {
    t2 = <Baz onclick={onDelete} />;
    $[2] = onDelete;
    $[3] = t2;
  } else {
    t2 = $[3];
  }
  let t3;
  if ($[4] !== t1 || $[5] !== t2) {
    t3 = (
      <Foo>
        {t1}
        {t2}
      </Foo>
    );
    $[4] = t1;
    $[5] = t2;
    $[6] = t3;
  } else {
    t3 = $[6];
  }
  return t3;
}
```

Now, when `countries` is updated by adding one single value, only the
newly added value is re-rendered and not the entire list. Rather than
having to do this manually, this PR teaches the react compiler to do
this transformation.

This PR adds a new pass (`OutlineJsx`) to capture nested jsx statements
and outline them in a separate component. This newly outlined component
can then by memoized by the compiler, giving us more fine grained
rendering.
2024-10-17 18:15:32 +01:00
Mike Vitousek
147374d71a [compiler] Kill markReactiveIdentifier and friends
Summary:
With the previous PR we no longer need to mark identifiers as reactive in contexts where we don't have places. We already deleted most uses of markReactiveId; the last case was to track identifiers through loadlocals etc -- but we already use a disjoint alias map that accounts for loadlocals when setting reactivity.

ghstack-source-id: 69ce0a78b0
Pull Request resolved: https://github.com/facebook/react/pull/31178
2024-10-11 17:26:57 -07:00
Mike Vitousek
6cf5bd9013 [compiler] Allow refs to be lazily initialized during render
Summary:
The official guidance for useRef notes an exception to the rule that refs cannot be accessed during render: to avoid recreating the ref's contents, you can test that the ref is uninitialized and then initialize it using an if statement:

```
if (ref.current == null) {
  ref.current = SomeExpensiveOperation()
}
```

The compiler didn't recognize this exception, however, leading to code that obeyed all the official guidance for refs being rejected by the compiler. This PR fixes that, by extending the ref validation machinery with an awareness of guard operations that allow lazy initialization. We now understand `== null` and similar operations, when applied to a ref and consumed by an if terminal, as marking the consequent of the if as a block in which the ref can be safely written to. In order to do so we need to create a notion of ref ids, which link different usages of the same ref via both the ref and the ref value.

ghstack-source-id: d2729274f351e1eb0268f28f629fa4c2568ebc4d
Pull Request resolved: https://github.com/facebook/react/pull/31188
2024-10-11 16:14:33 -07:00
Mike Vitousek
7b7fac073d [compiler] Represent phis with places rather than identifiers
Summary:
The fact that phis are identifiers rather than places is unfortunate in a few cases. In some later analyses, we might wish to know whether a phi is reactive, but we don't have an easy way to do that currently.

Most of the changes here is just replacing phi.id with phi.place.identifier and such. Interesting bits are EnterSSA (several functions now take places rather than identifiers, and InferReactivePlaces now needs to mark places as reactive explicitly.

ghstack-source-id: 5f4fb396cd86b421008c37832a5735ac40f8806e
Pull Request resolved: https://github.com/facebook/react/pull/31171
2024-10-10 12:41:03 -07:00
mofeiZ
fbfe37ee40
[compiler] Test fixture: non-reactive phi creates 'dangling ref' scope (#31103) 2024-10-10 12:23:44 -04:00
Mike Vitousek
131ae818a1 [compiler][ez] Include phi identifier in AssertValidMutableRanges
Summary:
Looks like we accidentally skipped validating this identifier.

ghstack-source-id: 05964331a8
Pull Request resolved: https://github.com/facebook/react/pull/31170
2024-10-09 22:15:41 -07:00
lauren
ed966dac4a
[compiler] Fix busted postinstall script (#31147) 2024-10-07 20:37:41 -04:00
lauren
23cd3aca28
[rcr] Remove runtimeModule compiler option (#31145)
Now that the compiler always injects `react-compiler-runtime`, this
option is unnecessary.
2024-10-07 18:07:12 -04:00
lauren
3fd3364107
[rcr] Update default runtimeModule to react-compiler-runtime (#31144)
Updates the compiler to always import from `react-compiler-runtime` by
default. The runtime then decides whether to use the official or
userspace implementation of useMemoCache.
2024-10-07 17:59:33 -04:00
lauren
8dd4cda380
[rcr] Add target flag to compiler (#31143) 2024-10-07 17:50:53 -04:00
lauren
68d5288359
[snap] Add react-compiler-runtime as a dependency (#31142)
We need `react-compiler-runtime` to use the same version of React as
snap
2024-10-07 16:38:06 -04:00
lauren
0e43aa7f7a
[snap] Remove unnecessary React.c override (#31141) 2024-10-07 16:31:59 -04:00
mofeiZ
68d59d43d5
[compiler][ez] Fix reanimated custom type defs for imports (#31137)
When we added support for Reanimated, we didn't distinguish between true
globals (i.e. identifiers with no static resolutions), module types, and
imports #29188. For the past 3-4 months, Reanimated imports were not
being matched to the correct hook / function shape we match globals and
module imports against two different registries.

This PR fixes our support for Reanimated library functions imported
under `react-native-reanimated`. See test fixtures for details
2024-10-07 13:09:39 -04:00
mofeiZ
1460d67c5b
[compiler][hir] Only hoist always-accessed PropertyLoads from function decls (#31066)
Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at
bottom):
* __->__ #31066
* #31032

Prior to this PR, we consider all of a nested function's accessed paths
as 'hoistable' (to the basic block in which the function was defined).
Now, we traverse nested functions and find all paths hoistable to their
*entry block*.

Note that this only replaces the *hoisting* part of function
declarations, not dependencies. This realistically only affects optional
chains within functions, which always get truncated to its inner
non-optional path (see
[todo-infer-function-uncond-optionals-hoisted.tsx](576f3c0aa8/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.tsx))

See newly added test fixtures for details

Update: Note that toggling `enableTreatFunctionDepsAsConditional` makes
a non-trivial impact on granularity of inferred deps (i.e. we find that
function declarations uniquely identify some paths as hoistable).
Snapshot comparison of internal code shows ~2.5% of files get worse
dependencies ([internal
link](https://www.internalfb.com/phabricator/paste/view/P1625792186))
2024-10-03 14:41:32 -04:00
mofeiZ
edacbde73f
[compiler][hir-rewrite] Check mutability of base identifier when hoisting (#31032)
Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at
bottom):
* #31066
* __->__ #31032

Prior to this PR, we check whether the property load source (e.g. the
evaluation of `<base>` in `<base>.property`) is mutable + scoped to
determine whether the property load itself is eligible for hoisting.
This changes to check the base identifier of the load.
- This is needed for the next PR #31066. We want to evaluate whether the
base identifier is mutable within the context of the *outermost
function*. This is because all LoadLocals and PropertyLoads within a
nested function declaration have mutable-ranges within the context of
the function, but the base identifier is a context variable.
- A side effect is that we no longer infer loads from props / other
function arguments as mutable in edge cases (e.g. props escaping out of
try-blocks or being assigned to context variables)
2024-10-03 13:55:59 -04:00
Mofei Zhang
0751fac747 [compiler] Optional chaining for dependencies (HIR rewrite)
Adds HIR version of `PropagateScopeDeps` to handle optional chaining.

Internally, this improves memoization on ~4% of compiled files (internal links: [1](https://www.internalfb.com/intern/paste/P1610406497/))

Summarizing the changes in this PR.
1. `CollectOptionalChainDependencies` recursively traverses optional blocks down to the base. From the base, we build up a set of `baseIdentifier.propertyA?.propertyB` mappings.
The tricky bit here is that optional blocks sometimes reference other optional blocks that are *not* part of the same chain e.g. a(c?.d)?.d. See code + comments in `traverseOptionalBlock` for how we avoid concatenating unrelated blocks.

2. Adding optional chains into non-null object calculation.
(Note that marking `a?.b` as 'non-null' means that `a?.b.c` is safe to evaluate, *not* `(a?.b).c`. Happy to rename this / reword comments accordingly if there's a better term)
This pass is split into two stages. (1) collecting non-null objects by block and (2) propagating non-null objects across blocks. The only significant change here was to (2). We add an extra reduce step `X=Reduce(Union(X, Intersect(X_neighbors)))` to merge optional and non-optional nodes (e.g. nonNulls=`{a, a?.b}` reduces to `{a, a.b}`)

3. Adding optional chains into dependency calculation.
This was the trickiest. We need to take the "maximal" property chain as a dependency. Prior to this PR, we avoided taking subpaths e.g. `a.b` of `a.b.c` as dependencies by only visiting non-PropertyLoad/LoadLocal instructions. This effectively only recorded the property-path at site-of-use.

    Unfortunately, this *quite* doesn't work for optional chains for a few reasons:
    - We would need to skip relevant `StoreLocal`/`Branch terminal` instructions (but only those within optional blocks that have been successfully read).
    - Given an optional chain, either (1) only a subpath or (2) the entire path can be represented as a PropertyLoad. We cannot directly add the last hoistable optional-block as a dependency as MethodCalls are an edge case e.g. given a?.b.c(), we should depend on `a?.b`, not `a?.b.c`
      This means that we add its dependency at either the innermost unhoistable optional-block or when encountering it within its phi-join.

4. Handle optional chains in DeriveMinimalDependenciesHIR.
This was also a bit tricky to formulate. Ideally, we would avoid a 2^3 case join (cond | uncond cfg, optional | not optional load, access | dependency). This PR attempts to simplify by building two trees
    1. First add each hoistable path into a tree containing `Optional | NonOptional` nodes.
    2. Then add each dependency into another tree containing `Optional | NonOptional`, `Access | Dependency` nodes, truncating the dependency at the earliest non-hoistable node (i.e. non-matching pair when walking the hoistable tree)

ghstack-source-id: a2170f26280dfbf65a4893d8a658f863a0fd0c88
Pull Request resolved: https://github.com/facebook/react/pull/31037
2024-10-02 13:30:55 -04:00
Mofei Zhang
c67e241c16 [compiler] Renames and no-op refactor for next PR
Rename for clarity:
- `CollectHoistablePropertyLoads:Tree` -> `CollectHoistablePropertyLoads:PropertyPathRegistry`
    - `getPropertyLoadNode` -> `getOrCreateProperty`
    - `getOrCreateRoot` -> `getOrCreateIdentifier`
- `PropertyLoadNode` -> `PropertyPathNode`

Refactor to CFG joining logic for `CollectHoistablePropertyLoads`. We now write to the same set of inferredNonNullObjects when traversing from entry and exit blocks. This is more correct, as non-nulls inferred from a forward traversal should be included when computing the backward traversal (and vice versa). This fix is needed by an edge case in #31036

Added invariant into fixed-point iteration to terminate (instead of infinite looping).

ghstack-source-id: 1e8eb2d566b649ede93de9a9c13dad09b96416a5
Pull Request resolved: https://github.com/facebook/react/pull/31036
2024-09-30 12:35:16 -04:00
Mofei Zhang
2cbea245cc [compiler][fixtures] Patch error-handling edge case in snap evaluator
Fix edge case in which we incorrectly returned a cached exception instead of trying to rerender with new props.
ghstack-source-id: 843fb85df4a2ae7a88f296104fb16b5f9a34c76e
Pull Request resolved: https://github.com/facebook/react/pull/31082
2024-09-30 12:35:16 -04:00
Mofei Zhang
5d12e9e10b [compiler] repro for dep merging edge case (non-hir)
Found when writing #31037, summary copied from comments:

This is an extreme edge case and not code we'd expect any reasonable developer to write. In most cases e.g. `(a?.b != null ? a.b : DEFAULT)`, we do want to take a dependency on `a?.b`.

I found this trying to come up with edge cases that break the current dependency + CFG merging logic. I think it makes sense to error on the side of correctness. After all, we still take `a` as a dependency if users write `a != null ? a.b : DEFAULT`, and the same fix (understanding the `<hoistable> != null` test expression) works for both. Can be convinced otherwise though!

ghstack-source-id: cc06afda59f7681e228495f5e35a596c20f875f5
Pull Request resolved: https://github.com/facebook/react/pull/31035
2024-09-30 12:35:16 -04:00
Mofei Zhang
58a3ca3b47 [compiler][hir-rewrite] Cleanup Identifier -> IdentifierId
Since removing ExitSSA, Identifier and IdentifierId should mean the same thing

ghstack-source-id: 076cacbe8360e716b0555088043502823f9ee72e
Pull Request resolved: https://github.com/facebook/react/pull/31034
2024-09-30 12:35:16 -04:00
Mofei Zhang
8c89fa7643 [compiler][hir-rewrite] Infer non-null props, destructure source
Followup from #30894.
This adds a new flagged mode `enablePropagateScopeDepsInHIR: "enabled_with_optimizations"`, under which we infer more hoistable loads:
- it's always safe to evaluate loads from `props` (i.e. first parameter of a `component`)
- destructuring sources are safe to evaluate loads from (e.g. given `{x} = obj`, we infer that it's safe to evaluate obj.y)
- computed load sources are safe to evaluate loads from (e.g. given `arr[0]`, we can infer that it's safe to evaluate arr.length)

ghstack-source-id: 32f3bb72e9f85922825579bd785d636f4ccf724d
Pull Request resolved: https://github.com/facebook/react/pull/31033
2024-09-30 12:35:16 -04:00
Mofei Zhang
1a779207a7 [compiler][test fixtures] Add enablePropagateDepsInHIR to forked tests
Annotates fixtures added in #31030 with `@enablePropagateDepsInHIR` to fork behavior (and commit snapshot differences)
ghstack-source-id: e423e8c42db62f1bb87562b770761be09fc8ffc6
Pull Request resolved: https://github.com/facebook/react/pull/31031
2024-09-30 12:35:15 -04:00
Mofei Zhang
943e45e910 [compiler][test fixtures] Fork more fixtures for hir-rewrite
Followup from #30894 , not sure how these got missed. Note that this PR just copies the fixtures without adding `@enablePropagateDepsInHIR`. #31032 follows and actually enables the HIR-version of propagateScopeDeps to run. I split this out into two PRs to make snapshot differences easier to review, but also happy to merge

Fixtures found from locally setting snap test runner to default to `enablePropagateDepsInHIR: 'enabled_baseline'` and forking fixtures files with different output.

ghstack-source-id: 7d7cf41aa923d83ad49f89079171b0411923ce6b
Pull Request resolved: https://github.com/facebook/react/pull/31030
2024-09-30 12:35:15 -04:00
Lauren Tan
db240980a3
[playground] Decouple playground from compiler
Currently the playground is setup as a linked workspace for the
compiler which complicates our yarn workspace setup and means that snap
can sometimes pull in a different version of react than was otherwise
specified.

There's no real reason to have these workspaces combined so let's split
them up.

ghstack-source-id: 56ab064b2f
Pull Request resolved: https://github.com/facebook/react/pull/31081
2024-09-27 15:25:07 -04:00
Lauren Tan
3edc000d77
[compiler] Fix broken tests
ghstack-source-id: 000a37ae1f819eef676dcd52410d5231cd2d50fe
Pull Request resolved: https://github.com/facebook/react/pull/31078
2024-09-26 17:44:28 -04:00
Jack Pope
632f88df11
[compiler] Allow ReactElement symbol to be configured when inlining jsx (#30996)
Based on https://github.com/facebook/react/pull/30995 ([rendered
diff](https://github.com/jackpope/react/compare/inline-jsx-2...jackpope:react:inline-jsx-3?expand=1))

____

Some apps still use `react.element` symbols. Not only do we want to test
there but we also want to be able to upgrade those sites to
`react.transitional.element` without blocking on the compiler (we can
change the symbol feature flag and compiler config at the same time).

The compiler runtime uses `react.transitional.element`, so the snap
fixture will fail if we change the default here. However I confirmed
that commenting out the fixture entrypoint and running snap with
`react.element` will update the fixture symbols as expected.
2024-09-19 10:34:24 -04:00
Jack Pope
d5e955d3c0
[compiler] Pass through unmodified props spread when inlining jsx (#30995)
If JSX receives a props spread without additional attributes (besides
`ref` and `key`), we can pass the spread object as a property directly
to avoid the extra object copy.

```
<Test {...propsToSpread} />
// {props: propsToSpread}
<Test {...propsToSpread} a="z" />
// {props: {...propsToSpread, a: "z"}}
```
2024-09-19 10:07:29 -04:00
Jack Pope
5dcb009760
[compiler] Add JSX inlining optimization (#30867)
This adds an `InlineJsxTransform` optimization pass, toggled by the
`enableInlineJsxTransform` flag. When enabled, JSX will be transformed
into React Element object literals, preventing runtime overhead during
element creation.

TODO:
- [ ] Add conditionals to make transform PROD-only
- [ ] Make the React element symbol configurable so this works with
runtimes that support `react.element` or `react.transitional.element`
- [ ] Look into additional optimization to pass props spread through
directly if none of the properties are mutated
2024-09-18 11:51:36 -04:00
Mike Vitousek
7b56a54298 [compiler][playground] create playground API in pipeline, and allow spaces in pass names
Summary:
1. Minor refactor to provide a stable API for calling the compiler from the playground
2. Allows spaces in pass names without breaking the appearance of the playground by replacing spaces with &nbsp; in pass tabs

ghstack-source-id: 12a43ad86c16c0e21f3e6b4086d531cdefd893eb
Pull Request resolved: https://github.com/facebook/react/pull/30988
2024-09-17 11:05:59 -07:00
Mike Vitousek
d7167c3505 [compiler] Implement support for hoisted and recursive functions
Summary:
Introduces a new binding kind for functions that allows them to be hoisted. Also has the result of causing all nested function declarations to be outputted as function declarations, not as let bindings.

ghstack-source-id: fa40d4909fb3d30c23691e36510ebb3c3cc41053
Pull Request resolved: https://github.com/facebook/react/pull/30922
2024-09-16 11:12:58 -07:00
Mike Vitousek
e78c9362c0 [compiler] Allow all hooks to take callbacks which access refs, but ban hooks from taking direct ref value arguments
Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans *directly* passing a ref.current value to a hook, which was previously allowed.

ghstack-source-id: e66ce7123e
Pull Request resolved: https://github.com/facebook/react/pull/30917
2024-09-16 10:53:34 -07:00
Mike Vitousek
1e68a0a3ae [compiler] Improve handling of refs
Summary:
This change expands our handling of refs to build an understanding of nested refs within objects and functions that may return refs. It builds a special-purpose type system within the ref analysis that gives a very lightweight structural type to objects and array expressions (merging the types of all their members), and then propagating those types throughout the analysis (e.g., if `ref` has type `Ref`, then `{ x: ref }` and `[ref]` have type `Structural(value=Ref)` and `{x: ref}.anything` and `[ref][anything]` have type `Ref`).

This allows us to support structures that contain refs, and functions that operate over them, being created and passed around during rendering without at runtime accessing a ref value.

The analysis here uses a fixpoint to allow types to be fully propagated through the system, and we defend against diverging by widening the type of a variable if it could grow infinitely: so, in something like
```
let x = ref;
while (condition) {
  x = [x]
}
```
we end up giving `x` the type `Structural(value=Ref)`.

ghstack-source-id: afb0b0cb01
Pull Request resolved: https://github.com/facebook/react/pull/30902
2024-09-16 10:53:32 -07:00
Mike Vitousek
c8a7cab13f [compiler] Fix issue where second argument of all functions was considered to be a ref
ghstack-source-id: 1817f3b816
Pull Request resolved: https://github.com/facebook/react/pull/30912
2024-09-16 10:53:29 -07:00
Mike Vitousek
633a0fe536 [compiler] Factor out function effects from reference effects
Summary:
This PR performs a major refactor of InferReferenceEffects to separate out the work on marking places with Effects from inferring FunctionEffects. The behavior should be identical after this change (see [internal sync](https://www.internalfb.com/intern/everpaste/?handle=GN74VxscnUaztTYDAL8q0CRWBIxibsIXAAAB)) but the FunctionEffect logic should be easier to work with.

These analyses are unfortunately still deeply linked--the FunctionEffect analysis needs to reason about the "current" value kind for each point in the program, while the InferReferenceEffects algorithm performs global updates on the state of the program (e.g. freezing). In the future, it might be possible to make these entirely separate passes if we store the ValueKind directly on places.

For the most part, the logic of reference effects and function effects can be cleanly separated: for each instruction and terminal, we visit its places and infer their effects, and then we visit its places and infer any function effects that they cause. The biggest wrinkle here is that when a transitive function freeze operation occurs, it has to happen *after* inferring the function effects on the place, because otherwise we may convert a value from Context to Frozen, which will cause the ContextualMutation function effect to be converted to a ReactMutation effect too early. This can be observed in a case like this:

```
export default component C() {
  foo(() => {
    const p = {};
    return () => {
      p['a'] = 1
    };
  });
}
```
Here when the outer function returns the inner function, it freezes the inner function which transitively freezes `p`. But before that freeze happens, we need to replay the ContextualMutation on the inner function to determine that the value is mutable in the outer context. If we froze `p` first, we would instead convert the ContextualMutation to a ReactMutation and error.

To handle this, InferReferenceEffects now delays the exection of the freezeValue action until after it's called the helper functions that generate function effects. So the order of operations on a given place is now

set effect --> generate function effects --> transitively freeze dependencies, if applicable

ghstack-source-id: 21cb50c14054e7e7a307acb595ef30b54c2f2a52
Pull Request resolved: https://github.com/facebook/react/pull/30920
2024-09-13 12:38:17 -07:00
Mofei Zhang
206df66e70 [compiler][rewrite] PropagateScopeDeps hir rewrite
Resubmission of #30079 -- core logic unchanged, but needed to rebase past #30573

### Quick background
#### Temporaries

The compiler currently treats temporaries and named variables (e.g. `x`) differently in this pass.
- named variables may be reassigned (in fact, since we're running after LeaveSSA, a single named identifier's IdentifierId may map to multiple `Identifier` instances -- each with its own scope and mutable range)
- temporaries are replaced with their represented expressions during codegen. This is correct (mostly correct, see #29878) as we're careful to always lower the correct evaluation semantics. However, since we rewrite reactive scopes entirely (to if/else blocks), we need to track temporaries that a scope produces in `ReactiveScope.declarations` and later promote them to named variables.
In the same example, $4, $5, and $6 need to be promoted: $2 ->`t0`,  $5 ->`t1`, and $6 ->`t2`.
```js
[1] $2 = LoadGlobal(global) foo
[2] $3 = LoadLocal bar$1
scope 0:
  [3] $4 = Call $2(<unknown> $3)
scope 1:
  [4] $5 = Object {  }
scope 2:
  [5] $6 = Object { a: $4, b: $5 }
[6] $8 = StoreLocal Const x$7 = $6
```

#### Dependencies
`ReactiveScope.dependencies` records the set of (read-only) values that a reactive scope is dependent on. This is currently limited to just variables (named variables from source and promoted temporaries) and property-loads.
All dependencies we record need to be hoistable -- i.e. reordered to just before the ReactiveScope begins. Not all PropertyLoads are hoistable.

In this example, we should not evaluate `obj.a.b` without before creating x and checking `objIsNull`.
```js
// reduce-reactive-deps/no-uncond.js
function useFoo({ obj, objIsNull }) {
  const x = [];
  if (isFalse(objIsNull)) {
    x.push(obj.a.b);
  }
  return x;
}
```

While other memoization strategies with different constraints exist, the current compiler requires that `ReactiveScope.dependencies` be re-orderable to the beginning of the reactive scope. But.. `PropertyLoad`s from null values will throw `TypeError`. This means that evaluating hoisted dependencies should throw if and only if the source program throws. (It is also a bug if source throws and compiler output does not throw. See https://github.com/facebook/react-forget/pull/2709)

---
### Rough high level overview
1. Pass 1
Walk over instructions to gather every temporary used outside of its defining scope (same as ReactiveFunction version). These determine the sidemaps we produce, as temporaries used outside of their declaring scopes get promoted to named variables later (and are not considered hoistable rvals).
2. Pass 2 (collectTemporariesSidemap)
Walk over instructions to generate a sidemap of temporary identifier -> named variable and property path (e.g. `$3 -> {obj: props, path: ["a", "b"]}`)
2. Pass 2 (collectHoistablePropertyLoads)
  a. Build a sidemap of block -> accessed variables and properties (e.g. `bb0 -> [ {obj: props, path: ["a", "b"]} ]`)
  b. Propagate "non-nullness" i.e. variables and properties for which we can safely evaluate `PropertyLoad`.
  A basic block can unconditionally read from identifier X if any of the following applies:
    - the block itself reads from identifier X
    - all predecessors of the block read from identifier X
    - all successors of the block read from identifier X
4. Pass 3: (collectDependencies)
Walks over instructions again to record dependencies and declarations, using the previously produced sidemaps. We do not record any control-flow here
5. Merge every scope's recorded dependencies with the set of hoistable PropertyLoads

Tested by syncing internally and (1) checking compilation output differences ([internal link](https://www.internalfb.com/intern/everpaste/?handle=GPCfUBt_HCoy_S4EAJDVFJyJJMR0bsIXAAAB)), running internally e2e tests ([internal link](https://fburl.com/sandcastle/cs5mlkxq))

---
### Followups:
1. Rewrite function expression deps
This change produces much more optimal output as the compiler now uses the function CFG to understand which variables / paths are assumed to be non-null. However, it may exacerbate [this function-expr hoisting bug](https://github.com/facebook/react/blob/main/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx). A short term fix here is to simply call some form of `collectNonNullObjects` on every function expression to find hoistable variable / paths. In the longer term, we should refactor out `FunctionExpression.deps`.

2. Enable optional paths
(a) don't count optional load temporaries as dependencies (e.g. `collectOptionalLoadRValues(...)`).
(b) record optional paths in both collectHoistablePropertyLoads and dependency collection

ghstack-source-id: 2507f6ea751dce09ad1dccd353ae6fc7cf411582
Pull Request resolved: https://github.com/facebook/react/pull/30894
2024-09-12 17:40:57 -04:00
Mofei Zhang
5ac4034e14 [compiler] Fork fixtures for enablePropagateDepsInHIR
- flip `enablePropagateDepsInHIR` to off by default
- fork fixtures which produce compilation differences in #30894 to separate directory `propagate-scope-deps-hir-fork`, to be cleaned up when we remove this flag

ghstack-source-id: 7d5b8dc29788a65c272c846af9877b09fbf2cd60
Pull Request resolved: https://github.com/facebook/react/pull/30949
2024-09-12 17:40:57 -04:00
Mofei Zhang
f6dcce5199 [compiler][ez] Add entrypoints to ssa fixtures
Adds evaluator support for a few compiler test fixtures

ghstack-source-id: 202654992a9876cea59885b54a338c908e369ddb
Pull Request resolved: https://github.com/facebook/react/pull/30948
2024-09-12 17:40:57 -04:00
Joe Savona
d724ba96ff [compiler] Validate type configs for hooks/non-hooks
Alternative to #30868. The goal is to ensure that the types coming out of moduleTypeProvider are valid wrt to hook typing. If something is named like a hook, then it must be typed as a hook (or don't type it).

ghstack-source-id: 3e8b5a0a7010d0c484bbb417fb258e76bf4e32bc
Pull Request resolved: https://github.com/facebook/react/pull/30888
2024-09-10 09:25:28 -07:00
Mofei Zhang
bd788b4180 [compiler] Add enablePropagateDepsInHIR flag
Adding new feature flag in preparation for #30894

ghstack-source-id: 59278028cf178d6b2c28732ded751e9c838183d2
Pull Request resolved: https://github.com/facebook/react/pull/30893
2024-09-06 13:02:08 -04:00
Mofei Zhang
7b98a168fd [compiler][cleanup] Delete now-unused reactive scope fork
Followup to #30891

ghstack-source-id: 6b42055b5d28da39d99a235bcd86a82eb7c270f4
Pull Request resolved: https://github.com/facebook/react/pull/30892
2024-09-06 13:02:08 -04:00
Mofei Zhang
43264a61d0 [compiler][cleanup] Remove unused enableReactiveScopesInHIR flag
Reactive scopes in HIR has been stable for over 3 months now and is the future direction of react compiler, removing this flag to reduce implementation forks.

ghstack-source-id: 65cdf63cf76029fa22d40fd85aba0ac976dcfc08
Pull Request resolved: https://github.com/facebook/react/pull/30891
2024-09-06 13:02:08 -04:00
Joe Savona
f820f5a8b6 [compiler] Type inference for tagged template literals
At Meta we have a pattern of using tagged template literals for features that are compiled away:

```
// Relay:
graphql`...graphql text...`
```

In many cases these tags produce a primitive value, and we can get even more optimal output if we can tell the compiler about these types. The new moduleTypeProvider gives us the ability to declare such types, this PR extends the compiler to use this type information for TaggedTemplateExpression values.

ghstack-source-id: 3cd6511b7f4e708bcb86f3f3fde5773bc51c7197
Pull Request resolved: https://github.com/facebook/react/pull/30869
2024-09-04 13:28:33 -07:00