From 469ae3269265f809bddcd9c1fb1059da5bba58ac Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Oct 2024 23:07:48 -0400 Subject: [PATCH 01/10] Add env config to infer effect deps --- .../babel-plugin-react-compiler/src/HIR/Environment.ts | 7 ++++++- compiler/packages/snap/src/compiler.ts | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) 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 855bc039abf37..f1c6b0b37ac96 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -232,7 +232,12 @@ const EnvironmentConfigSchema = z.object({ enableUseTypeAnnotations: z.boolean().default(false), enableFunctionDependencyRewrite: z.boolean().default(true), - + + /** + * Enables inference of effect dependencies. Still experimental. + */ + EXPERIMENTAL_inferEffectDependencies: z.boolean().default(false), + /** * Enables inlining ReactElement object literals in place of JSX * An alternative to the standard JSX transform which replaces JSX with React's jsxProd() runtime diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 95af40d62a880..9a3976bf5f74e 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -173,7 +173,12 @@ function makePluginOptions( .map(s => s.trim()) .filter(s => s.length > 0); } - + + let inferEffectDependencies = false; + if (firstLine.includes('@inferEffectDependencies')) { + inferEffectDependencies = true; + } + let logs: Array<{filename: string | null; event: LoggerEvent}> = []; let logger: Logger | null = null; if (firstLine.includes('@logger')) { @@ -197,6 +202,7 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, validateBlocklistedImports, + EXPERIMENTAL_inferEffectDependencies: inferEffectDependencies, }, compilationMode, logger, From 3bd6f87106f22d5b28174701da8e28e44e4d67cd Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 29 Oct 2024 23:09:00 -0400 Subject: [PATCH 02/10] First cut of inferring effect deps --- .../src/Entrypoint/Pipeline.ts | 5 ++ .../src/Inference/InferEffectDependencies.ts | 60 +++++++++++++ .../src/Inference/index.ts | 1 + .../infer-effect-dependencies.expect.md | 86 +++++++++++++++++++ .../compiler/infer-effect-dependencies.js | 24 ++++++ 5 files changed, 176 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js 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 1127e91029328..690e54607be0e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -36,6 +36,7 @@ import { inferReactivePlaces, inferReferenceEffects, inlineImmediatelyInvokedFunctionExpressions, + inferEffectDependencies, } from '../Inference'; import { constantPropagation, @@ -353,6 +354,10 @@ function* runWithEnvironment( name: 'PropagateScopeDependenciesHIR', value: hir, }); + + if (env.config.EXPERIMENTAL_inferEffectDependencies) { + inferEffectDependencies(env, hir); + } if (env.config.inlineJsxTransform) { inlineJsxTransform(hir, env.config.inlineJsxTransform); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts new file mode 100644 index 0000000000000..45af5bb5ac5b7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -0,0 +1,60 @@ +import { ArrayExpression, Effect, Environment, FunctionExpression, GeneratedSource, HIRFunction, IdentifierId, Instruction, isUseEffectHookType, makeInstructionId } from "../HIR"; +import { createTemporaryPlace } from "../HIR/HIRBuilder"; + +export function inferEffectDependencies( + env: Environment, + fn: HIRFunction, + ): void { + const fnExpressions = new Map(); + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + const {value, lvalue} = instr; + if ( + value.kind === 'FunctionExpression' + ) { + fnExpressions.set(lvalue.identifier.id, value) + } + } + } + + for (const [, block] of fn.body.blocks) { + let newInstructions = [...block.instructions]; + let addedInstrs = 0; + for (const [idx, instr] of block.instructions.entries()) { + const {value} = instr; + + /* + * This check is not final. Right now we only look for useEffects without a dependency array. + * This is likely not how we will ship this feature, but it is good enough for us to make progress + * on the implementation and test it. + */ + if ( + value.kind === 'CallExpression' && + isUseEffectHookType(value.callee.identifier) && + value.args[0].kind === 'Identifier' && + value.args.length === 1 + ) { + const fnExpr = fnExpressions.get(value.args[0].identifier.id); + if (fnExpr != null) { + const deps: ArrayExpression = { + kind: "ArrayExpression", + elements: [...fnExpr.loweredFunc.dependencies], + loc: GeneratedSource + }; + const depsPlace = createTemporaryPlace(env, GeneratedSource); + depsPlace.effect = Effect.Read; + const newInstruction: Instruction = { + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: depsPlace, + value: deps, + }; + newInstructions.splice(idx + addedInstrs, 0, newInstruction); + addedInstrs++; + value.args[1] = depsPlace; + } + } + } + block.instructions = newInstructions; + } + } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts index ee76a37bcb4b7..93b99fb385262 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts @@ -11,3 +11,4 @@ export {inferMutableRanges} from './InferMutableRanges'; export {inferReactivePlaces} from './InferReactivePlaces'; export {default as inferReferenceEffects} from './InferReferenceEffects'; export {inlineImmediatelyInvokedFunctionExpressions} from './InlineImmediatelyInvokedFunctionExpressions'; +export {inferEffectDependencies} from './InferEffectDependencies'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md new file mode 100644 index 0000000000000..30143e1cc0781 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +// @inferEffectDependencies +const nonreactive = 0; + +function Component({foo, bar}) { + useEffect(() => { + console.log(foo); + console.log(bar); + console.log(nonreactive); + }); + + useEffect(() => { + console.log(foo); + console.log(bar?.baz); + console.log(bar.qux); + }); + + function f() { + console.log(foo); + } + + useEffect(f); + +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +const nonreactive = 0; + +function Component(t0) { + const $ = _c(8); + const { foo, bar } = t0; + let t1; + if ($[0] !== foo || $[1] !== bar) { + t1 = () => { + console.log(foo); + console.log(bar); + console.log(nonreactive); + }; + $[0] = foo; + $[1] = bar; + $[2] = t1; + } else { + t1 = $[2]; + } + useEffect(t1, [foo, bar]); + let t2; + if ($[3] !== foo || $[4] !== bar) { + t2 = () => { + console.log(foo); + console.log(bar?.baz); + console.log(bar.qux); + }; + $[3] = foo; + $[4] = bar; + $[5] = t2; + } else { + t2 = $[5]; + } + useEffect(t2, [foo, bar, bar.qux]); + let t3; + if ($[6] !== foo) { + t3 = function f() { + console.log(foo); + }; + $[6] = foo; + $[7] = t3; + } else { + t3 = $[7]; + } + const f = t3; + + useEffect(f); +} + +``` + +### 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/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js new file mode 100644 index 0000000000000..86766641f031a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js @@ -0,0 +1,24 @@ +// @inferEffectDependencies +const nonreactive = 0; + +function Component({foo, bar}) { + useEffect(() => { + console.log(foo); + console.log(bar); + console.log(nonreactive); + }); + + useEffect(() => { + console.log(foo); + console.log(bar?.baz); + console.log(bar.qux); + }); + + function f() { + console.log(foo); + } + + // No inferred dep array, the argument is not a lambda + useEffect(f); + +} From 5d7cc78ded8b75dbe0dfb56f06d714fd91f1eb4e Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 1 Nov 2024 15:02:37 -0400 Subject: [PATCH 03/10] Address PR feedback + run prettier --- .../src/Entrypoint/Pipeline.ts | 2 +- .../src/HIR/Environment.ts | 14 +- .../src/Inference/InferEffectDependencies.ts | 125 ++++++++++-------- .../infer-effect-dependencies.expect.md | 47 ++++--- .../compiler/infer-effect-dependencies.js | 18 ++- compiler/packages/snap/src/compiler.ts | 4 +- 6 files changed, 125 insertions(+), 85 deletions(-) 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 690e54607be0e..d5ff27dd80309 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -355,7 +355,7 @@ function* runWithEnvironment( value: hir, }); - if (env.config.EXPERIMENTAL_inferEffectDependencies) { + if (env.config.inferEffectDependencies) { inferEffectDependencies(env, 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 f1c6b0b37ac96..1224c437dccc0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -232,12 +232,20 @@ const EnvironmentConfigSchema = z.object({ enableUseTypeAnnotations: z.boolean().default(false), enableFunctionDependencyRewrite: z.boolean().default(true), - + /** - * Enables inference of effect dependencies. Still experimental. + * Enables inference of optional dependency chains. Without this flag + * a property chain such as `props?.items?.foo` will infer as a dep on + * just `props`. With this flag enabled, we'll infer that full path as + * the dependency. */ - EXPERIMENTAL_inferEffectDependencies: z.boolean().default(false), + enableOptionalDependencies: z.boolean().default(true), + /** + * Enables inference and auto-insertion of effect dependencies. Still experimental. + */ + inferEffectDependencies: z.boolean().default(false), + /** * Enables inlining ReactElement object literals in place of JSX * An alternative to the standard JSX transform which replaces JSX with React's jsxProd() runtime diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 45af5bb5ac5b7..37b47ae3c404a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -1,60 +1,81 @@ -import { ArrayExpression, Effect, Environment, FunctionExpression, GeneratedSource, HIRFunction, IdentifierId, Instruction, isUseEffectHookType, makeInstructionId } from "../HIR"; -import { createTemporaryPlace } from "../HIR/HIRBuilder"; +import { + ArrayExpression, + Effect, + Environment, + FunctionExpression, + GeneratedSource, + HIRFunction, + IdentifierId, + Instruction, + isUseEffectHookType, + makeInstructionId, +} from '../HIR'; +import {createTemporaryPlace} from '../HIR/HIRBuilder'; +/** + * Infers reactive dependencies captured by useEffect lambdas and adds them as + * a second argument to the useEffect call if no dependency array is provided. + */ export function inferEffectDependencies( - env: Environment, - fn: HIRFunction, - ): void { - const fnExpressions = new Map(); - for (const [, block] of fn.body.blocks) { - for (const instr of block.instructions) { - const {value, lvalue} = instr; - if ( - value.kind === 'FunctionExpression' - ) { - fnExpressions.set(lvalue.identifier.id, value) - } + env: Environment, + fn: HIRFunction, +): void { + const fnExpressions = new Map(); + + for (const [, block] of fn.body.blocks) { + let rewriteInstrs = new Map(); + for (const instr of block.instructions) { + const {value, lvalue} = instr; + if (value.kind === 'FunctionExpression') { + fnExpressions.set(lvalue.identifier.id, value); + } else if ( + /* + * This check is not final. Right now we only look for useEffects without a dependency array. + * This is likely not how we will ship this feature, but it is good enough for us to make progress + * on the implementation and test it. + */ + value.kind === 'CallExpression' && + isUseEffectHookType(value.callee.identifier) && + value.args.length === 1 && + value.args[0].kind === 'Identifier' + ) { + const fnExpr = fnExpressions.get(value.args[0].identifier.id); + if (fnExpr != null) { + const deps: ArrayExpression = { + kind: 'ArrayExpression', + elements: [ + ...fnExpr.loweredFunc.dependencies.filter( + place => place.reactive, + ), + ], + loc: GeneratedSource, + }; + + const depsPlace = createTemporaryPlace(env, GeneratedSource); + depsPlace.effect = Effect.Read; + value.args[1] = depsPlace; + + const newInstruction: Instruction = { + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: depsPlace, + value: deps, + }; + rewriteInstrs.set(instr.id, newInstruction); + } } } - - for (const [, block] of fn.body.blocks) { - let newInstructions = [...block.instructions]; - let addedInstrs = 0; - for (const [idx, instr] of block.instructions.entries()) { - const {value} = instr; - - /* - * This check is not final. Right now we only look for useEffects without a dependency array. - * This is likely not how we will ship this feature, but it is good enough for us to make progress - * on the implementation and test it. - */ - if ( - value.kind === 'CallExpression' && - isUseEffectHookType(value.callee.identifier) && - value.args[0].kind === 'Identifier' && - value.args.length === 1 - ) { - const fnExpr = fnExpressions.get(value.args[0].identifier.id); - if (fnExpr != null) { - const deps: ArrayExpression = { - kind: "ArrayExpression", - elements: [...fnExpr.loweredFunc.dependencies], - loc: GeneratedSource - }; - const depsPlace = createTemporaryPlace(env, GeneratedSource); - depsPlace.effect = Effect.Read; - const newInstruction: Instruction = { - id: makeInstructionId(0), - loc: GeneratedSource, - lvalue: depsPlace, - value: deps, - }; - newInstructions.splice(idx + addedInstrs, 0, newInstruction); - addedInstrs++; - value.args[1] = depsPlace; - } + if (rewriteInstrs.size > 0) { + const newInstrs = []; + for (const instr of block.instructions) { + const newInstr = rewriteInstrs.get(instr.id); + if (newInstr != null) { + newInstrs.push(newInstr, instr); + } else { + newInstrs.push(instr); } } - block.instructions = newInstructions; + block.instructions = newInstrs; } } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index 30143e1cc0781..da3284df31cf7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -3,27 +3,34 @@ ```javascript // @inferEffectDependencies -const nonreactive = 0; +const moduleNonReactive = 0; function Component({foo, bar}) { + const localNonreactive = 0; useEffect(() => { console.log(foo); console.log(bar); - console.log(nonreactive); + console.log(moduleNonReactive); + console.log(localNonreactive); + console.log(globalValue); }); + + // Optional chains and property accesses + // TODO: we may be able to save bytes by omitting property accesses if the + // object of the member expression is already included in the inferred deps useEffect(() => { - console.log(foo); console.log(bar?.baz); console.log(bar.qux); + }); - + function f() { console.log(foo); } - + + // No inferred dep array, the argument is not a lambda useEffect(f); - } ``` @@ -32,17 +39,19 @@ function Component({foo, bar}) { ```javascript import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies -const nonreactive = 0; +const moduleNonReactive = 0; function Component(t0) { - const $ = _c(8); + const $ = _c(7); const { foo, bar } = t0; let t1; if ($[0] !== foo || $[1] !== bar) { t1 = () => { console.log(foo); console.log(bar); - console.log(nonreactive); + console.log(moduleNonReactive); + console.log(0); + console.log(globalValue); }; $[0] = foo; $[1] = bar; @@ -52,28 +61,26 @@ function Component(t0) { } useEffect(t1, [foo, bar]); let t2; - if ($[3] !== foo || $[4] !== bar) { + if ($[3] !== bar) { t2 = () => { - console.log(foo); console.log(bar?.baz); console.log(bar.qux); }; - $[3] = foo; - $[4] = bar; - $[5] = t2; + $[3] = bar; + $[4] = t2; } else { - t2 = $[5]; + t2 = $[4]; } - useEffect(t2, [foo, bar, bar.qux]); + useEffect(t2, [bar, bar.qux]); let t3; - if ($[6] !== foo) { + if ($[5] !== foo) { t3 = function f() { console.log(foo); }; - $[6] = foo; - $[7] = t3; + $[5] = foo; + $[6] = t3; } else { - t3 = $[7]; + t3 = $[6]; } const f = t3; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js index 86766641f031a..889130ebf3339 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js @@ -1,24 +1,28 @@ // @inferEffectDependencies -const nonreactive = 0; +const moduleNonReactive = 0; function Component({foo, bar}) { + const localNonreactive = 0; useEffect(() => { console.log(foo); console.log(bar); - console.log(nonreactive); + console.log(moduleNonReactive); + console.log(localNonreactive); + console.log(globalValue); }); - + + // Optional chains and property accesses + // TODO: we may be able to save bytes by omitting property accesses if the + // object of the member expression is already included in the inferred deps useEffect(() => { - console.log(foo); console.log(bar?.baz); console.log(bar.qux); }); - + function f() { console.log(foo); } - + // No inferred dep array, the argument is not a lambda useEffect(f); - } diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 9a3976bf5f74e..11c205c728780 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -173,7 +173,7 @@ function makePluginOptions( .map(s => s.trim()) .filter(s => s.length > 0); } - + let inferEffectDependencies = false; if (firstLine.includes('@inferEffectDependencies')) { inferEffectDependencies = true; @@ -202,7 +202,7 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, validateBlocklistedImports, - EXPERIMENTAL_inferEffectDependencies: inferEffectDependencies, + inferEffectDependencies, }, compilationMode, logger, From 72b6f1ba56ad4feedc7afc3bd89f577957fe8fe6 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 1 Nov 2024 15:05:45 -0400 Subject: [PATCH 04/10] No need to spread a filtered array --- .../src/Inference/InferEffectDependencies.ts | 6 +----- .../fixtures/compiler/infer-effect-dependencies.expect.md | 2 -- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 37b47ae3c404a..367883254d1fb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -43,11 +43,7 @@ export function inferEffectDependencies( if (fnExpr != null) { const deps: ArrayExpression = { kind: 'ArrayExpression', - elements: [ - ...fnExpr.loweredFunc.dependencies.filter( - place => place.reactive, - ), - ], + elements: fnExpr.loweredFunc.dependencies.filter(place => place.reactive), loc: GeneratedSource, }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index da3284df31cf7..edb1b252de60f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -14,7 +14,6 @@ function Component({foo, bar}) { console.log(localNonreactive); console.log(globalValue); }); - // Optional chains and property accesses // TODO: we may be able to save bytes by omitting property accesses if the @@ -22,7 +21,6 @@ function Component({foo, bar}) { useEffect(() => { console.log(bar?.baz); console.log(bar.qux); - }); function f() { From 0481dddce801f254eb31e4972d25cf1037da0050 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 1 Nov 2024 15:25:16 -0400 Subject: [PATCH 05/10] prettier again lol --- .../src/Inference/InferEffectDependencies.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 367883254d1fb..55a252f53e25c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -29,11 +29,11 @@ export function inferEffectDependencies( if (value.kind === 'FunctionExpression') { fnExpressions.set(lvalue.identifier.id, value); } else if ( - /* - * This check is not final. Right now we only look for useEffects without a dependency array. - * This is likely not how we will ship this feature, but it is good enough for us to make progress - * on the implementation and test it. - */ + /* + * This check is not final. Right now we only look for useEffects without a dependency array. + * This is likely not how we will ship this feature, but it is good enough for us to make progress + * on the implementation and test it. + */ value.kind === 'CallExpression' && isUseEffectHookType(value.callee.identifier) && value.args.length === 1 && @@ -43,7 +43,9 @@ export function inferEffectDependencies( if (fnExpr != null) { const deps: ArrayExpression = { kind: 'ArrayExpression', - elements: fnExpr.loweredFunc.dependencies.filter(place => place.reactive), + elements: fnExpr.loweredFunc.dependencies.filter( + place => place.reactive, + ), loc: GeneratedSource, }; From cedc4b698bb51194d76c58b157044819f86a8b55 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 19 Nov 2024 11:45:26 -0500 Subject: [PATCH 06/10] update tests --- .../infer-effect-dependencies.expect.md | 75 +++++++++++++------ .../compiler/infer-effect-dependencies.js | 8 ++ 2 files changed, 61 insertions(+), 22 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index edb1b252de60f..10f193b247643 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -7,12 +7,20 @@ const moduleNonReactive = 0; function Component({foo, bar}) { const localNonreactive = 0; + const ref = useRef(0); + const localNonPrimitiveReactive = { + foo, + }; + const localNonPrimitiveNonreactive = {}; useEffect(() => { console.log(foo); console.log(bar); console.log(moduleNonReactive); console.log(localNonreactive); console.log(globalValue); + console.log(ref.current); + console.log(localNonPrimitiveReactive); + console.log(localNonPrimitiveNonreactive); }); // Optional chains and property accesses @@ -40,47 +48,70 @@ import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies const moduleNonReactive = 0; function Component(t0) { - const $ = _c(7); + const $ = _c(11); const { foo, bar } = t0; + + const ref = useRef(0); let t1; - if ($[0] !== foo || $[1] !== bar) { - t1 = () => { + if ($[0] !== foo) { + t1 = { foo }; + $[0] = foo; + $[1] = t1; + } else { + t1 = $[1]; + } + const localNonPrimitiveReactive = t1; + let t2; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = {}; + $[2] = t2; + } else { + t2 = $[2]; + } + const localNonPrimitiveNonreactive = t2; + let t3; + if ($[3] !== foo || $[4] !== bar || $[5] !== localNonPrimitiveReactive) { + t3 = () => { console.log(foo); console.log(bar); console.log(moduleNonReactive); console.log(0); console.log(globalValue); + console.log(ref.current); + console.log(localNonPrimitiveReactive); + console.log(localNonPrimitiveNonreactive); }; - $[0] = foo; - $[1] = bar; - $[2] = t1; + $[3] = foo; + $[4] = bar; + $[5] = localNonPrimitiveReactive; + $[6] = t3; } else { - t1 = $[2]; + t3 = $[6]; } - useEffect(t1, [foo, bar]); - let t2; - if ($[3] !== bar) { - t2 = () => { + useEffect(t3, [foo, bar, localNonPrimitiveReactive]); + let t4; + if ($[7] !== bar) { + t4 = () => { console.log(bar?.baz); console.log(bar.qux); }; - $[3] = bar; - $[4] = t2; + $[7] = bar; + $[8] = t4; } else { - t2 = $[4]; + t4 = $[8]; } - useEffect(t2, [bar, bar.qux]); - let t3; - if ($[5] !== foo) { - t3 = function f() { + useEffect(t4, [bar, bar.qux]); + let t5; + if ($[9] !== foo) { + t5 = function f() { console.log(foo); }; - $[5] = foo; - $[6] = t3; + $[9] = foo; + $[10] = t5; } else { - t3 = $[6]; + t5 = $[10]; } - const f = t3; + const f = t5; useEffect(f); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js index 889130ebf3339..6a70bc1298b0a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.js @@ -3,12 +3,20 @@ const moduleNonReactive = 0; function Component({foo, bar}) { const localNonreactive = 0; + const ref = useRef(0); + const localNonPrimitiveReactive = { + foo, + }; + const localNonPrimitiveNonreactive = {}; useEffect(() => { console.log(foo); console.log(bar); console.log(moduleNonReactive); console.log(localNonreactive); console.log(globalValue); + console.log(ref.current); + console.log(localNonPrimitiveReactive); + console.log(localNonPrimitiveNonreactive); }); // Optional chains and property accesses From d99e393f820b4b09d345f970be5843e446b6a402 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 19 Nov 2024 11:56:33 -0500 Subject: [PATCH 07/10] update tests --- .../infer-effect-dependencies.expect.md | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index 10f193b247643..68714d7e4e16f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -48,7 +48,7 @@ import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies const moduleNonReactive = 0; function Component(t0) { - const $ = _c(11); + const $ = _c(12); const { foo, bar } = t0; const ref = useRef(0); @@ -70,7 +70,7 @@ function Component(t0) { } const localNonPrimitiveNonreactive = t2; let t3; - if ($[3] !== foo || $[4] !== bar || $[5] !== localNonPrimitiveReactive) { + if ($[3] !== bar || $[4] !== foo || $[5] !== localNonPrimitiveReactive) { t3 = () => { console.log(foo); console.log(bar); @@ -81,8 +81,8 @@ function Component(t0) { console.log(localNonPrimitiveReactive); console.log(localNonPrimitiveNonreactive); }; - $[3] = foo; - $[4] = bar; + $[3] = bar; + $[4] = foo; $[5] = localNonPrimitiveReactive; $[6] = t3; } else { @@ -90,26 +90,27 @@ function Component(t0) { } useEffect(t3, [foo, bar, localNonPrimitiveReactive]); let t4; - if ($[7] !== bar) { + if ($[7] !== bar.baz || $[8] !== bar.qux) { t4 = () => { console.log(bar?.baz); console.log(bar.qux); }; - $[7] = bar; - $[8] = t4; + $[7] = bar.baz; + $[8] = bar.qux; + $[9] = t4; } else { - t4 = $[8]; + t4 = $[9]; } useEffect(t4, [bar, bar.qux]); let t5; - if ($[9] !== foo) { + if ($[10] !== foo) { t5 = function f() { console.log(foo); }; - $[9] = foo; - $[10] = t5; + $[10] = foo; + $[11] = t5; } else { - t5 = $[10]; + t5 = $[11]; } const f = t5; From d77ad3d0bb3e6acab2ebcc283b507d2e48116484 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 19 Nov 2024 12:01:17 -0500 Subject: [PATCH 08/10] prettier --- .../babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts | 2 +- .../packages/babel-plugin-react-compiler/src/HIR/Environment.ts | 2 +- compiler/packages/snap/src/compiler.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 d5ff27dd80309..bbd076e46a914 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -354,7 +354,7 @@ function* runWithEnvironment( name: 'PropagateScopeDependenciesHIR', value: hir, }); - + if (env.config.inferEffectDependencies) { inferEffectDependencies(env, 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 1224c437dccc0..432eaf96ff99b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -240,7 +240,7 @@ const EnvironmentConfigSchema = z.object({ * the dependency. */ enableOptionalDependencies: z.boolean().default(true), - + /** * Enables inference and auto-insertion of effect dependencies. Still experimental. */ diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 11c205c728780..6942ffb2adc7b 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -178,7 +178,7 @@ function makePluginOptions( if (firstLine.includes('@inferEffectDependencies')) { inferEffectDependencies = true; } - + let logs: Array<{filename: string | null; event: LoggerEvent}> = []; let logger: Logger | null = null; if (firstLine.includes('@logger')) { From 9b6d39f998b97ea5364ef8754227db4d0bf7593f Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 21 Nov 2024 16:30:34 -0500 Subject: [PATCH 09/10] refactor to use scope deps instead of FunctionExpression.dependencies --- .../src/Inference/InferEffectDependencies.ts | 188 ++++++++++++++++-- .../compiler/nonreactive-dep.expect.md | 80 ++++++++ .../fixtures/compiler/nonreactive-dep.js | 25 +++ .../compiler/nonreactive-ref.expect.md | 51 +++++ .../fixtures/compiler/nonreactive-ref.js | 14 ++ .../compiler/outlined-function.expect.md | 46 +++++ .../fixtures/compiler/outlined-function.js | 14 ++ .../compiler/pruned-nonreactive-obj.expect.md | 119 +++++++++++ .../compiler/pruned-nonreactive-obj.js | 48 +++++ .../reactive-memberexpr-merge.expect.md | 49 +++++ .../compiler/reactive-memberexpr-merge.js | 8 + .../compiler/reactive-memberexpr.expect.md | 49 +++++ .../fixtures/compiler/reactive-memberexpr.js | 8 + .../reactive-optional-chain.expect.md | 60 ++++++ .../compiler/reactive-optional-chain.js | 9 + .../compiler/reactive-variable.expect.md | 49 +++++ .../fixtures/compiler/reactive-variable.js | 8 + .../todo-import-namespace-useEffect.expect.md | 50 +++++ .../todo-import-namespace-useEffect.js | 10 + 19 files changed, 872 insertions(+), 13 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 55a252f53e25c..6407956ddd828 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -1,3 +1,4 @@ +import {CompilerError, SourceLocation} from '..'; import { ArrayExpression, Effect, @@ -9,8 +10,19 @@ import { Instruction, isUseEffectHookType, makeInstructionId, + TInstruction, + InstructionId, + ScopeId, + ReactiveScopeDependency, + Place, + ReactiveScopeDependencies, } from '../HIR'; -import {createTemporaryPlace} from '../HIR/HIRBuilder'; +import { + createTemporaryPlace, + fixScopeAndIdentifierRanges, + markInstructionIds, +} from '../HIR/HIRBuilder'; +import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; /** * Infers reactive dependencies captured by useEffect lambdas and adds them as @@ -20,14 +32,48 @@ export function inferEffectDependencies( env: Environment, fn: HIRFunction, ): void { - const fnExpressions = new Map(); + let hasRewrite = false; + const fnExpressions = new Map< + IdentifierId, + TInstruction + >(); + const scopeInfos = new Map< + ScopeId, + {pruned: boolean; deps: ReactiveScopeDependencies; hasSingleInstr: boolean} + >(); + + /** + * When inserting LoadLocals, we need to retain the reactivity of the base + * identifier, as later passes e.g. PruneNonReactiveDeps take the reactivity of + * a base identifier as the "maximal" reactivity of all its references. + * Concretely, + * reactive(Identifier i) = Union_{reference of i}(reactive(reference)) + */ + const reactiveIds = inferReactiveIdentifiers(fn); for (const [, block] of fn.body.blocks) { - let rewriteInstrs = new Map(); + if ( + block.terminal.kind === 'scope' || + block.terminal.kind === 'pruned-scope' + ) { + const scopeBlock = fn.body.blocks.get(block.terminal.block)!; + scopeInfos.set(block.terminal.scope.id, { + pruned: block.terminal.kind === 'pruned-scope', + deps: block.terminal.scope.dependencies, + hasSingleInstr: + scopeBlock.instructions.length === 1 && + scopeBlock.terminal.kind === 'goto' && + scopeBlock.terminal.block === block.terminal.fallthrough, + }); + } + const rewriteInstrs = new Map>(); for (const instr of block.instructions) { const {value, lvalue} = instr; if (value.kind === 'FunctionExpression') { - fnExpressions.set(lvalue.identifier.id, value); + fnExpressions.set( + lvalue.identifier.id, + instr as TInstruction, + ); } else if ( /* * This check is not final. Right now we only look for useEffects without a dependency array. @@ -41,34 +87,72 @@ export function inferEffectDependencies( ) { const fnExpr = fnExpressions.get(value.args[0].identifier.id); if (fnExpr != null) { + const scopeInfo = + fnExpr.lvalue.identifier.scope != null + ? scopeInfos.get(fnExpr.lvalue.identifier.scope.id) + : null; + CompilerError.invariant(scopeInfo != null, { + reason: 'Expected function expression scope to exist', + loc: value.loc, + }); + if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) { + // TODO: retry pipeline that ensures effect function expressions + // are placed into their own scope + CompilerError.throwTodo({ + reason: + '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction', + loc: fnExpr.loc, + }); + } + + /** + * Step 1: write new instructions to insert a dependency array + * + * Note that it's invalid to prune non-reactive deps in this pass, see + * the `infer-effect-deps/pruned-nonreactive-obj` fixture for an + * explanation. + */ + const effectDeps: Array = []; + const newInstructions: Array = []; + for (const dep of scopeInfo.deps) { + const {place, instructions} = writeDependencyToInstructions( + dep, + reactiveIds.has(dep.identifier.id), + fn.env, + fnExpr.loc, + ); + newInstructions.push(...instructions); + effectDeps.push(place); + } const deps: ArrayExpression = { kind: 'ArrayExpression', - elements: fnExpr.loweredFunc.dependencies.filter( - place => place.reactive, - ), + elements: effectDeps, loc: GeneratedSource, }; const depsPlace = createTemporaryPlace(env, GeneratedSource); depsPlace.effect = Effect.Read; - value.args[1] = depsPlace; - const newInstruction: Instruction = { + newInstructions.push({ id: makeInstructionId(0), loc: GeneratedSource, - lvalue: depsPlace, + lvalue: {...depsPlace, effect: Effect.Mutate}, value: deps, - }; - rewriteInstrs.set(instr.id, newInstruction); + }); + + // Step 2: insert the deps array as an argument of the useEffect + value.args[1] = {...depsPlace, effect: Effect.Freeze}; + rewriteInstrs.set(instr.id, newInstructions); } } } if (rewriteInstrs.size > 0) { + hasRewrite = true; const newInstrs = []; for (const instr of block.instructions) { const newInstr = rewriteInstrs.get(instr.id); if (newInstr != null) { - newInstrs.push(newInstr, instr); + newInstrs.push(...newInstr, instr); } else { newInstrs.push(instr); } @@ -76,4 +160,82 @@ export function inferEffectDependencies( block.instructions = newInstrs; } } + if (hasRewrite) { + // Renumber instructions and fix scope ranges + markInstructionIds(fn.body); + fixScopeAndIdentifierRanges(fn.body); + } +} + +function writeDependencyToInstructions( + dep: ReactiveScopeDependency, + reactive: boolean, + env: Environment, + loc: SourceLocation, +): {place: Place; instructions: Array} { + const instructions: Array = []; + let currValue = createTemporaryPlace(env, GeneratedSource); + currValue.reactive = reactive; + instructions.push({ + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...currValue, effect: Effect.Mutate}, + value: { + kind: 'LoadLocal', + place: { + kind: 'Identifier', + identifier: dep.identifier, + effect: Effect.Capture, + reactive, + loc: loc, + }, + loc: loc, + }, + }); + for (const path of dep.path) { + if (path.optional) { + // TODO: instead of truncating optional paths, reuse + // instructions from hoisted dependencies block + break; + } + const nextValue = createTemporaryPlace(env, GeneratedSource); + nextValue.reactive = reactive; + instructions.push({ + id: makeInstructionId(0), + loc: GeneratedSource, + lvalue: {...nextValue, effect: Effect.Mutate}, + value: { + kind: 'PropertyLoad', + object: {...currValue, effect: Effect.Capture}, + property: path.property, + loc: loc, + }, + }); + currValue = nextValue; + } + currValue.effect = Effect.Freeze; + return {place: currValue, instructions}; +} + +function inferReactiveIdentifiers(fn: HIRFunction): Set { + const reactiveIds: Set = new Set(); + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + // no need to traverse into nested functions as + // 1. Their effects are recorded in `LoweredFunction.dependencies` + // 2. we don't mark `reactive` in these anyways + for (const place of eachInstructionOperand(instr)) { + if (place.reactive) { + reactiveIds.add(place.identifier.id); + } + } + } + + for (const place of eachTerminalOperand(block.terminal)) { + if (place.reactive) { + reactiveIds.add(place.identifier.id); + } + } + } + return reactiveIds; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.expect.md new file mode 100644 index 0000000000000..1cfbe71e78843 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {makeObject_Primitives, print} from 'shared-runtime'; + +/** + * Note that `obj` is currently added to the effect dependency array, even + * though it's non-reactive due to memoization. + * + * This is a TODO in effect dependency inference. Note that we cannot simply + * filter out non-reactive effect dependencies, as some non-reactive (by data + * flow) values become reactive due to scope pruning. See the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for why this matters. + * + * Realizing that this `useEffect` should have an empty dependency array + * requires effect dependency inference to be structured similarly to memo + * dependency inference. + * Pass 1: add all potential dependencies regardless of dataflow reactivity + * Pass 2: (todo) prune non-reactive dependencies + * + * Note that instruction reordering should significantly reduce scope pruning + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + useEffect(() => print(obj)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { makeObject_Primitives, print } from "shared-runtime"; + +/** + * Note that `obj` is currently added to the effect dependency array, even + * though it's non-reactive due to memoization. + * + * This is a TODO in effect dependency inference. Note that we cannot simply + * filter out non-reactive effect dependencies, as some non-reactive (by data + * flow) values become reactive due to scope pruning. See the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for why this matters. + * + * Realizing that this `useEffect` should have an empty dependency array + * requires effect dependency inference to be structured similarly to memo + * dependency inference. + * Pass 1: add all potential dependencies regardless of dataflow reactivity + * Pass 2: (todo) prune non-reactive dependencies + * + * Note that instruction reordering should significantly reduce scope pruning + */ +function NonReactiveDepInEffect() { + const $ = _c(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeObject_Primitives(); + $[0] = t0; + } else { + t0 = $[0]; + } + const obj = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = () => print(obj); + $[1] = t1; + } else { + t1 = $[1]; + } + useEffect(t1, [obj]); +} + +``` + +### 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/nonreactive-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.js new file mode 100644 index 0000000000000..85d9699750293 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-dep.js @@ -0,0 +1,25 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {makeObject_Primitives, print} from 'shared-runtime'; + +/** + * Note that `obj` is currently added to the effect dependency array, even + * though it's non-reactive due to memoization. + * + * This is a TODO in effect dependency inference. Note that we cannot simply + * filter out non-reactive effect dependencies, as some non-reactive (by data + * flow) values become reactive due to scope pruning. See the + * `infer-effect-deps/pruned-nonreactive-obj` fixture for why this matters. + * + * Realizing that this `useEffect` should have an empty dependency array + * requires effect dependency inference to be structured similarly to memo + * dependency inference. + * Pass 1: add all potential dependencies regardless of dataflow reactivity + * Pass 2: (todo) prune non-reactive dependencies + * + * Note that instruction reordering should significantly reduce scope pruning + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + useEffect(() => print(obj)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.expect.md new file mode 100644 index 0000000000000..cf1ebd9252f6a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect, useRef} from 'react'; +import {print} from 'shared-runtime'; + +/** + * Special case of `infer-effect-deps/nonreactive-dep`. + * + * We know that local `useRef` return values are stable, regardless of + * inferred memoization. + */ +function NonReactiveRefInEffect() { + const ref = useRef('initial value'); + useEffect(() => print(ref.current)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect, useRef } from "react"; +import { print } from "shared-runtime"; + +/** + * Special case of `infer-effect-deps/nonreactive-dep`. + * + * We know that local `useRef` return values are stable, regardless of + * inferred memoization. + */ +function NonReactiveRefInEffect() { + const $ = _c(1); + const ref = useRef("initial value"); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => print(ref.current); + $[0] = t0; + } else { + t0 = $[0]; + } + useEffect(t0, [ref]); +} + +``` + +### 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/nonreactive-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.js new file mode 100644 index 0000000000000..8a8ab2f636be7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-ref.js @@ -0,0 +1,14 @@ +// @inferEffectDependencies +import {useEffect, useRef} from 'react'; +import {print} from 'shared-runtime'; + +/** + * Special case of `infer-effect-deps/nonreactive-dep`. + * + * We know that local `useRef` return values are stable, regardless of + * inferred memoization. + */ +function NonReactiveRefInEffect() { + const ref = useRef('initial value'); + useEffect(() => print(ref.current)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md new file mode 100644 index 0000000000000..8272d4793fb8a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; +/** + * This compiled output is technically incorrect but this is currently the same + * case as a bailout (an effect that overfires). + * + * To ensure an empty deps array is passed, we need special case + * `InferEffectDependencies` for outlined functions (likely easier) or run it + * before OutlineFunctions + */ +function OutlinedFunctionInEffect() { + useEffect(() => print('hello world!')); +} + +``` + +## Code + +```javascript +// @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; +/** + * This compiled output is technically incorrect but this is currently the same + * case as a bailout (an effect that overfires). + * + * To ensure an empty deps array is passed, we need special case + * `InferEffectDependencies` for outlined functions (likely easier) or run it + * before OutlineFunctions + */ +function OutlinedFunctionInEffect() { + useEffect(_temp); +} +function _temp() { + return print("hello world!"); +} + +``` + +### 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/outlined-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.js new file mode 100644 index 0000000000000..2ee2c94e3c249 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.js @@ -0,0 +1,14 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; +/** + * This compiled output is technically incorrect but this is currently the same + * case as a bailout (an effect that overfires). + * + * To ensure an empty deps array is passed, we need special case + * `InferEffectDependencies` for outlined functions (likely easier) or run it + * before OutlineFunctions + */ +function OutlinedFunctionInEffect() { + useEffect(() => print('hello world!')); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.expect.md new file mode 100644 index 0000000000000..fbb88fb32801d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.expect.md @@ -0,0 +1,119 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useIdentity, mutate, makeObject} from 'shared-runtime'; +import {useEffect} from 'react'; + +/** + * When a semantically non-reactive value has a pruned scope (i.e. the object + * identity becomes reactive, but the underlying value it represents should be + * constant), the compiler can choose to either + * - add it as a dependency (and rerun the effect) + * - not add it as a dependency + * + * We keep semantically non-reactive values in both memo block and effect + * dependency arrays to avoid versioning invariants e.g. `x !== y.aliasedX`. + * ```js + * function Component() { + * // obj is semantically non-reactive, but its memo scope is pruned due to + * // the interleaving hook call + * const obj = {}; + * useHook(); + * write(obj); + * + * const ref = useRef(); + * + * // this effect needs to be rerun when obj's referential identity changes, + * // because it might alias obj to a useRef / mutable store. + * useEffect(() => ref.current = obj, ???); + * + * // in a custom hook (or child component), the user might expect versioning + * // invariants to hold + * useHook(ref, obj); + * } + * + * // defined elsewhere + * function useHook(someRef, obj) { + * useEffect( + * () => assert(someRef.current === obj), + * [someRef, obj] + * ); + * } + * ``` + */ +function PrunedNonReactive() { + const obj = makeObject(); + useIdentity(null); + mutate(obj); + + useEffect(() => print(obj.value)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useIdentity, mutate, makeObject } from "shared-runtime"; +import { useEffect } from "react"; + +/** + * When a semantically non-reactive value has a pruned scope (i.e. the object + * identity becomes reactive, but the underlying value it represents should be + * constant), the compiler can choose to either + * - add it as a dependency (and rerun the effect) + * - not add it as a dependency + * + * We keep semantically non-reactive values in both memo block and effect + * dependency arrays to avoid versioning invariants e.g. `x !== y.aliasedX`. + * ```js + * function Component() { + * // obj is semantically non-reactive, but its memo scope is pruned due to + * // the interleaving hook call + * const obj = {}; + * useHook(); + * write(obj); + * + * const ref = useRef(); + * + * // this effect needs to be rerun when obj's referential identity changes, + * // because it might alias obj to a useRef / mutable store. + * useEffect(() => ref.current = obj, ???); + * + * // in a custom hook (or child component), the user might expect versioning + * // invariants to hold + * useHook(ref, obj); + * } + * + * // defined elsewhere + * function useHook(someRef, obj) { + * useEffect( + * () => assert(someRef.current === obj), + * [someRef, obj] + * ); + * } + * ``` + */ +function PrunedNonReactive() { + const $ = _c(2); + const obj = makeObject(); + useIdentity(null); + mutate(obj); + let t0; + if ($[0] !== obj.value) { + t0 = () => print(obj.value); + $[0] = obj.value; + $[1] = t0; + } else { + t0 = $[1]; + } + useEffect(t0, [obj.value]); +} + +``` + +### 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/pruned-nonreactive-obj.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.js new file mode 100644 index 0000000000000..692b97b5144b3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/pruned-nonreactive-obj.js @@ -0,0 +1,48 @@ +// @inferEffectDependencies +import {useIdentity, mutate, makeObject} from 'shared-runtime'; +import {useEffect} from 'react'; + +/** + * When a semantically non-reactive value has a pruned scope (i.e. the object + * identity becomes reactive, but the underlying value it represents should be + * constant), the compiler can choose to either + * - add it as a dependency (and rerun the effect) + * - not add it as a dependency + * + * We keep semantically non-reactive values in both memo block and effect + * dependency arrays to avoid versioning invariants e.g. `x !== y.aliasedX`. + * ```js + * function Component() { + * // obj is semantically non-reactive, but its memo scope is pruned due to + * // the interleaving hook call + * const obj = {}; + * useHook(); + * write(obj); + * + * const ref = useRef(); + * + * // this effect needs to be rerun when obj's referential identity changes, + * // because it might alias obj to a useRef / mutable store. + * useEffect(() => ref.current = obj, ???); + * + * // in a custom hook (or child component), the user might expect versioning + * // invariants to hold + * useHook(ref, obj); + * } + * + * // defined elsewhere + * function useHook(someRef, obj) { + * useEffect( + * () => assert(someRef.current === obj), + * [someRef, obj] + * ); + * } + * ``` + */ +function PrunedNonReactive() { + const obj = makeObject(); + useIdentity(null); + mutate(obj); + + useEffect(() => print(obj.value)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.expect.md new file mode 100644 index 0000000000000..82eb54a908829 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExprMerge({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a, obj.a.b)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function ReactiveMemberExprMerge(t0) { + const $ = _c(4); + const { propVal } = t0; + let t1; + if ($[0] !== propVal) { + t1 = { a: { b: propVal } }; + $[0] = propVal; + $[1] = t1; + } else { + t1 = $[1]; + } + const obj = t1; + let t2; + if ($[2] !== obj.a) { + t2 = () => print(obj.a, obj.a.b); + $[2] = obj.a; + $[3] = t2; + } else { + t2 = $[3]; + } + useEffect(t2, [obj.a]); +} + +``` + +### 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/reactive-memberexpr-merge.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.js new file mode 100644 index 0000000000000..071f5abbf545c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr-merge.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExprMerge({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a, obj.a.b)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.expect.md new file mode 100644 index 0000000000000..74c4b9eb1ee20 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExpr({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a.b)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function ReactiveMemberExpr(t0) { + const $ = _c(4); + const { propVal } = t0; + let t1; + if ($[0] !== propVal) { + t1 = { a: { b: propVal } }; + $[0] = propVal; + $[1] = t1; + } else { + t1 = $[1]; + } + const obj = t1; + let t2; + if ($[2] !== obj.a.b) { + t2 = () => print(obj.a.b); + $[2] = obj.a.b; + $[3] = t2; + } else { + t2 = $[3]; + } + useEffect(t2, [obj.a.b]); +} + +``` + +### 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/reactive-memberexpr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.js new file mode 100644 index 0000000000000..0eabc54657339 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-memberexpr.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveMemberExpr({propVal}) { + const obj = {a: {b: propVal}}; + useEffect(() => print(obj.a.b)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.expect.md new file mode 100644 index 0000000000000..7c9f21b85cdab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr({cond, propVal}) { + const obj = {a: cond ? {b: propVal} : null}; + useEffect(() => print(obj.a?.b)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr(t0) { + const $ = _c(7); + const { cond, propVal } = t0; + let t1; + if ($[0] !== cond || $[1] !== propVal) { + t1 = cond ? { b: propVal } : null; + $[0] = cond; + $[1] = propVal; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== t1) { + t2 = { a: t1 }; + $[3] = t1; + $[4] = t2; + } else { + t2 = $[4]; + } + const obj = t2; + let t3; + if ($[5] !== obj.a?.b) { + t3 = () => print(obj.a?.b); + $[5] = obj.a?.b; + $[6] = t3; + } else { + t3 = $[6]; + } + useEffect(t3, [obj.a]); +} + +``` + +### 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/reactive-optional-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.js new file mode 100644 index 0000000000000..8a76784e241b5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-optional-chain.js @@ -0,0 +1,9 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +// TODO: take optional chains as dependencies +function ReactiveMemberExpr({cond, propVal}) { + const obj = {a: cond ? {b: propVal} : null}; + useEffect(() => print(obj.a?.b)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.expect.md new file mode 100644 index 0000000000000..6443b90e4affa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveVariable({propVal}) { + const arr = [propVal]; + useEffect(() => print(arr)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import { useEffect } from "react"; +import { print } from "shared-runtime"; + +function ReactiveVariable(t0) { + const $ = _c(4); + const { propVal } = t0; + let t1; + if ($[0] !== propVal) { + t1 = [propVal]; + $[0] = propVal; + $[1] = t1; + } else { + t1 = $[1]; + } + const arr = t1; + let t2; + if ($[2] !== arr) { + t2 = () => print(arr); + $[2] = arr; + $[3] = t2; + } else { + t2 = $[3]; + } + useEffect(t2, [arr]); +} + +``` + +### 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/reactive-variable.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.js new file mode 100644 index 0000000000000..ae3ee2c8e2f64 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-variable.js @@ -0,0 +1,8 @@ +// @inferEffectDependencies +import {useEffect} from 'react'; +import {print} from 'shared-runtime'; + +function ReactiveVariable({propVal}) { + const arr = [propVal]; + useEffect(() => print(arr)); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.expect.md new file mode 100644 index 0000000000000..6302067a5a5eb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.expect.md @@ -0,0 +1,50 @@ + +## Input + +```javascript +// @inferEffectDependencies +import * as React from 'react'; + +/** + * TODO: recognize import namespace + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + React.useEffect(() => print(obj)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies +import * as React from "react"; + +/** + * TODO: recognize import namespace + */ +function NonReactiveDepInEffect() { + const $ = _c(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeObject_Primitives(); + $[0] = t0; + } else { + t0 = $[0]; + } + const obj = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = () => print(obj); + $[1] = t1; + } else { + t1 = $[1]; + } + React.useEffect(t1); +} + +``` + +### 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/todo-import-namespace-useEffect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.js new file mode 100644 index 0000000000000..4c9eec898614f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-import-namespace-useEffect.js @@ -0,0 +1,10 @@ +// @inferEffectDependencies +import * as React from 'react'; + +/** + * TODO: recognize import namespace + */ +function NonReactiveDepInEffect() { + const obj = makeObject_Primitives(); + React.useEffect(() => print(obj)); +} From bd14dde42893ce4f982c68580de1e63d5d8cbeea Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 21 Nov 2024 16:37:46 -0500 Subject: [PATCH 10/10] lints --- .../src/Inference/InferEffectDependencies.ts | 20 ++++++++++++------- .../infer-effect-dependencies.expect.md | 10 ++++++++-- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 6407956ddd828..9dc7dff78a6af 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -96,8 +96,10 @@ export function inferEffectDependencies( loc: value.loc, }); if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) { - // TODO: retry pipeline that ensures effect function expressions - // are placed into their own scope + /** + * TODO: retry pipeline that ensures effect function expressions + * are placed into their own scope + */ CompilerError.throwTodo({ reason: '[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction', @@ -194,8 +196,10 @@ function writeDependencyToInstructions( }); for (const path of dep.path) { if (path.optional) { - // TODO: instead of truncating optional paths, reuse - // instructions from hoisted dependencies block + /** + * TODO: instead of truncating optional paths, reuse + * instructions from hoisted dependencies block(s) + */ break; } const nextValue = createTemporaryPlace(env, GeneratedSource); @@ -221,9 +225,11 @@ function inferReactiveIdentifiers(fn: HIRFunction): Set { const reactiveIds: Set = new Set(); for (const [, block] of fn.body.blocks) { for (const instr of block.instructions) { - // no need to traverse into nested functions as - // 1. Their effects are recorded in `LoweredFunction.dependencies` - // 2. we don't mark `reactive` in these anyways + /** + * No need to traverse into nested functions as + * 1. their effects are recorded in `LoweredFunction.dependencies` + * 2. we don't mark `reactive` in these anyways + */ for (const place of eachInstructionOperand(instr)) { if (place.reactive) { reactiveIds.add(place.identifier.id); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md index 68714d7e4e16f..febdd005a3cb0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies.expect.md @@ -88,7 +88,13 @@ function Component(t0) { } else { t3 = $[6]; } - useEffect(t3, [foo, bar, localNonPrimitiveReactive]); + useEffect(t3, [ + foo, + bar, + ref, + localNonPrimitiveReactive, + localNonPrimitiveNonreactive, + ]); let t4; if ($[7] !== bar.baz || $[8] !== bar.qux) { t4 = () => { @@ -101,7 +107,7 @@ function Component(t0) { } else { t4 = $[9]; } - useEffect(t4, [bar, bar.qux]); + useEffect(t4, [bar.baz, bar.qux]); let t5; if ($[10] !== foo) { t5 = function f() {