diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 590aba2fdc0c4..8307e8817b4f9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -105,6 +105,7 @@ import {outlineFunctions} from '../Optimization/OutlineFunctions'; import {propagatePhiTypes} from '../TypeInference/PropagatePhiTypes'; import {lowerContextAccess} from '../Optimization/LowerContextAccess'; import {validateNoSetStateInPassiveEffects} from '../Validation/ValidateNoSetStateInPassiveEffects'; +import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryStatement'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -249,6 +250,10 @@ function* runWithEnvironment( validateNoSetStateInPassiveEffects(hir); } + if (env.config.validateNoJSXInTryStatements) { + validateNoJSXInTryStatement(hir); + } + inferReactivePlaces(hir); yield log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index ca03b8a7b1e39..a5614ac244a14 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -237,6 +237,12 @@ const EnvironmentConfigSchema = z.object({ */ validateNoSetStateInPassiveEffects: z.boolean().default(false), + /** + * Validates against creating JSX within a try block and recommends using an error boundary + * instead. + */ + validateNoJSXInTryStatements: z.boolean().default(false), + /** * Validates that the dependencies of all effect hooks are memoized. This helps ensure * that Forget does not introduce infinite renders caused by a dependency changing, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoJSXInTryStatement.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoJSXInTryStatement.ts new file mode 100644 index 0000000000000..b92a89d764301 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoJSXInTryStatement.ts @@ -0,0 +1,52 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {CompilerError, ErrorSeverity} from '..'; +import {BlockId, HIRFunction} from '../HIR'; +import {retainWhere} from '../Utils/utils'; + +/** + * Developers may not be aware of error boundaries and lazy evaluation of JSX, leading them + * to use patterns such as `let el; try { el = } catch { ... }` to attempt to + * catch rendering errors. Such code will fail to catch errors in rendering, but developers + * may not realize this right away. + * + * This validation pass validates against this pattern: specifically, it errors for JSX + * created within a try block. JSX is allowed within a catch statement, unless that catch + * is itself nested inside an outer try. + */ +export function validateNoJSXInTryStatement(fn: HIRFunction): void { + const activeTryBlocks: Array = []; + const errors = new CompilerError(); + for (const [, block] of fn.body.blocks) { + retainWhere(activeTryBlocks, id => id !== block.id); + + if (activeTryBlocks.length !== 0) { + for (const instr of block.instructions) { + const {value} = instr; + switch (value.kind) { + case 'JsxExpression': + case 'JsxFragment': { + errors.push({ + severity: ErrorSeverity.InvalidReact, + reason: `Unexpected JSX element within a try statement. To catch errors in rendering a given component, wrap that component in an error boundary. (https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary)`, + loc: value.loc, + }); + break; + } + } + } + } + + if (block.terminal.kind === 'try') { + activeTryBlocks.push(block.terminal.handler); + } + } + if (errors.hasErrors()) { + throw errors; + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-catch-in-outer-try-with-catch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-catch-in-outer-try-with-catch.expect.md new file mode 100644 index 0000000000000..40cebff89a75e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-catch-in-outer-try-with-catch.expect.md @@ -0,0 +1,38 @@ + +## Input + +```javascript +// @validateNoJSXInTryStatements +import {identity} from 'shared-runtime'; + +function Component(props) { + let el; + try { + let value; + try { + value = identity(props.foo); + } catch { + el =
; + } + } catch { + return null; + } + return el; +} + +``` + + +## Error + +``` + 9 | value = identity(props.foo); + 10 | } catch { +> 11 | el =
; + | ^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Unexpected JSX element within a try statement. To catch errors in rendering a given component, wrap that component in an error boundary. (https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary) (11:11) + 12 | } + 13 | } catch { + 14 | return null; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-catch-in-outer-try-with-catch.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-catch-in-outer-try-with-catch.js new file mode 100644 index 0000000000000..0935a1a63cd83 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-catch-in-outer-try-with-catch.js @@ -0,0 +1,17 @@ +// @validateNoJSXInTryStatements +import {identity} from 'shared-runtime'; + +function Component(props) { + let el; + try { + let value; + try { + value = identity(props.foo); + } catch { + el =
; + } + } catch { + return null; + } + return el; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-try-with-catch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-try-with-catch.expect.md new file mode 100644 index 0000000000000..ee1f5335ef624 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-try-with-catch.expect.md @@ -0,0 +1,31 @@ + +## Input + +```javascript +// @validateNoJSXInTryStatements +function Component(props) { + let el; + try { + el =
; + } catch { + return null; + } + return el; +} + +``` + + +## Error + +``` + 3 | let el; + 4 | try { +> 5 | el =
; + | ^^^^^^^ InvalidReact: Unexpected JSX element within a try statement. To catch errors in rendering a given component, wrap that component in an error boundary. (https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary) (5:5) + 6 | } catch { + 7 | return null; + 8 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-try-with-catch.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-try-with-catch.js new file mode 100644 index 0000000000000..3e7747c875b3c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-jsx-in-try-with-catch.js @@ -0,0 +1,10 @@ +// @validateNoJSXInTryStatements +function Component(props) { + let el; + try { + el =
; + } catch { + return null; + } + return el; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.expect.md new file mode 100644 index 0000000000000..a7ea7b7739c6a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.expect.md @@ -0,0 +1,56 @@ + +## Input + +```javascript +// @validateNoJSXInTryStatements +import {identity} from 'shared-runtime'; + +function Component(props) { + let el; + try { + let value; + try { + value = identity(props.foo); + } catch { + el =
; + } + } finally { + console.log(el); + } + return el; +} + +``` + + +## Error + +``` + 4 | function Component(props) { + 5 | let el; +> 6 | try { + | ^^^^^ +> 7 | let value; + | ^^^^^^^^^^^^^^ +> 8 | try { + | ^^^^^^^^^^^^^^ +> 9 | value = identity(props.foo); + | ^^^^^^^^^^^^^^ +> 10 | } catch { + | ^^^^^^^^^^^^^^ +> 11 | el =
; + | ^^^^^^^^^^^^^^ +> 12 | } + | ^^^^^^^^^^^^^^ +> 13 | } finally { + | ^^^^^^^^^^^^^^ +> 14 | console.log(el); + | ^^^^^^^^^^^^^^ +> 15 | } + | ^^^^ Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (6:15) + 16 | return el; + 17 | } + 18 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.js new file mode 100644 index 0000000000000..9db091a2fb7ed --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-catch-in-outer-try-with-finally.js @@ -0,0 +1,17 @@ +// @validateNoJSXInTryStatements +import {identity} from 'shared-runtime'; + +function Component(props) { + let el; + try { + let value; + try { + value = identity(props.foo); + } catch { + el =
; + } + } finally { + console.log(el); + } + return el; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.expect.md new file mode 100644 index 0000000000000..a6a85d4519bcb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @validateNoJSXInTryStatements +function Component(props) { + let el; + try { + el =
; + } finally { + console.log(el); + } + return el; +} + +``` + + +## Error + +``` + 2 | function Component(props) { + 3 | let el; +> 4 | try { + | ^^^^^ +> 5 | el =
; + | ^^^^^^^^^^^^^^^^^ +> 6 | } finally { + | ^^^^^^^^^^^^^^^^^ +> 7 | console.log(el); + | ^^^^^^^^^^^^^^^^^ +> 8 | } + | ^^^^ Todo: (BuildHIR::lowerStatement) Handle TryStatement without a catch clause (4:8) + 9 | return el; + 10 | } + 11 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.js new file mode 100644 index 0000000000000..f0a17391c0eef --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-invalid-jsx-in-try-with-finally.js @@ -0,0 +1,10 @@ +// @validateNoJSXInTryStatements +function Component(props) { + let el; + try { + el =
; + } finally { + console.log(el); + } + return el; +}