@samitouri / QOS-React / commits / f457d0b4c6

[compiler][ez] Only fail gating hoisting check for referenced identifiers (#32596)

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

mofeiZ committed Mar 13, 2025 at 12:10 UTC f457d0b4c6dd70c10acb9c93c7d01c80d8e23b92
3 files changed +77 -1
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+1 -1
@@ -1143,7 +1143,7 @@ function checkFunctionReferencedBeforeDeclarationAtTopLevel(
1143 * A null scope means there's no function scope, which means we're at the
1144 * top level scope.
1145 */
1146 - if (scope === null) {
1146 + if (scope === null && id.isReferencedIdentifier()) {
1147 errors.pushErrorDetail(
1148 new CompilerErrorDetail({
1149 reason: `Encountered a function used before its declaration, which breaks Forget's gating codegen due to hoisting`,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-nonreferenced-identifier-collision.expect.md new
+60
@@ -0,0 +1,60 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @gating
6 +import {identity, useHook as useRenamed} from 'shared-runtime';
7 +const _ = {
8 + useHook: () => {},
9 +};
10 +identity(_.useHook);
11 +
12 +function useHook() {
13 + useRenamed();
14 + return <div>hello world!</div>;
15 +}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: useHook,
19 + params: [{}],
20 +};
21 +
22 +```
23 +
24 +## Code
25 +
26 +```javascript
27 +import { isForgetEnabled_Fixtures } from "ReactForgetFeatureFlag";
28 +import { c as _c } from "react/compiler-runtime"; // @gating
29 +import { identity, useHook as useRenamed } from "shared-runtime";
30 +const _ = {
31 + useHook: isForgetEnabled_Fixtures() ? () => {} : () => {},
32 +};
33 +identity(_.useHook);
34 +const useHook = isForgetEnabled_Fixtures()
35 + ? function useHook() {
36 + const $ = _c(1);
37 + useRenamed();
38 + let t0;
39 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = <div>hello world!</div>;
41 + $[0] = t0;
42 + } else {
43 + t0 = $[0];
44 + }
45 + return t0;
46 + }
47 + : function useHook() {
48 + useRenamed();
49 + return <div>hello world!</div>;
50 + };
51 +
52 +export const FIXTURE_ENTRYPOINT = {
53 + fn: useHook,
54 + params: [{}],
55 +};
56 +
57 +```
58 +
59 +### Eval output
60 +(kind: ok) <div>hello world!</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-nonreferenced-identifier-collision.js new
+16
@@ -0,0 +1,16 @@
1 +// @gating
2 +import {identity, useHook as useRenamed} from 'shared-runtime';
3 +const _ = {
4 + useHook: () => {},
5 +};
6 +identity(_.useHook);
7 +
8 +function useHook() {
9 + useRenamed();
10 + return <div>hello world!</div>;
11 +}
12 +
13 +export const FIXTURE_ENTRYPOINT = {
14 + fn: useHook,
15 + params: [{}],
16 +};