From f0c8f952f53681f954266d83e5193f1ed456c95b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 28 Aug 2024 17:16:51 -0700 Subject: [PATCH] [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: a82cca9855adab77493e0a140d2a142788375d0d Pull Request resolved: https://github.com/facebook/react/pull/30844 --- .../src/Entrypoint/Program.ts | 36 ++++++------- ...alid-unclosed-eslint-suppression.expect.md | 2 - ...iple-components-first-is-invalid.expect.md | 50 +++++++++++++++++++ .../multiple-components-first-is-invalid.js | 13 +++++ 4 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index 979e9f88d1b57..fc0d6b343cf46 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -310,8 +310,6 @@ export function compileProgram( pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS, pass.opts.flowSuppressions, ); - const lintError = suppressionsToCompilerError(suppressions); - let hasCriticalError = lintError != null; const queue: Array<{ kind: 'original' | 'outlined'; fn: BabelFn; @@ -385,22 +383,21 @@ export function compileProgram( ); } - if (lintError != null) { - /** - * Note that Babel does not attach comment nodes to nodes; they are dangling off of the - * Program node itself. We need to figure out whether an eslint suppression range - * applies to this function first. - */ - const suppressionsInFunction = filterSuppressionsThatAffectFunction( - suppressions, - fn, - ); - if (suppressionsInFunction.length > 0) { - if (optOutDirectives.length > 0) { - logError(lintError, pass, fn.node.loc ?? null); - } else { - handleError(lintError, pass, fn.node.loc ?? null); - } + /** + * Note that Babel does not attach comment nodes to nodes; they are dangling off of the + * Program node itself. We need to figure out whether an eslint suppression range + * applies to this function first. + */ + const suppressionsInFunction = filterSuppressionsThatAffectFunction( + suppressions, + fn, + ); + if (suppressionsInFunction.length > 0) { + const lintError = suppressionsToCompilerError(suppressionsInFunction); + if (optOutDirectives.length > 0) { + logError(lintError, pass, fn.node.loc ?? null); + } else { + handleError(lintError, pass, fn.node.loc ?? null); } } @@ -436,7 +433,6 @@ export function compileProgram( return null; } } - hasCriticalError ||= isCriticalError(err); handleError(err, pass, fn.node.loc ?? null); return null; } @@ -470,7 +466,7 @@ export function compileProgram( return null; } - if (!pass.opts.noEmit && !hasCriticalError) { + if (!pass.opts.noEmit) { return compiledFn; } return null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md index b81dadf409301..9f8e15592df6f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unclosed-eslint-suppression.expect.md @@ -39,8 +39,6 @@ function CrimesAgainstReact() { 1 | // Note: Everything below this is sketchy > 2 | /* eslint-disable react-hooks/rules-of-hooks */ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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) - -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) 3 | function lowercasecomponent() { 4 | 'use forget'; 5 | const x = []; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.expect.md new file mode 100644 index 0000000000000..3224997b40343 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.expect.md @@ -0,0 +1,50 @@ + +## Input + +```javascript +// @panicThreshold(none) +import {useHook} from 'shared-runtime'; + +function InvalidComponent(props) { + if (props.cond) { + useHook(); + } + return
Hello World!
; +} + +function ValidComponent(props) { + return
{props.greeting}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @panicThreshold(none) +import { useHook } from "shared-runtime"; + +function InvalidComponent(props) { + if (props.cond) { + useHook(); + } + return
Hello World!
; +} + +function ValidComponent(props) { + const $ = _c(2); + let t0; + if ($[0] !== props.greeting) { + t0 =
{props.greeting}
; + $[0] = props.greeting; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.js new file mode 100644 index 0000000000000..6a3d52c86406a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-components-first-is-invalid.js @@ -0,0 +1,13 @@ +// @panicThreshold(none) +import {useHook} from 'shared-runtime'; + +function InvalidComponent(props) { + if (props.cond) { + useHook(); + } + return
Hello World!
; +} + +function ValidComponent(props) { + return
{props.greeting}
; +}