Skip to content

Commit 04bf1d3

Browse files
committed
[compiler] Errors in earlier functions dont stop subsequent compilation
Errors in an earlier component/hook shouldn't stop later components from compiling. ghstack-source-id: 8218c3a Pull Request resolved: #30844
1 parent c741d2d commit 04bf1d3

7 files changed

+125
-14
lines changed

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,6 @@ export function compileProgram(
310310
pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS,
311311
pass.opts.flowSuppressions,
312312
);
313-
const lintError = suppressionsToCompilerError(suppressions);
314-
let hasCriticalError = lintError != null;
315313
const queue: Array<{
316314
kind: 'original' | 'outlined';
317315
fn: BabelFn;
@@ -385,7 +383,8 @@ export function compileProgram(
385383
);
386384
}
387385

388-
if (lintError != null) {
386+
let compiledFn: CodegenFunction;
387+
try {
389388
/**
390389
* Note that Babel does not attach comment nodes to nodes; they are dangling off of the
391390
* Program node itself. We need to figure out whether an eslint suppression range
@@ -396,16 +395,15 @@ export function compileProgram(
396395
fn,
397396
);
398397
if (suppressionsInFunction.length > 0) {
398+
const lintError = suppressionsToCompilerError(suppressionsInFunction);
399399
if (optOutDirectives.length > 0) {
400400
logError(lintError, pass, fn.node.loc ?? null);
401401
} else {
402402
handleError(lintError, pass, fn.node.loc ?? null);
403403
}
404+
return null;
404405
}
405-
}
406406

407-
let compiledFn: CodegenFunction;
408-
try {
409407
compiledFn = compileFn(
410408
fn,
411409
environment,
@@ -436,7 +434,6 @@ export function compileProgram(
436434
return null;
437435
}
438436
}
439-
hasCriticalError ||= isCriticalError(err);
440437
handleError(err, pass, fn.node.loc ?? null);
441438
return null;
442439
}
@@ -470,7 +467,7 @@ export function compileProgram(
470467
return null;
471468
}
472469

473-
if (!pass.opts.noEmit && !hasCriticalError) {
470+
if (!pass.opts.noEmit) {
474471
return compiledFn;
475472
}
476473
return null;

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
ErrorSeverity,
1515
} from '../CompilerError';
1616
import {assertExhaustive} from '../Utils/utils';
17+
import {GeneratedSource} from '../HIR';
1718

1819
/**
1920
* Captures the start and end range of a pair of eslint-disable ... eslint-enable comments. In the
@@ -148,10 +149,11 @@ export function findProgramSuppressions(
148149

149150
export function suppressionsToCompilerError(
150151
suppressionRanges: Array<SuppressionRange>,
151-
): CompilerError | null {
152-
if (suppressionRanges.length === 0) {
153-
return null;
154-
}
152+
): CompilerError {
153+
CompilerError.invariant(suppressionRanges.length !== 0, {
154+
reason: `Expected at least suppression comment source range`,
155+
loc: GeneratedSource,
156+
});
155157
const error = new CompilerError();
156158
for (const suppressionRange of suppressionRanges) {
157159
if (

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ function CrimesAgainstReact() {
3939
1 | // Note: Everything below this is sketchy
4040
> 2 | /* eslint-disable react-hooks/rules-of-hooks */
4141
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled. React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. eslint-disable react-hooks/rules-of-hooks (2:2)
42-
43-
InvalidReact: React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled. React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. eslint-disable-next-line react-hooks/rules-of-hooks (25:25)
4442
3 | function lowercasecomponent() {
4543
4 | 'use forget';
4644
5 | const x = [];
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @panicThreshold(none)
6+
import {useHook} from 'shared-runtime';
7+
8+
function InvalidComponent(props) {
9+
if (props.cond) {
10+
useHook();
11+
}
12+
return <div>Hello World!</div>;
13+
}
14+
15+
function ValidComponent(props) {
16+
return <div>{props.greeting}</div>;
17+
}
18+
19+
```
20+
21+
## Code
22+
23+
```javascript
24+
import { c as _c } from "react/compiler-runtime"; // @panicThreshold(none)
25+
import { useHook } from "shared-runtime";
26+
27+
function InvalidComponent(props) {
28+
if (props.cond) {
29+
useHook();
30+
}
31+
return <div>Hello World!</div>;
32+
}
33+
34+
function ValidComponent(props) {
35+
const $ = _c(2);
36+
let t0;
37+
if ($[0] !== props.greeting) {
38+
t0 = <div>{props.greeting}</div>;
39+
$[0] = props.greeting;
40+
$[1] = t0;
41+
} else {
42+
t0 = $[1];
43+
}
44+
return t0;
45+
}
46+
47+
```
48+
49+
### Eval output
50+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// @panicThreshold(none)
2+
import {useHook} from 'shared-runtime';
3+
4+
function InvalidComponent(props) {
5+
if (props.cond) {
6+
useHook();
7+
}
8+
return <div>Hello World!</div>;
9+
}
10+
11+
function ValidComponent(props) {
12+
return <div>{props.greeting}</div>;
13+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @panicThreshold(none)
6+
7+
// unclosed disable rule should affect all components
8+
/* eslint-disable react-hooks/rules-of-hooks */
9+
10+
function ValidComponent1(props) {
11+
return <div>Hello World!</div>;
12+
}
13+
14+
function ValidComponent2(props) {
15+
return <div>{props.greeting}</div>;
16+
}
17+
18+
```
19+
20+
## Code
21+
22+
```javascript
23+
// @panicThreshold(none)
24+
25+
// unclosed disable rule should affect all components
26+
/* eslint-disable react-hooks/rules-of-hooks */
27+
28+
function ValidComponent1(props) {
29+
return <div>Hello World!</div>;
30+
}
31+
32+
function ValidComponent2(props) {
33+
return <div>{props.greeting}</div>;
34+
}
35+
36+
```
37+
38+
### Eval output
39+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// @panicThreshold(none)
2+
3+
// unclosed disable rule should affect all components
4+
/* eslint-disable react-hooks/rules-of-hooks */
5+
6+
function ValidComponent1(props) {
7+
return <div>Hello World!</div>;
8+
}
9+
10+
function ValidComponent2(props) {
11+
return <div>{props.greeting}</div>;
12+
}

0 commit comments

Comments
 (0)