Skip to content

Commit 7f29580

Browse files
committed
[compiler][rfc] Simple retry for fire
Start of a simple retry pipeline. Would love feedback on how we implement this to be extensible to other compiler non-memoization features (e.g. inlineJSX)
1 parent e5a2062 commit 7f29580

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
}
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+
}
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)