Skip to content

Commit 152bfe3

Browse files
authored
[compiler][rfc] Hacky retry pipeline for fire (facebook#32164)
Hacky retry pipeline for when transforming `fire(...)` calls encounters validation, todo, or memoization invariant bailouts. Would love feedback on how we implement this to be extensible to other compiler non-memoization features (e.g. inlineJSX) Some observations: - Compiler "front-end" passes (e.g. lower, type, effect, and mutability inferences) should be shared for all compiler features -- memo and otherwise - Many passes (anything dealing with reactive scope ranges, scope blocks / dependencies, and optimizations such as ReactiveIR facebook#31974) can be left out of the retry pipeline. This PR hackily skips memoization features by removing reactive scope creation, but we probably should restructure the pipeline to skip these entirely on a retry - We should maintain a canonical set of "validation flags" Note the newly added fixtures are prefixed with `bailout-...` when the retry fire pipeline is used. These fixture outputs contain correctly inserted `useFire` calls and no memoization.
1 parent 19ca800 commit 152bfe3

21 files changed

+617
-90
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ function runWithEnvironment(
162162
if (
163163
!env.config.enablePreserveExistingManualUseMemo &&
164164
!env.config.disableMemoizationForDebugging &&
165-
!env.config.enableChangeDetectionForDebugging
165+
!env.config.enableChangeDetectionForDebugging &&
166+
!env.config.enableMinimalTransformsForRetry
166167
) {
167168
dropManualMemoization(hir);
168169
log({kind: 'hir', name: 'DropManualMemoization', value: hir});
@@ -279,8 +280,10 @@ function runWithEnvironment(
279280
value: hir,
280281
});
281282

282-
inferReactiveScopeVariables(hir);
283-
log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
283+
if (!env.config.enableMinimalTransformsForRetry) {
284+
inferReactiveScopeVariables(hir);
285+
log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
286+
}
284287

285288
const fbtOperands = memoizeFbtAndMacroOperandsInSameScope(hir);
286289
log({

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

Lines changed: 75 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
EnvironmentConfig,
1717
ExternalFunction,
1818
ReactFunctionType,
19+
MINIMAL_RETRY_CONFIG,
1920
tryParseExternalFunction,
2021
} from '../HIR/Environment';
2122
import {CodegenFunction} from '../ReactiveScopes';
@@ -382,66 +383,92 @@ export function compileProgram(
382383
);
383384
}
384385

385-
let compiledFn: CodegenFunction;
386-
try {
387-
/**
388-
* Note that Babel does not attach comment nodes to nodes; they are dangling off of the
389-
* Program node itself. We need to figure out whether an eslint suppression range
390-
* applies to this function first.
391-
*/
392-
const suppressionsInFunction = filterSuppressionsThatAffectFunction(
393-
suppressions,
394-
fn,
395-
);
396-
if (suppressionsInFunction.length > 0) {
397-
const lintError = suppressionsToCompilerError(suppressionsInFunction);
398-
if (optOutDirectives.length > 0) {
399-
logError(lintError, pass, fn.node.loc ?? null);
400-
} else {
401-
handleError(lintError, pass, fn.node.loc ?? null);
402-
}
403-
return null;
386+
/**
387+
* Note that Babel does not attach comment nodes to nodes; they are dangling off of the
388+
* Program node itself. We need to figure out whether an eslint suppression range
389+
* applies to this function first.
390+
*/
391+
const suppressionsInFunction = filterSuppressionsThatAffectFunction(
392+
suppressions,
393+
fn,
394+
);
395+
let compileResult:
396+
| {kind: 'compile'; compiledFn: CodegenFunction}
397+
| {kind: 'error'; error: unknown};
398+
if (suppressionsInFunction.length > 0) {
399+
compileResult = {
400+
kind: 'error',
401+
error: suppressionsToCompilerError(suppressionsInFunction),
402+
};
403+
} else {
404+
try {
405+
compileResult = {
406+
kind: 'compile',
407+
compiledFn: compileFn(
408+
fn,
409+
environment,
410+
fnType,
411+
useMemoCacheIdentifier.name,
412+
pass.opts.logger,
413+
pass.filename,
414+
pass.code,
415+
),
416+
};
417+
} catch (err) {
418+
compileResult = {kind: 'error', error: err};
404419
}
405-
406-
compiledFn = compileFn(
407-
fn,
408-
environment,
409-
fnType,
410-
useMemoCacheIdentifier.name,
411-
pass.opts.logger,
412-
pass.filename,
413-
pass.code,
414-
);
415-
pass.opts.logger?.logEvent(pass.filename, {
416-
kind: 'CompileSuccess',
417-
fnLoc: fn.node.loc ?? null,
418-
fnName: compiledFn.id?.name ?? null,
419-
memoSlots: compiledFn.memoSlotsUsed,
420-
memoBlocks: compiledFn.memoBlocks,
421-
memoValues: compiledFn.memoValues,
422-
prunedMemoBlocks: compiledFn.prunedMemoBlocks,
423-
prunedMemoValues: compiledFn.prunedMemoValues,
424-
});
425-
} catch (err) {
420+
}
421+
// If non-memoization features are enabled, retry regardless of error kind
422+
if (compileResult.kind === 'error' && environment.enableFire) {
423+
try {
424+
compileResult = {
425+
kind: 'compile',
426+
compiledFn: compileFn(
427+
fn,
428+
{
429+
...environment,
430+
...MINIMAL_RETRY_CONFIG,
431+
},
432+
fnType,
433+
useMemoCacheIdentifier.name,
434+
pass.opts.logger,
435+
pass.filename,
436+
pass.code,
437+
),
438+
};
439+
} catch (err) {
440+
compileResult = {kind: 'error', error: err};
441+
}
442+
}
443+
if (compileResult.kind === 'error') {
426444
/**
427445
* If an opt out directive is present, log only instead of throwing and don't mark as
428446
* containing a critical error.
429447
*/
430-
if (fn.node.body.type === 'BlockStatement') {
431-
if (optOutDirectives.length > 0) {
432-
logError(err, pass, fn.node.loc ?? null);
433-
return null;
434-
}
448+
if (optOutDirectives.length > 0) {
449+
logError(compileResult.error, pass, fn.node.loc ?? null);
450+
} else {
451+
handleError(compileResult.error, pass, fn.node.loc ?? null);
435452
}
436-
handleError(err, pass, fn.node.loc ?? null);
437453
return null;
438454
}
439455

456+
pass.opts.logger?.logEvent(pass.filename, {
457+
kind: 'CompileSuccess',
458+
fnLoc: fn.node.loc ?? null,
459+
fnName: compileResult.compiledFn.id?.name ?? null,
460+
memoSlots: compileResult.compiledFn.memoSlotsUsed,
461+
memoBlocks: compileResult.compiledFn.memoBlocks,
462+
memoValues: compileResult.compiledFn.memoValues,
463+
prunedMemoBlocks: compileResult.compiledFn.prunedMemoBlocks,
464+
prunedMemoValues: compileResult.compiledFn.prunedMemoValues,
465+
});
466+
440467
/**
441468
* Always compile functions with opt in directives.
442469
*/
443470
if (optInDirectives.length > 0) {
444-
return compiledFn;
471+
return compileResult.compiledFn;
445472
} else if (pass.opts.compilationMode === 'annotation') {
446473
/**
447474
* No opt-in directive in annotation mode, so don't insert the compiled function.
@@ -467,7 +494,7 @@ export function compileProgram(
467494
}
468495

469496
if (!pass.opts.noEmit) {
470-
return compiledFn;
497+
return compileResult.compiledFn;
471498
}
472499
return null;
473500
};

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,8 @@ const EnvironmentConfigSchema = z.object({
552552
*/
553553
disableMemoizationForDebugging: z.boolean().default(false),
554554

555+
enableMinimalTransformsForRetry: z.boolean().default(false),
556+
555557
/**
556558
* When true, rather using memoized values, the compiler will always re-compute
557559
* values, and then use a heuristic to compare the memoized value to the newly
@@ -626,6 +628,17 @@ const EnvironmentConfigSchema = z.object({
626628

627629
export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
628630

631+
export const MINIMAL_RETRY_CONFIG: PartialEnvironmentConfig = {
632+
validateHooksUsage: false,
633+
validateRefAccessDuringRender: false,
634+
validateNoSetStateInRender: false,
635+
validateNoSetStateInPassiveEffects: false,
636+
validateNoJSXInTryStatements: false,
637+
validateMemoizedEffectDependencies: false,
638+
validateNoCapitalizedCalls: null,
639+
validateBlocklistedImports: null,
640+
enableMinimalTransformsForRetry: true,
641+
};
629642
/**
630643
* For test fixtures and playground only.
631644
*

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ export default function inferReferenceEffects(
241241

242242
if (options.isFunctionExpression) {
243243
fn.effects = functionEffects;
244-
} else {
244+
} else if (!fn.env.config.enableMinimalTransformsForRetry) {
245245
raiseFunctionEffectErrors(functionEffects);
246246
}
247247
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoCapitalizedCalls @enableFire
6+
import {fire} from 'react';
7+
const CapitalizedCall = require('shared-runtime').sum;
8+
9+
function Component({prop1, bar}) {
10+
const foo = () => {
11+
console.log(prop1);
12+
};
13+
useEffect(() => {
14+
fire(foo(prop1));
15+
fire(foo());
16+
fire(bar());
17+
});
18+
19+
return CapitalizedCall();
20+
}
21+
22+
```
23+
24+
## Code
25+
26+
```javascript
27+
import { useFire } from "react/compiler-runtime"; // @validateNoCapitalizedCalls @enableFire
28+
import { fire } from "react";
29+
const CapitalizedCall = require("shared-runtime").sum;
30+
31+
function Component(t0) {
32+
const { prop1, bar } = t0;
33+
const foo = () => {
34+
console.log(prop1);
35+
};
36+
const t1 = useFire(foo);
37+
const t2 = useFire(bar);
38+
39+
useEffect(() => {
40+
t1(prop1);
41+
t1();
42+
t2();
43+
});
44+
return CapitalizedCall();
45+
}
46+
47+
```
48+
49+
### Eval output
50+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// @validateNoCapitalizedCalls @enableFire
2+
import {fire} from 'react';
3+
const CapitalizedCall = require('shared-runtime').sum;
4+
5+
function Component({prop1, bar}) {
6+
const foo = () => {
7+
console.log(prop1);
8+
};
9+
useEffect(() => {
10+
fire(foo(prop1));
11+
fire(foo());
12+
fire(bar());
13+
});
14+
15+
return CapitalizedCall();
16+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @enableFire
6+
import {useRef} from 'react';
7+
8+
function Component({props, bar}) {
9+
const foo = () => {
10+
console.log(props);
11+
};
12+
useEffect(() => {
13+
fire(foo(props));
14+
fire(foo());
15+
fire(bar());
16+
});
17+
18+
const ref = useRef(null);
19+
// eslint-disable-next-line react-hooks/rules-of-hooks
20+
ref.current = 'bad';
21+
return <button ref={ref} />;
22+
}
23+
24+
```
25+
26+
## Code
27+
28+
```javascript
29+
import { useFire } from "react/compiler-runtime"; // @enableFire
30+
import { useRef } from "react";
31+
32+
function Component(t0) {
33+
const { props, bar } = t0;
34+
const foo = () => {
35+
console.log(props);
36+
};
37+
const t1 = useFire(foo);
38+
const t2 = useFire(bar);
39+
40+
useEffect(() => {
41+
t1(props);
42+
t1();
43+
t2();
44+
});
45+
46+
const ref = useRef(null);
47+
48+
ref.current = "bad";
49+
return <button ref={ref} />;
50+
}
51+
52+
```
53+
54+
### Eval output
55+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// @enableFire
2+
import {useRef} from 'react';
3+
4+
function Component({props, bar}) {
5+
const foo = () => {
6+
console.log(props);
7+
};
8+
useEffect(() => {
9+
fire(foo(props));
10+
fire(foo());
11+
fire(bar());
12+
});
13+
14+
const ref = useRef(null);
15+
// eslint-disable-next-line react-hooks/rules-of-hooks
16+
ref.current = 'bad';
17+
return <button ref={ref} />;
18+
}

0 commit comments

Comments
 (0)