@samitouri / QOS-React / commits / 071dd00366

[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: 6e04a5bb2e2045303cbddad6d6d4bd38d5f7990b Pull Request resolved: https://github.com/facebook/react/pull/30844

Joe Savona committed Aug 29, 2024 at 22:40 UTC 071dd00366b3accb649e3f5978454e993e0b11aa
7 files changed +125 -14
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
+5 -8
@@ -310,8 +310,6 @@ export function compileProgram(
310 pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS,
311 pass.opts.flowSuppressions,
312 );
313 - const lintError = suppressionsToCompilerError(suppressions);
314 - let hasCriticalError = lintError != null;
313 const queue: Array<{
314 kind: 'original' | 'outlined';
315 fn: BabelFn;
@@ -385,7 +383,8 @@ export function compileProgram(
383 );
384 }
385
388 - if (lintError != null) {
386 + let compiledFn: CodegenFunction;
387 + try {
388 /**
389 * Note that Babel does not attach comment nodes to nodes; they are dangling off of the
390 * Program node itself. We need to figure out whether an eslint suppression range
@@ -396,16 +395,15 @@ export function compileProgram(
395 fn,
396 );
397 if (suppressionsInFunction.length > 0) {
398 + const lintError = suppressionsToCompilerError(suppressionsInFunction);
399 if (optOutDirectives.length > 0) {
400 logError(lintError, pass, fn.node.loc ?? null);
401 } else {
402 handleError(lintError, pass, fn.node.loc ?? null);
403 }
404 + return null;
405 }
405 - }
406
407 - let compiledFn: CodegenFunction;
408 - try {
407 compiledFn = compileFn(
408 fn,
409 environment,
@@ -436,7 +434,6 @@ export function compileProgram(
434 return null;
435 }
436 }
439 - hasCriticalError ||= isCriticalError(err);
437 handleError(err, pass, fn.node.loc ?? null);
438 return null;
439 }
@@ -470,7 +467,7 @@ export function compileProgram(
467 return null;
468 }
469
473 - if (!pass.opts.noEmit && !hasCriticalError) {
470 + if (!pass.opts.noEmit) {
471 return compiledFn;
472 }
473 return null;
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts
+6 -4
@@ -14,6 +14,7 @@ import {
14 ErrorSeverity,
15 } from '../CompilerError';
16 import {assertExhaustive} from '../Utils/utils';
17 +import {GeneratedSource} from '../HIR';
18
19 /**
20 * Captures the start and end range of a pair of eslint-disable ... eslint-enable comments. In the
@@ -148,10 +149,11 @@ export function findProgramSuppressions(
149
150 export function suppressionsToCompilerError(
151 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 + });
157 const error = new CompilerError();
158 for (const suppressionRange of suppressionRanges) {
159 if (
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md
-2
@@ -39,8 +39,6 @@ function CrimesAgainstReact() {
39 1 | // Note: Everything below this is sketchy
40 > 2 | /* eslint-disable react-hooks/rules-of-hooks */
41 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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)
42 3 | function lowercasecomponent() {
43 4 | 'use forget';
44 5 | const x = [];
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.expect.md new
+50
@@ -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
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.js new
+13
@@ -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 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.expect.md new
+39
@@ -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
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.js new
+12
@@ -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 +}