[compiler] detect and throw on untransformed required features

Traverse program after running compiler transform to find untransformed references to compiler features (e.g. `inferEffectDeps`, `fire`).

Hard error to fail the babel pipeline when the compiler fails to transform these features to give predictable runtime semantics. Untransformed calls to functions like `fire` will throw at runtime anyways, so let's fail the build to catch these earlier.

Note that with this fails the build *regardless of panicThreshold*

This PR also throws retry pipeline errors, which shows better errors than the current generic TODO message
This commit is contained in:
Mofei Zhang
2025-03-05 10:23:12 -05:00
parent 48c873bb59
commit 4b91f12388
28 changed files with 695 additions and 139 deletions
@@ -11,6 +11,7 @@ import {
injectReanimatedFlag,
pipelineUsesReanimatedPlugin,
} from '../Entrypoint/Reanimated';
import validateNoUntransformedReferences from '../Entrypoint/ValidateNoUntransformedReferences';
const ENABLE_REACT_COMPILER_TIMINGS =
process.env['ENABLE_REACT_COMPILER_TIMINGS'] === '1';
@@ -67,6 +68,7 @@ export default function BabelPluginReactCompiler(
comments: pass.file.ast.comments ?? [],
code: pass.file.code,
});
validateNoUntransformedReferences(prog, opts.environment);
if (ENABLE_REACT_COMPILER_TIMINGS === true) {
performance.mark(`${filename}:end`, {
detail: 'BabelPlugin:Program:end',
@@ -419,7 +419,10 @@ export function compileProgram(
}
}
// If non-memoization features are enabled, retry regardless of error kind
if (compileResult.kind === 'error' && environment.enableFire) {
if (
compileResult.kind === 'error' &&
(environment.enableFire || environment.inferEffectDependencies != null)
) {
try {
compileResult = {
kind: 'compile',
@@ -435,6 +438,7 @@ export function compileProgram(
),
};
} catch (err) {
// TODO: maybe stash the error for later reporting in ValidateNoUntransformedReferences
compileResult = {kind: 'error', error: err};
}
}
@@ -0,0 +1,188 @@
import {NodePath} from '@babel/core';
import * as t from '@babel/types';
import {CompilerError, EnvironmentConfig} from '..';
import {getOrInsertWith} from '../Utils/utils';
import {Environment} from '../HIR';
export default function validateNoUntransformedReferences(
path: NodePath<t.Program>,
env: EnvironmentConfig,
): void {
const moduleLoadChecks = new Map<
string,
Map<string, CheckInvalidReferenceFn>
>();
if (env.enableFire) {
/**
* Error on any untransformed references to `fire` (e.g. including non-call
* expressions)
*/
for (const module of Environment.knownReactModules) {
const react = getOrInsertWith(moduleLoadChecks, module, () => new Map());
react.set('fire', assertNone);
}
}
if (env.inferEffectDependencies) {
/**
* Only error on untransformed references of the form `useMyEffect(...)` or
* `moduleNamespace.useMyEffect(...)`, with matching argument counts.
* TODO: add mode to also hard error on non-call references
*/
for (const {
function: {source, importSpecifierName},
numRequiredArgs,
} of env.inferEffectDependencies) {
const module = getOrInsertWith(moduleLoadChecks, source, () => new Map());
module.set(
importSpecifierName,
assertNoAutoDepCalls.bind(null, numRequiredArgs),
);
}
}
if (moduleLoadChecks.size > 0) {
transformProgram(path, moduleLoadChecks);
}
}
type TraversalState = {
shouldInvalidateScopes: boolean;
program: NodePath<t.Program>;
};
type CheckInvalidReferenceFn = (paths: Array<NodePath<t.Node>>) => void;
function validateImportSpecifier(
specifier: NodePath<t.ImportSpecifier>,
importSpecifierChecks: Map<string, CheckInvalidReferenceFn>,
state: TraversalState,
): void {
const imported = specifier.get('imported');
const specifierName: string =
imported.node.type === 'Identifier'
? imported.node.name
: imported.node.value;
const checkFn = importSpecifierChecks.get(specifierName);
if (checkFn == null) {
return;
}
if (state.shouldInvalidateScopes) {
state.shouldInvalidateScopes = false;
state.program.scope.crawl();
}
const local = specifier.get('local');
const binding = local.scope.getBinding(local.node.name);
CompilerError.invariant(binding != null, {
reason: 'Expected binding to be found for import specifier',
loc: local.node.loc ?? null,
});
checkFn(binding.referencePaths);
}
function validateNamespacedImport(
specifier: NodePath<t.ImportNamespaceSpecifier | t.ImportDefaultSpecifier>,
importSpecifierChecks: Map<string, CheckInvalidReferenceFn>,
state: TraversalState,
): void {
if (state.shouldInvalidateScopes) {
state.shouldInvalidateScopes = false;
state.program.scope.crawl();
}
const local = specifier.get('local');
const binding = local.scope.getBinding(local.node.name);
CompilerError.invariant(binding != null, {
reason: 'Expected binding to be found for import specifier',
loc: local.node.loc ?? null,
});
const filteredReferences = new Map<
CheckInvalidReferenceFn,
Array<NodePath<t.Node>>
>();
for (const reference of binding.referencePaths) {
const parent = reference.parentPath;
if (
parent != null &&
parent.isMemberExpression() &&
parent.get('object') === reference
) {
if (parent.node.computed || parent.node.property.type !== 'Identifier') {
continue;
}
const checkFn = importSpecifierChecks.get(parent.node.property.name);
if (checkFn != null) {
getOrInsertWith(filteredReferences, checkFn, () => []).push(parent);
}
}
}
for (const [checkFn, references] of filteredReferences) {
checkFn(references);
}
}
function transformProgram(
path: NodePath<t.Program>,
moduleLoadChecks: Map<string, Map<string, CheckInvalidReferenceFn>>,
): void {
const traversalState = {shouldInvalidateScopes: true, program: path};
path.traverse({
ImportDeclaration(path: NodePath<t.ImportDeclaration>) {
const importSpecifierChecks = moduleLoadChecks.get(
path.node.source.value,
);
if (importSpecifierChecks == null) {
return;
}
const specifiers = path.get('specifiers');
for (const specifier of specifiers) {
if (specifier.isImportSpecifier()) {
validateImportSpecifier(
specifier,
importSpecifierChecks,
traversalState,
);
} else {
validateNamespacedImport(
specifier as NodePath<
t.ImportNamespaceSpecifier | t.ImportDefaultSpecifier
>,
importSpecifierChecks,
traversalState,
);
}
}
},
});
}
function assertNoAutoDepCalls(
numArgs: number,
paths: Array<NodePath<t.Node>>,
): void {
for (const path of paths) {
const parent = path.parentPath;
if (parent != null && parent.isCallExpression()) {
const args = parent.get('arguments');
if (args.length === numArgs) {
/**
* Note that we cannot easily check the type of the first argument here,
* as it may have already been transformed by the compiler (and not
* memoized).
*/
CompilerError.throwTodo({
reason:
'Untransformed reference to experimental compiler-only feature',
loc: parent.node.loc ?? null,
});
}
}
}
}
function assertNone(paths: Array<NodePath<t.Node>>): void {
if (paths.length > 0) {
CompilerError.throwTodo({
reason: 'Untransformed reference to experimental compiler-only feature',
loc: paths[0].node.loc ?? null,
});
}
}
@@ -1121,6 +1121,7 @@ export class Environment {
moduleName.toLowerCase() === 'react-dom'
);
}
static knownReactModules: ReadonlyArray<string> = ['react', 'react-dom'];
getPropertyType(
receiver: Type,
@@ -0,0 +1,26 @@
## Input
```javascript
// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none)
import {useEffect} from 'react';
function nonReactFn(arg) {
useEffect(() => [1, 2, arg]);
}
```
## Error
```
3 |
4 | function nonReactFn(arg) {
> 5 | useEffect(() => [1, 2, arg]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Todo: Untransformed reference to experimental compiler-only feature (5:5)
6 | }
7 |
```
@@ -0,0 +1,6 @@
// @inferEffectDependencies @compilationMode(infer) @panicThreshold(none)
import {useEffect} from 'react';
function nonReactFn(arg) {
useEffect(() => [1, 2, arg]);
}
@@ -0,0 +1,41 @@
## Input
```javascript
// @inferEffectDependencies @panicThreshold(none)
import {useEffect} from 'react';
/**
* Error on non-inlined effect functions:
* 1. From the effect hook callee's perspective, it only makes sense
* to either
* (a) never hard error (i.e. failing to infer deps is acceptable) or
* (b) always hard error,
* regardless of whether the callback function is an inline fn.
* 2. (Technical detail) it's harder to support detecting cases in which
* function (pre-Forget transform) was inline but becomes memoized
*/
function Component({foo}) {
function f() {
console.log(foo);
}
// No inferred dep array, the argument is not a lambda
useEffect(f);
}
```
## Error
```
18 |
19 | // No inferred dep array, the argument is not a lambda
> 20 | useEffect(f);
| ^^^^^^^^^^^^ Todo: Untransformed reference to experimental compiler-only feature (20:20)
21 | }
22 |
```
@@ -0,0 +1,21 @@
// @inferEffectDependencies @panicThreshold(none)
import {useEffect} from 'react';
/**
* Error on non-inlined effect functions:
* 1. From the effect hook callee's perspective, it only makes sense
* to either
* (a) never hard error (i.e. failing to infer deps is acceptable) or
* (b) always hard error,
* regardless of whether the callback function is an inline fn.
* 2. (Technical detail) it's harder to support detecting cases in which
* function (pre-Forget transform) was inline but becomes memoized
*/
function Component({foo}) {
function f() {
console.log(foo);
}
// No inferred dep array, the argument is not a lambda
useEffect(f);
}
@@ -0,0 +1,27 @@
## Input
```javascript
// @inferEffectDependencies @panicThreshold(none)
import React from 'react';
function NonReactiveDepInEffect() {
const obj = makeObject_Primitives();
React.useEffect(() => print(obj));
}
```
## Error
```
4 | function NonReactiveDepInEffect() {
5 | const obj = makeObject_Primitives();
> 6 | React.useEffect(() => print(obj));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Todo: Untransformed reference to experimental compiler-only feature (6:6)
7 | }
8 |
```
@@ -0,0 +1,51 @@
## Input
```javascript
// @inferEffectDependencies @panicThreshold(none)
import {useSpecialEffect} from 'shared-runtime';
/**
* Note that a react compiler-based transform still has limitations on JS syntax.
* We should surface these as actionable lint / build errors to devs.
*/
function Component({prop1}) {
'use memo';
useSpecialEffect(() => {
try {
console.log(prop1);
} finally {
console.log('exiting');
}
}, [prop1]);
return <div>{prop1}</div>;
}
```
## Error
```
8 | function Component({prop1}) {
9 | 'use memo';
> 10 | useSpecialEffect(() => {
| ^^^^^^^^^^^^^^^^^^^^^^^^
> 11 | try {
| ^^^^^^^^^
> 12 | console.log(prop1);
| ^^^^^^^^^
> 13 | } finally {
| ^^^^^^^^^
> 14 | console.log('exiting');
| ^^^^^^^^^
> 15 | }
| ^^^^^^^^^
> 16 | }, [prop1]);
| ^^^^^^^^^^^^^^ Todo: Untransformed reference to experimental compiler-only feature (10:16)
17 | return <div>{prop1}</div>;
18 | }
19 |
```
@@ -0,0 +1,18 @@
// @inferEffectDependencies @panicThreshold(none)
import {useSpecialEffect} from 'shared-runtime';
/**
* Note that a react compiler-based transform still has limitations on JS syntax.
* We should surface these as actionable lint / build errors to devs.
*/
function Component({prop1}) {
'use memo';
useSpecialEffect(() => {
try {
console.log(prop1);
} finally {
console.log('exiting');
}
}, [prop1]);
return <div>{prop1}</div>;
}
@@ -0,0 +1,27 @@
## Input
```javascript
// @inferEffectDependencies @panicThreshold(none)
import {useEffect} from 'react';
function Component({propVal}) {
'use no memo';
useEffect(() => [propVal]);
}
```
## Error
```
4 | function Component({propVal}) {
5 | 'use no memo';
> 6 | useEffect(() => [propVal]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ Todo: Untransformed reference to experimental compiler-only feature (6:6)
7 | }
8 |
```
@@ -0,0 +1,7 @@
// @inferEffectDependencies @panicThreshold(none)
import {useEffect} from 'react';
function Component({propVal}) {
'use no memo';
useEffect(() => [propVal]);
}
@@ -34,13 +34,6 @@ function Component({foo, bar}) {
console.log(bar.qux);
});
function f() {
console.log(foo);
}
// No inferred dep array, the argument is not a lambda
useEffect(f);
useEffectWrapper(() => {
console.log(foo);
});
@@ -58,7 +51,7 @@ import useEffectWrapper from "useEffectWrapper";
const moduleNonReactive = 0;
function Component(t0) {
const $ = _c(14);
const $ = _c(12);
const { foo, bar } = t0;
const ref = useRef(0);
@@ -119,7 +112,7 @@ function Component(t0) {
useEffect(t4, [bar.baz, bar.qux]);
let t5;
if ($[10] !== foo) {
t5 = function f() {
t5 = () => {
console.log(foo);
};
$[10] = foo;
@@ -127,20 +120,7 @@ function Component(t0) {
} else {
t5 = $[11];
}
const f = t5;
useEffect(f);
let t6;
if ($[12] !== foo) {
t6 = () => {
console.log(foo);
};
$[12] = foo;
$[13] = t6;
} else {
t6 = $[13];
}
useEffectWrapper(t6, [foo]);
useEffectWrapper(t5, [foo]);
}
```
@@ -30,13 +30,6 @@ function Component({foo, bar}) {
console.log(bar.qux);
});
function f() {
console.log(foo);
}
// No inferred dep array, the argument is not a lambda
useEffect(f);
useEffectWrapper(() => {
console.log(foo);
});
@@ -1,44 +0,0 @@
## Input
```javascript
// @inferEffectDependencies
import React from 'react';
function NonReactiveDepInEffect() {
const obj = makeObject_Primitives();
React.useEffect(() => print(obj));
}
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
import React from "react";
function NonReactiveDepInEffect() {
const $ = _c(2);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = makeObject_Primitives();
$[0] = t0;
} else {
t0 = $[0];
}
const obj = t0;
let t1;
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = () => print(obj);
$[1] = t1;
} else {
t1 = $[1];
}
React.useEffect(t1);
}
```
### Eval output
(kind: exception) Fixture not implemented
@@ -0,0 +1,42 @@
## Input
```javascript
// @inferEffectDependencies @panicThreshold(none)
import {useRef} from 'react';
import {useSpecialEffect} from 'shared-runtime';
/**
* The retry pipeline disables memoization features, which means we need to
* provide an alternate implementation of effect dependencies which does not
* rely on memoization.
*/
function useFoo({cond}) {
const ref = useRef();
const derived = cond ? ref.current : makeObject();
useSpecialEffect(() => {
log(derived);
}, [derived]);
return ref;
}
```
## Error
```
11 | const ref = useRef();
12 | const derived = cond ? ref.current : makeObject();
> 13 | useSpecialEffect(() => {
| ^^^^^^^^^^^^^^^^^^^^^^^^
> 14 | log(derived);
| ^^^^^^^^^^^^^^^^^
> 15 | }, [derived]);
| ^^^^^^^^^^^^^^^^ Todo: Untransformed reference to experimental compiler-only feature (13:15)
16 | return ref;
17 | }
18 |
```
@@ -0,0 +1,17 @@
// @inferEffectDependencies @panicThreshold(none)
import {useRef} from 'react';
import {useSpecialEffect} from 'shared-runtime';
/**
* The retry pipeline disables memoization features, which means we need to
* provide an alternate implementation of effect dependencies which does not
* rely on memoization.
*/
function useFoo({cond}) {
const ref = useRef();
const derived = cond ? ref.current : makeObject();
useSpecialEffect(() => {
log(derived);
}, [derived]);
return ref;
}
@@ -2,7 +2,7 @@
## Input
```javascript
// @enableFire
// @enableFire @panicThreshold(none)
import {fire} from 'react';
/**
@@ -29,21 +29,13 @@ function Component({prop1}) {
## Error
```
9 | function Component({prop1}) {
10 | const foo = () => {
> 11 | try {
| ^^^^^
> 12 | console.log(prop1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
> 13 | } finally {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
> 14 | console.log('jbrown215');
| ^^^^^^^^^^^^^^^^^^^^^^^^^
> 15 | }
| ^^^^^^ Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (11:15)
16 | };
17 | useEffect(() => {
18 | fire(foo());
> 18 | fire(foo());
| ^^^^ Todo: Untransformed reference to experimental compiler-only feature (18:18)
19 | });
20 | }
21 |
```
@@ -1,4 +1,4 @@
// @enableFire
// @enableFire @panicThreshold(none)
import {fire} from 'react';
/**
@@ -0,0 +1,23 @@
## Input
```javascript
// @enableFire @panicThreshold(none)
import {fire} from 'react';
console.log(fire == null);
```
## Error
```
2 | import {fire} from 'react';
3 |
> 4 | console.log(fire == null);
| ^^^^ Todo: Untransformed reference to experimental compiler-only feature (4:4)
5 |
```
@@ -0,0 +1,4 @@
// @enableFire @panicThreshold(none)
import {fire} from 'react';
console.log(fire == null);
@@ -0,0 +1,42 @@
## Input
```javascript
// @enableFire @panicThreshold(none)
import {fire} from 'react';
/**
* TODO: we should eventually distinguish between `use no memo` and `use no
* compiler` directives. The former should be used to *only* disable memoization
* features.
*/
function Component({props, bar}) {
'use no memo';
const foo = () => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
fire(foo());
fire(bar());
});
return null;
}
```
## Error
```
13 | };
14 | useEffect(() => {
> 15 | fire(foo(props));
| ^^^^ Todo: Untransformed reference to experimental compiler-only feature (15:15)
16 | fire(foo());
17 | fire(bar());
18 | });
```
@@ -1,6 +1,11 @@
// @enableFire
// @enableFire @panicThreshold(none)
import {fire} from 'react';
/**
* TODO: we should eventually distinguish between `use no memo` and `use no
* compiler` directives. The former should be used to *only* disable memoization
* features.
*/
function Component({props, bar}) {
'use no memo';
const foo = () => {
@@ -0,0 +1,95 @@
## Input
```javascript
// @enableFire @panicThreshold(none)
import {fire} from 'react';
/**
* Compilation of this file should succeed.
*/
function NonFireComponent({prop1}) {
/**
* This component bails out but does not use fire
*/
const foo = () => {
try {
console.log(prop1);
} finally {
console.log('jbrown215');
}
};
useEffect(() => {
foo();
});
}
function FireComponent(props) {
/**
* This component uses fire and compiles successfully
*/
const foo = props => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
});
return null;
}
```
## Code
```javascript
import { useFire } from "react/compiler-runtime";
import { c as _c } from "react/compiler-runtime"; // @enableFire @panicThreshold(none)
import { fire } from "react";
/**
* Compilation of this file should succeed.
*/
function NonFireComponent({ prop1 }) {
/**
* This component bails out but does not use fire
*/
const foo = () => {
try {
console.log(prop1);
} finally {
console.log("jbrown215");
}
};
useEffect(() => {
foo();
});
}
function FireComponent(props) {
const $ = _c(3);
const foo = _temp;
const t0 = useFire(foo);
let t1;
if ($[0] !== props || $[1] !== t0) {
t1 = () => {
t0(props);
};
$[0] = props;
$[1] = t0;
$[2] = t1;
} else {
t1 = $[2];
}
useEffect(t1);
return null;
}
function _temp(props_0) {
console.log(props_0);
}
```
### Eval output
(kind: exception) Fixture not implemented
@@ -0,0 +1,35 @@
// @enableFire @panicThreshold(none)
import {fire} from 'react';
/**
* Compilation of this file should succeed.
*/
function NonFireComponent({prop1}) {
/**
* This component bails out but does not use fire
*/
const foo = () => {
try {
console.log(prop1);
} finally {
console.log('jbrown215');
}
};
useEffect(() => {
foo();
});
}
function FireComponent(props) {
/**
* This component uses fire and compiles successfully
*/
const foo = props => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
});
return null;
}
@@ -1,47 +0,0 @@
## Input
```javascript
// @enableFire
import {fire} from 'react';
function Component({props, bar}) {
'use no memo';
const foo = () => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
fire(foo());
fire(bar());
});
return null;
}
```
## Code
```javascript
// @enableFire
import { fire } from "react";
function Component({ props, bar }) {
"use no memo";
const foo = () => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
fire(foo());
fire(bar());
});
return null;
}
```
### Eval output
(kind: exception) Fixture not implemented