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..c2c7d8d640846 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,7 +383,8 @@ export function compileProgram( ); } - if (lintError != null) { + let compiledFn: CodegenFunction; + try { /** * 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 @@ -396,16 +395,15 @@ export function compileProgram( 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); } + return null; } - } - let compiledFn: CodegenFunction; - try { compiledFn = compileFn( fn, environment, @@ -436,7 +434,6 @@ export function compileProgram( return null; } } - hasCriticalError ||= isCriticalError(err); handleError(err, pass, fn.node.loc ?? null); return null; } @@ -470,7 +467,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/Entrypoint/Suppression.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts index 71341989a7592..4d0369f5210ca 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts @@ -14,6 +14,7 @@ import { ErrorSeverity, } from '../CompilerError'; import {assertExhaustive} from '../Utils/utils'; +import {GeneratedSource} from '../HIR'; /** * Captures the start and end range of a pair of eslint-disable ... eslint-enable comments. In the @@ -148,10 +149,11 @@ export function findProgramSuppressions( export function suppressionsToCompilerError( suppressionRanges: Array, -): CompilerError | null { - if (suppressionRanges.length === 0) { - return null; - } +): CompilerError { + CompilerError.invariant(suppressionRanges.length !== 0, { + reason: `Expected at least suppression comment source range`, + loc: GeneratedSource, + }); const error = new CompilerError(); for (const suppressionRange of suppressionRanges) { if ( 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}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.expect.md new file mode 100644 index 0000000000000..f0c6bce34222e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @panicThreshold(none) + +// unclosed disable rule should affect all components +/* eslint-disable react-hooks/rules-of-hooks */ + +function ValidComponent1(props) { + return
Hello World!
; +} + +function ValidComponent2(props) { + return
{props.greeting}
; +} + +``` + +## Code + +```javascript +// @panicThreshold(none) + +// unclosed disable rule should affect all components +/* eslint-disable react-hooks/rules-of-hooks */ + +function ValidComponent1(props) { + return
Hello World!
; +} + +function ValidComponent2(props) { + return
{props.greeting}
; +} + +``` + +### 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/unclosed-eslint-suppression-skips-all-components.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.js new file mode 100644 index 0000000000000..121f10041821f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unclosed-eslint-suppression-skips-all-components.js @@ -0,0 +1,12 @@ +// @panicThreshold(none) + +// unclosed disable rule should affect all components +/* eslint-disable react-hooks/rules-of-hooks */ + +function ValidComponent1(props) { + return
Hello World!
; +} + +function ValidComponent2(props) { + return
{props.greeting}
; +}