Skip to content

Commit 5398b71

Browse files
authored
[compiler] detect and throw on untransformed required features (#32512)
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*
1 parent f3c9560 commit 5398b71

31 files changed

+864
-146
lines changed

compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
injectReanimatedFlag,
1212
pipelineUsesReanimatedPlugin,
1313
} from '../Entrypoint/Reanimated';
14+
import validateNoUntransformedReferences from '../Entrypoint/ValidateNoUntransformedReferences';
1415

1516
const ENABLE_REACT_COMPILER_TIMINGS =
1617
process.env['ENABLE_REACT_COMPILER_TIMINGS'] === '1';
@@ -61,12 +62,19 @@ export default function BabelPluginReactCompiler(
6162
},
6263
};
6364
}
64-
compileProgram(prog, {
65+
const result = compileProgram(prog, {
6566
opts,
6667
filename: pass.filename ?? null,
6768
comments: pass.file.ast.comments ?? [],
6869
code: pass.file.code,
6970
});
71+
validateNoUntransformedReferences(
72+
prog,
73+
pass.filename ?? null,
74+
opts.logger,
75+
opts.environment,
76+
result?.retryErrors ?? [],
77+
);
7078
if (ENABLE_REACT_COMPILER_TIMINGS === true) {
7179
performance.mark(`${filename}:end`, {
7280
detail: 'BabelPlugin:Program:end',

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,9 @@ function isFilePartOfSources(
271271
return false;
272272
}
273273

274+
type CompileProgramResult = {
275+
retryErrors: Array<{fn: BabelFn; error: CompilerError}>;
276+
};
274277
/**
275278
* `compileProgram` is directly invoked by the react-compiler babel plugin, so
276279
* exceptions thrown by this function will fail the babel build.
@@ -285,16 +288,16 @@ function isFilePartOfSources(
285288
export function compileProgram(
286289
program: NodePath<t.Program>,
287290
pass: CompilerPass,
288-
): void {
291+
): CompileProgramResult | null {
289292
if (shouldSkipCompilation(program, pass)) {
290-
return;
293+
return null;
291294
}
292295

293296
const environment = pass.opts.environment;
294297
const restrictedImportsErr = validateRestrictedImports(program, environment);
295298
if (restrictedImportsErr) {
296299
handleError(restrictedImportsErr, pass, null);
297-
return;
300+
return null;
298301
}
299302
const useMemoCacheIdentifier = program.scope.generateUidIdentifier('c');
300303

@@ -365,7 +368,7 @@ export function compileProgram(
365368
filename: pass.filename ?? null,
366369
},
367370
);
368-
371+
const retryErrors: Array<{fn: BabelFn; error: CompilerError}> = [];
369372
const processFn = (
370373
fn: BabelFn,
371374
fnType: ReactFunctionType,
@@ -429,7 +432,9 @@ export function compileProgram(
429432
handleError(compileResult.error, pass, fn.node.loc ?? null);
430433
}
431434
// If non-memoization features are enabled, retry regardless of error kind
432-
if (!environment.enableFire) {
435+
if (
436+
!(environment.enableFire || environment.inferEffectDependencies != null)
437+
) {
433438
return null;
434439
}
435440
try {
@@ -448,6 +453,9 @@ export function compileProgram(
448453
};
449454
} catch (err) {
450455
// TODO: we might want to log error here, but this will also result in duplicate logging
456+
if (err instanceof CompilerError) {
457+
retryErrors.push({fn, error: err});
458+
}
451459
return null;
452460
}
453461
}
@@ -538,7 +546,7 @@ export function compileProgram(
538546
program.node.directives,
539547
);
540548
if (moduleScopeOptOutDirectives.length > 0) {
541-
return;
549+
return null;
542550
}
543551
let gating: null | {
544552
gatingFn: ExternalFunction;
@@ -596,7 +604,7 @@ export function compileProgram(
596604
}
597605
} catch (err) {
598606
handleError(err, pass, null);
599-
return;
607+
return null;
600608
}
601609

602610
/*
@@ -638,6 +646,7 @@ export function compileProgram(
638646
}
639647
addImportsToProgram(program, externalFunctions);
640648
}
649+
return {retryErrors};
641650
}
642651

643652
function shouldSkipCompilation(
Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
import {NodePath} from '@babel/core';
2+
import * as t from '@babel/types';
3+
4+
import {
5+
CompilerError,
6+
CompilerErrorDetailOptions,
7+
EnvironmentConfig,
8+
ErrorSeverity,
9+
Logger,
10+
} from '..';
11+
import {getOrInsertWith} from '../Utils/utils';
12+
import {Environment} from '../HIR';
13+
import {DEFAULT_EXPORT} from '../HIR/Environment';
14+
15+
function throwInvalidReact(
16+
options: Omit<CompilerErrorDetailOptions, 'severity'>,
17+
{logger, filename}: TraversalState,
18+
): never {
19+
const detail: CompilerErrorDetailOptions = {
20+
...options,
21+
severity: ErrorSeverity.InvalidReact,
22+
};
23+
logger?.logEvent(filename, {
24+
kind: 'CompileError',
25+
fnLoc: null,
26+
detail,
27+
});
28+
CompilerError.throw(detail);
29+
}
30+
function assertValidEffectImportReference(
31+
numArgs: number,
32+
paths: Array<NodePath<t.Node>>,
33+
context: TraversalState,
34+
): void {
35+
for (const path of paths) {
36+
const parent = path.parentPath;
37+
if (parent != null && parent.isCallExpression()) {
38+
const args = parent.get('arguments');
39+
/**
40+
* Only error on untransformed references of the form `useMyEffect(...)`
41+
* or `moduleNamespace.useMyEffect(...)`, with matching argument counts.
42+
* TODO: do we also want a mode to also hard error on non-call references?
43+
*/
44+
if (args.length === numArgs) {
45+
const maybeErrorDiagnostic = matchCompilerDiagnostic(
46+
path,
47+
context.transformErrors,
48+
);
49+
/**
50+
* Note that we cannot easily check the type of the first argument here,
51+
* as it may have already been transformed by the compiler (and not
52+
* memoized).
53+
*/
54+
throwInvalidReact(
55+
{
56+
reason:
57+
'[InferEffectDependencies] React Compiler is unable to infer dependencies of this effect. ' +
58+
'This will break your build! ' +
59+
'To resolve, either pass your own dependency array or fix reported compiler bailout diagnostics.',
60+
description: maybeErrorDiagnostic
61+
? `(Bailout reason: ${maybeErrorDiagnostic})`
62+
: null,
63+
loc: parent.node.loc ?? null,
64+
},
65+
context,
66+
);
67+
}
68+
}
69+
}
70+
}
71+
72+
function assertValidFireImportReference(
73+
paths: Array<NodePath<t.Node>>,
74+
context: TraversalState,
75+
): void {
76+
if (paths.length > 0) {
77+
const maybeErrorDiagnostic = matchCompilerDiagnostic(
78+
paths[0],
79+
context.transformErrors,
80+
);
81+
throwInvalidReact(
82+
{
83+
reason:
84+
'[Fire] Untransformed reference to compiler-required feature. ' +
85+
'Either remove this `fire` call or ensure it is successfully transformed by the compiler',
86+
description: maybeErrorDiagnostic
87+
? `(Bailout reason: ${maybeErrorDiagnostic})`
88+
: null,
89+
loc: paths[0].node.loc ?? null,
90+
},
91+
context,
92+
);
93+
}
94+
}
95+
export default function validateNoUntransformedReferences(
96+
path: NodePath<t.Program>,
97+
filename: string | null,
98+
logger: Logger | null,
99+
env: EnvironmentConfig,
100+
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>,
101+
): void {
102+
const moduleLoadChecks = new Map<
103+
string,
104+
Map<string, CheckInvalidReferenceFn>
105+
>();
106+
if (env.enableFire) {
107+
/**
108+
* Error on any untransformed references to `fire` (e.g. including non-call
109+
* expressions)
110+
*/
111+
for (const module of Environment.knownReactModules) {
112+
const react = getOrInsertWith(moduleLoadChecks, module, () => new Map());
113+
react.set('fire', assertValidFireImportReference);
114+
}
115+
}
116+
if (env.inferEffectDependencies) {
117+
for (const {
118+
function: {source, importSpecifierName},
119+
numRequiredArgs,
120+
} of env.inferEffectDependencies) {
121+
const module = getOrInsertWith(moduleLoadChecks, source, () => new Map());
122+
module.set(
123+
importSpecifierName,
124+
assertValidEffectImportReference.bind(null, numRequiredArgs),
125+
);
126+
}
127+
}
128+
if (moduleLoadChecks.size > 0) {
129+
transformProgram(path, moduleLoadChecks, filename, logger, transformErrors);
130+
}
131+
}
132+
133+
type TraversalState = {
134+
shouldInvalidateScopes: boolean;
135+
program: NodePath<t.Program>;
136+
logger: Logger | null;
137+
filename: string | null;
138+
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>;
139+
};
140+
type CheckInvalidReferenceFn = (
141+
paths: Array<NodePath<t.Node>>,
142+
context: TraversalState,
143+
) => void;
144+
145+
function validateImportSpecifier(
146+
specifier: NodePath<t.ImportSpecifier>,
147+
importSpecifierChecks: Map<string, CheckInvalidReferenceFn>,
148+
state: TraversalState,
149+
): void {
150+
const imported = specifier.get('imported');
151+
const specifierName: string =
152+
imported.node.type === 'Identifier'
153+
? imported.node.name
154+
: imported.node.value;
155+
const checkFn = importSpecifierChecks.get(specifierName);
156+
if (checkFn == null) {
157+
return;
158+
}
159+
if (state.shouldInvalidateScopes) {
160+
state.shouldInvalidateScopes = false;
161+
state.program.scope.crawl();
162+
}
163+
164+
const local = specifier.get('local');
165+
const binding = local.scope.getBinding(local.node.name);
166+
CompilerError.invariant(binding != null, {
167+
reason: 'Expected binding to be found for import specifier',
168+
loc: local.node.loc ?? null,
169+
});
170+
checkFn(binding.referencePaths, state);
171+
}
172+
173+
function validateNamespacedImport(
174+
specifier: NodePath<t.ImportNamespaceSpecifier | t.ImportDefaultSpecifier>,
175+
importSpecifierChecks: Map<string, CheckInvalidReferenceFn>,
176+
state: TraversalState,
177+
): void {
178+
if (state.shouldInvalidateScopes) {
179+
state.shouldInvalidateScopes = false;
180+
state.program.scope.crawl();
181+
}
182+
const local = specifier.get('local');
183+
const binding = local.scope.getBinding(local.node.name);
184+
const defaultCheckFn = importSpecifierChecks.get(DEFAULT_EXPORT);
185+
186+
CompilerError.invariant(binding != null, {
187+
reason: 'Expected binding to be found for import specifier',
188+
loc: local.node.loc ?? null,
189+
});
190+
const filteredReferences = new Map<
191+
CheckInvalidReferenceFn,
192+
Array<NodePath<t.Node>>
193+
>();
194+
for (const reference of binding.referencePaths) {
195+
if (defaultCheckFn != null) {
196+
getOrInsertWith(filteredReferences, defaultCheckFn, () => []).push(
197+
reference,
198+
);
199+
}
200+
const parent = reference.parentPath;
201+
if (
202+
parent != null &&
203+
parent.isMemberExpression() &&
204+
parent.get('object') === reference
205+
) {
206+
if (parent.node.computed || parent.node.property.type !== 'Identifier') {
207+
continue;
208+
}
209+
const checkFn = importSpecifierChecks.get(parent.node.property.name);
210+
if (checkFn != null) {
211+
getOrInsertWith(filteredReferences, checkFn, () => []).push(parent);
212+
}
213+
}
214+
}
215+
216+
for (const [checkFn, references] of filteredReferences) {
217+
checkFn(references, state);
218+
}
219+
}
220+
function transformProgram(
221+
path: NodePath<t.Program>,
222+
223+
moduleLoadChecks: Map<string, Map<string, CheckInvalidReferenceFn>>,
224+
filename: string | null,
225+
logger: Logger | null,
226+
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>,
227+
): void {
228+
const traversalState: TraversalState = {
229+
shouldInvalidateScopes: true,
230+
program: path,
231+
filename,
232+
logger,
233+
transformErrors,
234+
};
235+
path.traverse({
236+
ImportDeclaration(path: NodePath<t.ImportDeclaration>) {
237+
const importSpecifierChecks = moduleLoadChecks.get(
238+
path.node.source.value,
239+
);
240+
if (importSpecifierChecks == null) {
241+
return;
242+
}
243+
const specifiers = path.get('specifiers');
244+
for (const specifier of specifiers) {
245+
if (specifier.isImportSpecifier()) {
246+
validateImportSpecifier(
247+
specifier,
248+
importSpecifierChecks,
249+
traversalState,
250+
);
251+
} else {
252+
validateNamespacedImport(
253+
specifier as NodePath<
254+
t.ImportNamespaceSpecifier | t.ImportDefaultSpecifier
255+
>,
256+
importSpecifierChecks,
257+
traversalState,
258+
);
259+
}
260+
}
261+
},
262+
});
263+
}
264+
265+
function matchCompilerDiagnostic(
266+
badReference: NodePath<t.Node>,
267+
transformErrors: Array<{fn: NodePath<t.Node>; error: CompilerError}>,
268+
): string | null {
269+
for (const {fn, error} of transformErrors) {
270+
if (fn.isAncestor(badReference)) {
271+
return error.toString();
272+
}
273+
}
274+
return null;
275+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,7 @@ export class Environment {
11211121
moduleName.toLowerCase() === 'react-dom'
11221122
);
11231123
}
1124+
static knownReactModules: ReadonlyArray<string> = ['react', 'react-dom'];
11241125

11251126
getFallthroughPropertyType(
11261127
receiver: Type,

0 commit comments

Comments
 (0)