From 2e114b0b2ef258ef56e5794746637ddacbd29a68 Mon Sep 17 00:00:00 2001 From: Mike Vitousek Date: Thu, 1 Aug 2024 14:05:37 -0700 Subject: [PATCH] Update on "[compiler] Bail out and log calls that likely have side effects" [ghstack-poisoned] --- .../ReactiveScopes/PruneTemporaryLValues.ts | 3 +++ .../ReactiveScopes/ValidateNoMutationCalls.ts | 12 +++++++---- .../block-scoping-switch-variable-scoping.js | 21 +++++++------------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts index 535a4e5e91..330cb3455a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts @@ -36,6 +36,9 @@ class Visitor extends ReactiveFunctionVisitor { instruction: ReactiveInstruction, state: LValues, ): void { + if (instruction.value.kind === 'FunctionExpression') { + this.visitHirFunction(instruction.value.loweredFunc.func, state) + } this.traverseInstruction(instruction, state); if ( instruction.lvalue !== null && diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ValidateNoMutationCalls.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ValidateNoMutationCalls.ts index 194f4af2ea..ecade61f14 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ValidateNoMutationCalls.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ValidateNoMutationCalls.ts @@ -18,7 +18,7 @@ import { getHookKind, isSetStateType, } from '../HIR/HIR'; -import { eachInstructionValueLValue } from '../HIR/visitors'; +import { eachInstructionLValue, eachInstructionValueLValue } from '../HIR/visitors'; import { ReactiveFunctionVisitor, eachReactiveValueOperand, @@ -115,6 +115,8 @@ class Visitor extends ReactiveFunctionVisitor { break; } } + } else { + super.visitInstruction(instr, state); } let hookKind = null; @@ -131,11 +133,11 @@ class Visitor extends ReactiveFunctionVisitor { hookKind = getHookKind(this.#env, callee.identifier); } - if (hookKind !== 'useEffect' && hookKind !== 'useLayoutEffect' && hookKind !== 'useInsertionEffect' && instr.value.kind !== "JsxExpression") { + if (instr.value.kind !== 'JsxExpression') { for (const operand of eachReactiveValueOperand(instr.value)) { const errors = this.#functions.get(operand.identifier.id); if (errors != null) { - for (const lval of eachInstructionValueLValue(instr.value)) { + for (const lval of eachInstructionLValue(instr)) { const existing = this.#functions.get(lval.identifier.id) ?? new CompilerError(); errors.details.forEach(detail => existing.pushErrorDetail(detail)); this.#functions.set(lval.identifier.id, existing); @@ -150,7 +152,9 @@ class Visitor extends ReactiveFunctionVisitor { isSetStateType(callee.identifier); const name = this.getName(callee.identifier) ?? "(unknown)"; - this.#functions.get(callee.identifier.id)?.details?.forEach(detail => state.pushErrorDetail(detail)); + if (hookKind !== 'useEffect' && hookKind !== 'useLayoutEffect' && hookKind !== 'useInsertionEffect') { + [...eachReactiveValueOperand(instr.value)].forEach(operand => this.#functions.get(operand.identifier.id)?.details?.forEach(detail => state.pushErrorDetail(detail))); + } if (instr.lvalue === null && !isException && !allowedNames.has(name)) { let allReads = true; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js index 6b005c0e04..4dc4f84b8c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.js @@ -1,19 +1,12 @@ -import {useMemo} from 'react'; +import {useEffect, useMemo} from 'react'; function Component(props) { - const outerHandlers = useMemo(() => { - let handlers = {value: props.value}; - switch (props.test) { - case true: { - console.log(handlers.value); - break; - } - default: { - } - } - return handlers; - }); - return outerHandlers; + function foo() { + mutate(); + } + const h = [foo]; + useEffect(() => { a(h) }); + return lengh(); } export const FIXTURE_ENTRYPOINT = {