From 6584a6eec488a7a155fe2231874aecf178b07a9a Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Tue, 18 Mar 2025 18:00:08 -0400 Subject: [PATCH 1/3] [compiler] Hoist dependencies from functions more conservatively (#32616) Alternative to facebook/react#31584 which sets enableTreatFunctionDepsAsConditional:true` by default. This PR changes dependency hoisting to be more conservative while trying to preserve an optimal "happy path". We assume that a function "is likely called" if we observe the following in the react function body. - a direct callsite - passed directly as a jsx attribute or child - passed directly to a hook - a direct return A function is also "likely called" if it is directly called, passed to jsx / hooks, or returned from another function that "is likely called". Note that this approach marks the function definition site with its hoistable properties (not its use site). I tried implementing use-site hoisting semantics, but it felt both unpredictable (i.e. as a developer, I can't trust that callbacks are well memoized) and not helpful (type + null checks of a value are usually colocated with their use site) In this fixture (copied here for easy reference), it should be safe to use `a.value` and `b.value` as dependencies, even though these functions are conditionally called. ```js // inner-function/nullable-objects/assume-invoked/conditional-call-chain.tsx function Component({a, b}) { const logA = () => { console.log(a.value); }; const logB = () => { console.log(b.value); }; const hasLogged = useRef(false); const log = () => { if (!hasLogged.current) { logA(); logB(); hasLogged.current = true; } }; return ; } ``` On the other hand, this means that we produce invalid output for code like manually implementing `Array.map` ```js // inner-function/nullable-objects/bug-invalid-array-map-manual.js function useFoo({arr1, arr2}) { const cb = e => arr2[0].value + e.value; const y = []; for (let i = 0; i < arr1.length; i++) { y.push(cb(arr1[i])); } return y; } ``` --- .../src/HIR/CollectHoistablePropertyLoads.ts | 193 +++++++++++++++--- ...nction-expression-prototype-call.expect.md | 4 +- ...map-named-callback-cross-context.expect.md | 133 ++++++++++++ .../array-map-named-callback-cross-context.js | 35 ++++ .../array-map-named-callback.expect.md | 108 ++++++++++ .../array-map-named-callback.js | 23 +++ ...rray-map-named-chained-callbacks.expect.md | 130 ++++++++++++ .../array-map-named-chained-callbacks.js | 26 +++ .../array-map-simple.expect.md | 111 ++++++++++ .../nullable-objects/array-map-simple.js | 25 +++ .../conditional-call-chain.expect.md | 112 ++++++++++ .../assume-invoked/conditional-call-chain.tsx | 29 +++ .../assume-invoked/conditional-call.expect.md | 85 ++++++++ .../assume-invoked/conditional-call.ts | 23 +++ .../conditionally-return-fn.expect.md | 87 ++++++++ .../assume-invoked/conditionally-return-fn.ts | 32 +++ .../assume-invoked/direct-call.expect.md | 74 +++++++ .../assume-invoked/direct-call.ts | 17 ++ ...nal-callsite-in-another-function.expect.md | 130 ++++++++++++ ...onditional-callsite-in-another-function.ts | 51 +++++ .../assume-invoked/hook-call.expect.md | 80 ++++++++ .../assume-invoked/hook-call.ts | 29 +++ .../assume-invoked/jsx-and-passed.expect.md | 80 ++++++++ .../assume-invoked/jsx-and-passed.ts | 17 ++ .../assume-invoked/jsx-function.expect.md | 75 +++++++ .../assume-invoked/jsx-function.tsx | 29 +++ .../assume-invoked/return-function.expect.md | 78 +++++++ .../assume-invoked/return-function.ts | 28 +++ .../use-memo-returned.expect.md | 82 ++++++++ .../assume-invoked/use-memo-returned.ts | 28 +++ .../bug-invalid-array-map-manual.expect.md | 68 ++++++ .../bug-invalid-array-map-manual.js | 18 ++ .../return-object-of-functions.expect.md | 57 ++++++ .../return-object-of-functions.js | 17 ++ ...function-uncond-access-local-var.expect.md | 4 +- ...nfer-object-method-uncond-access.expect.md | 4 +- ...l-dependency-on-context-variable.expect.md | 4 +- .../context-var-granular-dep.expect.md | 4 +- ...e-variables-nested-object-method.expect.md | 4 +- .../useEffect-nested-lambdas.expect.md | 25 +-- .../packages/snap/src/SproutTodoFilter.ts | 1 + 41 files changed, 2107 insertions(+), 53 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index 7b35269686..cb6854d1b3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -13,11 +13,13 @@ import { BlockId, DependencyPathEntry, GeneratedSource, + getHookKind, HIRFunction, Identifier, IdentifierId, InstructionId, InstructionValue, + LoweredFunction, PropertyLiteral, ReactiveScopeDependency, ScopeId, @@ -112,6 +114,9 @@ export function collectHoistablePropertyLoads( hoistableFromOptionals, registry, nestedFnImmutableContext: null, + assumedInvokedFns: fn.env.config.enableTreatFunctionDepsAsConditional + ? new Set() + : getAssumedInvokedFunctions(fn), }); } @@ -127,6 +132,11 @@ type CollectHoistablePropertyLoadsContext = { * but are currently kept separate for readability. */ nestedFnImmutableContext: ReadonlySet | null; + /** + * Functions which are assumed to be eventually called (as opposed to ones which might + * not be called, e.g. the 0th argument of Array.map) + */ + assumedInvokedFns: ReadonlySet; }; function collectHoistablePropertyLoadsImpl( fn: HIRFunction, @@ -338,7 +348,13 @@ function collectNonNullsInBlocks( context.registry.getOrCreateIdentifier(identifier), ); } - const nodes = new Map(); + const nodes = new Map< + BlockId, + { + block: BasicBlock; + assumedNonNullObjects: Set; + } + >(); for (const [_, block] of fn.body.blocks) { const assumedNonNullObjects = new Set( knownNonNullIdentifiers, @@ -358,32 +374,30 @@ function collectNonNullsInBlocks( ) { assumedNonNullObjects.add(maybeNonNull); } - if ( - (instr.value.kind === 'FunctionExpression' || - instr.value.kind === 'ObjectMethod') && - !fn.env.config.enableTreatFunctionDepsAsConditional - ) { + if (instr.value.kind === 'FunctionExpression') { const innerFn = instr.value.loweredFunc; - const innerHoistableMap = collectHoistablePropertyLoadsImpl( - innerFn.func, - { - ...context, - nestedFnImmutableContext: - context.nestedFnImmutableContext ?? - new Set( - innerFn.func.context - .filter(place => - isImmutableAtInstr(place.identifier, instr.id, context), - ) - .map(place => place.identifier.id), - ), - }, - ); - const innerHoistables = assertNonNull( - innerHoistableMap.get(innerFn.func.body.entry), - ); - for (const entry of innerHoistables.assumedNonNullObjects) { - assumedNonNullObjects.add(entry); + if (context.assumedInvokedFns.has(innerFn)) { + const innerHoistableMap = collectHoistablePropertyLoadsImpl( + innerFn.func, + { + ...context, + nestedFnImmutableContext: + context.nestedFnImmutableContext ?? + new Set( + innerFn.func.context + .filter(place => + isImmutableAtInstr(place.identifier, instr.id, context), + ) + .map(place => place.identifier.id), + ), + }, + ); + const innerHoistables = assertNonNull( + innerHoistableMap.get(innerFn.func.body.entry), + ); + for (const entry of innerHoistables.assumedNonNullObjects) { + assumedNonNullObjects.add(entry); + } } } } @@ -591,3 +605,130 @@ function reduceMaybeOptionalChains( } } while (changed); } + +function getAssumedInvokedFunctions( + fn: HIRFunction, + temporaries: Map< + IdentifierId, + {fn: LoweredFunction; mayInvoke: Set} + > = new Map(), +): ReadonlySet { + const hoistableFunctions = new Set(); + /** + * Step 1: Conservatively collect identifier to function expression mappings + */ + for (const block of fn.body.blocks.values()) { + for (const {lvalue, value} of block.instructions) { + /** + * Conservatively only match function expressions which can have guaranteed ssa. + * ObjectMethods and ObjectProperties do not. + */ + if (value.kind === 'FunctionExpression') { + temporaries.set(lvalue.identifier.id, { + fn: value.loweredFunc, + mayInvoke: new Set(), + }); + } else if (value.kind === 'StoreLocal') { + const lvalue = value.lvalue.place.identifier; + const maybeLoweredFunc = temporaries.get(value.value.identifier.id); + if (maybeLoweredFunc != null) { + temporaries.set(lvalue.id, maybeLoweredFunc); + } + } else if (value.kind === 'LoadLocal') { + const maybeLoweredFunc = temporaries.get(value.place.identifier.id); + if (maybeLoweredFunc != null) { + temporaries.set(lvalue.identifier.id, maybeLoweredFunc); + } + } + } + } + /** + * Step 2: Forward pass to do analysis of assumed function calls. Note that + * this is conservative and does not count indirect references through + * containers (e.g. `return {cb: () => {...}})`). + */ + for (const block of fn.body.blocks.values()) { + for (const {lvalue, value} of block.instructions) { + if (value.kind === 'CallExpression') { + const callee = value.callee; + const maybeHook = getHookKind(fn.env, callee.identifier); + const maybeLoweredFunc = temporaries.get(callee.identifier.id); + if (maybeLoweredFunc != null) { + // Direct calls + hoistableFunctions.add(maybeLoweredFunc.fn); + } else if (maybeHook != null) { + /** + * Assume arguments to all hooks are safe to invoke + */ + for (const arg of value.args) { + if (arg.kind === 'Identifier') { + const maybeLoweredFunc = temporaries.get(arg.identifier.id); + if (maybeLoweredFunc != null) { + hoistableFunctions.add(maybeLoweredFunc.fn); + } + } + } + } + } else if (value.kind === 'JsxExpression') { + /** + * Assume JSX attributes and children are safe to invoke + */ + for (const attr of value.props) { + if (attr.kind === 'JsxSpreadAttribute') { + continue; + } + const maybeLoweredFunc = temporaries.get(attr.place.identifier.id); + if (maybeLoweredFunc != null) { + hoistableFunctions.add(maybeLoweredFunc.fn); + } + } + for (const child of value.children ?? []) { + const maybeLoweredFunc = temporaries.get(child.identifier.id); + if (maybeLoweredFunc != null) { + hoistableFunctions.add(maybeLoweredFunc.fn); + } + } + } else if (value.kind === 'FunctionExpression') { + /** + * Recursively traverse into other function expressions which may invoke + * or pass already declared functions to react (e.g. as JSXAttributes). + * + * If lambda A calls lambda B, we assume lambda B is safe to invoke if + * lambda A is -- even if lambda B is conditionally called. (see + * `conditional-call-chain` fixture for example). + */ + const loweredFunc = value.loweredFunc.func; + const lambdasCalled = getAssumedInvokedFunctions( + loweredFunc, + temporaries, + ); + const maybeLoweredFunc = temporaries.get(lvalue.identifier.id); + if (maybeLoweredFunc != null) { + for (const called of lambdasCalled) { + maybeLoweredFunc.mayInvoke.add(called); + } + } + } + } + if (block.terminal.kind === 'return') { + /** + * Assume directly returned functions are safe to call + */ + const maybeLoweredFunc = temporaries.get( + block.terminal.value.identifier.id, + ); + if (maybeLoweredFunc != null) { + hoistableFunctions.add(maybeLoweredFunc.fn); + } + } + } + + for (const [_, {fn, mayInvoke}] of temporaries) { + if (hoistableFunctions.has(fn)) { + for (const called of mayInvoke) { + hoistableFunctions.add(called); + } + } + } + return hoistableFunctions; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md index 5666876f00..2df5b90890 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expression-prototype-call.expect.md @@ -23,11 +23,11 @@ import { c as _c } from "react/compiler-runtime"; function Component(props) { const $ = _c(4); let t0; - if ($[0] !== props.name) { + if ($[0] !== props) { t0 = function () { return
{props.name}
; }; - $[0] = props.name; + $[0] = props; $[1] = t0; } else { t0 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.expect.md new file mode 100644 index 0000000000..c1a6dfb3ea --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.expect.md @@ -0,0 +1,133 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Forked from array-map-simple.js + * + * Named lambdas (e.g. cb1) may be defined in the top scope of a function and + * used in a different lambda (getArrMap1). + * + * Here, we should try to determine if cb1 is actually called. In this case: + * - getArrMap1 is assumed to be called as it's passed to JSX + * - cb1 is not assumed to be called since it's only used as a call operand + */ +function useFoo({arr1, arr2}) { + const cb1 = e => arr1[0].value + e.value; + const getArrMap1 = () => arr1.map(cb1); + const cb2 = e => arr2[0].value + e.value; + const getArrMap2 = () => arr1.map(cb2); + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Forked from array-map-simple.js + * + * Named lambdas (e.g. cb1) may be defined in the top scope of a function and + * used in a different lambda (getArrMap1). + * + * Here, we should try to determine if cb1 is actually called. In this case: + * - getArrMap1 is assumed to be called as it's passed to JSX + * - cb1 is not assumed to be called since it's only used as a call operand + */ +function useFoo(t0) { + const $ = _c(13); + const { arr1, arr2 } = t0; + let t1; + if ($[0] !== arr1[0]) { + t1 = (e) => arr1[0].value + e.value; + $[0] = arr1[0]; + $[1] = t1; + } else { + t1 = $[1]; + } + const cb1 = t1; + let t2; + if ($[2] !== arr1 || $[3] !== cb1) { + t2 = () => arr1.map(cb1); + $[2] = arr1; + $[3] = cb1; + $[4] = t2; + } else { + t2 = $[4]; + } + const getArrMap1 = t2; + let t3; + if ($[5] !== arr2) { + t3 = (e_0) => arr2[0].value + e_0.value; + $[5] = arr2; + $[6] = t3; + } else { + t3 = $[6]; + } + const cb2 = t3; + let t4; + if ($[7] !== arr1 || $[8] !== cb2) { + t4 = () => arr1.map(cb2); + $[7] = arr1; + $[8] = cb2; + $[9] = t4; + } else { + t4 = $[9]; + } + const getArrMap2 = t4; + let t5; + if ($[10] !== getArrMap1 || $[11] !== getArrMap2) { + t5 = ( + + ); + $[10] = getArrMap1; + $[11] = getArrMap2; + $[12] = t5; + } else { + t5 = $[12]; + } + return t5; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ arr1: [], arr2: [] }], + sequentialRenders: [ + { arr1: [], arr2: [] }, + { arr1: [], arr2: null }, + { arr1: [{ value: 1 }, { value: 2 }], arr2: [{ value: -1 }] }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"getArrMap1":{"kind":"Function","result":[]},"getArrMap2":{"kind":"Function","result":[]},"shouldInvokeFns":true}
+
{"getArrMap1":{"kind":"Function","result":[]},"getArrMap2":{"kind":"Function","result":[]},"shouldInvokeFns":true}
+
{"getArrMap1":{"kind":"Function","result":[2,3]},"getArrMap2":{"kind":"Function","result":[0,1]},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.js new file mode 100644 index 0000000000..e905656226 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.js @@ -0,0 +1,35 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Forked from array-map-simple.js + * + * Named lambdas (e.g. cb1) may be defined in the top scope of a function and + * used in a different lambda (getArrMap1). + * + * Here, we should try to determine if cb1 is actually called. In this case: + * - getArrMap1 is assumed to be called as it's passed to JSX + * - cb1 is not assumed to be called since it's only used as a call operand + */ +function useFoo({arr1, arr2}) { + const cb1 = e => arr1[0].value + e.value; + const getArrMap1 = () => arr1.map(cb1); + const cb2 = e => arr2[0].value + e.value; + const getArrMap2 = () => arr1.map(cb2); + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.expect.md new file mode 100644 index 0000000000..a741eb59f2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.expect.md @@ -0,0 +1,108 @@ + +## Input + +```javascript +/** + * Forked from array-map-simple.js + * + * Whether lambdas are named or passed inline shouldn't affect whether we expect + * it to be called. + */ +function useFoo({arr1, arr2}) { + const cb1 = e => arr1[0].value + e.value; + const x = arr1.map(cb1); + const cb2 = e => arr2[0].value + e.value; + const y = arr1.map(cb2); + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * Forked from array-map-simple.js + * + * Whether lambdas are named or passed inline shouldn't affect whether we expect + * it to be called. + */ +function useFoo(t0) { + const $ = _c(13); + const { arr1, arr2 } = t0; + let t1; + if ($[0] !== arr1[0]) { + t1 = (e) => arr1[0].value + e.value; + $[0] = arr1[0]; + $[1] = t1; + } else { + t1 = $[1]; + } + const cb1 = t1; + let t2; + if ($[2] !== arr1 || $[3] !== cb1) { + t2 = arr1.map(cb1); + $[2] = arr1; + $[3] = cb1; + $[4] = t2; + } else { + t2 = $[4]; + } + const x = t2; + let t3; + if ($[5] !== arr2) { + t3 = (e_0) => arr2[0].value + e_0.value; + $[5] = arr2; + $[6] = t3; + } else { + t3 = $[6]; + } + const cb2 = t3; + let t4; + if ($[7] !== arr1 || $[8] !== cb2) { + t4 = arr1.map(cb2); + $[7] = arr1; + $[8] = cb2; + $[9] = t4; + } else { + t4 = $[9]; + } + const y = t4; + let t5; + if ($[10] !== x || $[11] !== y) { + t5 = [x, y]; + $[10] = x; + $[11] = y; + $[12] = t5; + } else { + t5 = $[12]; + } + return t5; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ arr1: [], arr2: [] }], + sequentialRenders: [ + { arr1: [], arr2: [] }, + { arr1: [], arr2: null }, + { arr1: [{ value: 1 }, { value: 2 }], arr2: [{ value: -1 }] }, + ], +}; + +``` + +### Eval output +(kind: ok) [[],[]] +[[],[]] +[[2,3],[0,1]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.js new file mode 100644 index 0000000000..bf4f3ba661 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback.js @@ -0,0 +1,23 @@ +/** + * Forked from array-map-simple.js + * + * Whether lambdas are named or passed inline shouldn't affect whether we expect + * it to be called. + */ +function useFoo({arr1, arr2}) { + const cb1 = e => arr1[0].value + e.value; + const x = arr1.map(cb1); + const cb2 = e => arr2[0].value + e.value; + const y = arr1.map(cb2); + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.expect.md new file mode 100644 index 0000000000..9bf77fd127 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.expect.md @@ -0,0 +1,130 @@ + +## Input + +```javascript +/** + * Forked from array-map-simple.js + * + * Here, getVal1 has a known callsite in `cb1`, but `cb1` isn't known to be + * called (it's only passed to array.map). In this case, we should be + * conservative and assume that all named lambdas are conditionally called. + */ +function useFoo({arr1, arr2}) { + const getVal1 = () => arr1[0].value; + const cb1 = e => getVal1() + e.value; + const x = arr1.map(cb1); + const getVal2 = () => arr2[0].value; + const cb2 = e => getVal2() + e.value; + const y = arr1.map(cb2); + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * Forked from array-map-simple.js + * + * Here, getVal1 has a known callsite in `cb1`, but `cb1` isn't known to be + * called (it's only passed to array.map). In this case, we should be + * conservative and assume that all named lambdas are conditionally called. + */ +function useFoo(t0) { + const $ = _c(17); + const { arr1, arr2 } = t0; + let t1; + if ($[0] !== arr1[0]) { + t1 = () => arr1[0].value; + $[0] = arr1[0]; + $[1] = t1; + } else { + t1 = $[1]; + } + const getVal1 = t1; + let t2; + if ($[2] !== getVal1) { + t2 = (e) => getVal1() + e.value; + $[2] = getVal1; + $[3] = t2; + } else { + t2 = $[3]; + } + const cb1 = t2; + let t3; + if ($[4] !== arr1 || $[5] !== cb1) { + t3 = arr1.map(cb1); + $[4] = arr1; + $[5] = cb1; + $[6] = t3; + } else { + t3 = $[6]; + } + const x = t3; + let t4; + if ($[7] !== arr2) { + t4 = () => arr2[0].value; + $[7] = arr2; + $[8] = t4; + } else { + t4 = $[8]; + } + const getVal2 = t4; + let t5; + if ($[9] !== getVal2) { + t5 = (e_0) => getVal2() + e_0.value; + $[9] = getVal2; + $[10] = t5; + } else { + t5 = $[10]; + } + const cb2 = t5; + let t6; + if ($[11] !== arr1 || $[12] !== cb2) { + t6 = arr1.map(cb2); + $[11] = arr1; + $[12] = cb2; + $[13] = t6; + } else { + t6 = $[13]; + } + const y = t6; + let t7; + if ($[14] !== x || $[15] !== y) { + t7 = [x, y]; + $[14] = x; + $[15] = y; + $[16] = t7; + } else { + t7 = $[16]; + } + return t7; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ arr1: [], arr2: [] }], + sequentialRenders: [ + { arr1: [], arr2: [] }, + { arr1: [], arr2: null }, + { arr1: [{ value: 1 }, { value: 2 }], arr2: [{ value: -1 }] }, + ], +}; + +``` + +### Eval output +(kind: ok) [[],[]] +[[],[]] +[[2,3],[0,1]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.js new file mode 100644 index 0000000000..598faba46d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-chained-callbacks.js @@ -0,0 +1,26 @@ +/** + * Forked from array-map-simple.js + * + * Here, getVal1 has a known callsite in `cb1`, but `cb1` isn't known to be + * called (it's only passed to array.map). In this case, we should be + * conservative and assume that all named lambdas are conditionally called. + */ +function useFoo({arr1, arr2}) { + const getVal1 = () => arr1[0].value; + const cb1 = e => getVal1() + e.value; + const x = arr1.map(cb1); + const getVal2 = () => arr2[0].value; + const cb2 = e => getVal2() + e.value; + const y = arr1.map(cb2); + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.expect.md new file mode 100644 index 0000000000..5eb97aa1bf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.expect.md @@ -0,0 +1,111 @@ + +## Input + +```javascript +/** + * Test that we're not hoisting property reads from lambdas that are created to + * pass to opaque functions, which often have maybe-invoke semantics. + * + * In this example, we shouldn't hoist `arr[0].value` out of the lambda. + * ```js + * e => arr[0].value + e.value <-- created to pass to map + * arr.map() <-- argument only invoked if array is non-empty + * ``` + */ +function useFoo({arr1, arr2}) { + const x = arr1.map(e => arr1[0].value + e.value); + const y = arr1.map(e => arr2[0].value + e.value); + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * Test that we're not hoisting property reads from lambdas that are created to + * pass to opaque functions, which often have maybe-invoke semantics. + * + * In this example, we shouldn't hoist `arr[0].value` out of the lambda. + * ```js + * e => arr[0].value + e.value <-- created to pass to map + * arr.map() <-- argument only invoked if array is non-empty + * ``` + */ +function useFoo(t0) { + const $ = _c(12); + const { arr1, arr2 } = t0; + let t1; + if ($[0] !== arr1) { + let t2; + if ($[2] !== arr1[0]) { + t2 = (e) => arr1[0].value + e.value; + $[2] = arr1[0]; + $[3] = t2; + } else { + t2 = $[3]; + } + t1 = arr1.map(t2); + $[0] = arr1; + $[1] = t1; + } else { + t1 = $[1]; + } + const x = t1; + let t2; + if ($[4] !== arr1 || $[5] !== arr2) { + let t3; + if ($[7] !== arr2) { + t3 = (e_0) => arr2[0].value + e_0.value; + $[7] = arr2; + $[8] = t3; + } else { + t3 = $[8]; + } + t2 = arr1.map(t3); + $[4] = arr1; + $[5] = arr2; + $[6] = t2; + } else { + t2 = $[6]; + } + const y = t2; + let t3; + if ($[9] !== x || $[10] !== y) { + t3 = [x, y]; + $[9] = x; + $[10] = y; + $[11] = t3; + } else { + t3 = $[11]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ arr1: [], arr2: [] }], + sequentialRenders: [ + { arr1: [], arr2: [] }, + { arr1: [], arr2: null }, + { arr1: [{ value: 1 }, { value: 2 }], arr2: [{ value: -1 }] }, + ], +}; + +``` + +### Eval output +(kind: ok) [[],[]] +[[],[]] +[[2,3],[0,1]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.js new file mode 100644 index 0000000000..80748d6131 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-simple.js @@ -0,0 +1,25 @@ +/** + * Test that we're not hoisting property reads from lambdas that are created to + * pass to opaque functions, which often have maybe-invoke semantics. + * + * In this example, we shouldn't hoist `arr[0].value` out of the lambda. + * ```js + * e => arr[0].value + e.value <-- created to pass to map + * arr.map() <-- argument only invoked if array is non-empty + * ``` + */ +function useFoo({arr1, arr2}) { + const x = arr1.map(e => arr1[0].value + e.value); + const y = arr1.map(e => arr2[0].value + e.value); + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.expect.md new file mode 100644 index 0000000000..e0dc1eeb5f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.expect.md @@ -0,0 +1,112 @@ + +## Input + +```javascript +import {useRef} from 'react'; +import {Stringify} from 'shared-runtime'; + +function Component({a, b}) { + const logA = () => { + console.log(a.value); + }; + const logB = () => { + console.log(b.value); + }; + const hasLogged = useRef(false); + const log = () => { + if (!hasLogged.current) { + logA(); + logB(); + hasLogged.current = true; + } + }; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {value: 1}, b: {value: 2}}], + sequentialRenders: [ + {a: {value: 1}, b: {value: 2}}, + {a: {value: 3}, b: {value: 4}}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useRef } from "react"; +import { Stringify } from "shared-runtime"; + +function Component(t0) { + const $ = _c(9); + const { a, b } = t0; + let t1; + if ($[0] !== a.value) { + t1 = () => { + console.log(a.value); + }; + $[0] = a.value; + $[1] = t1; + } else { + t1 = $[1]; + } + const logA = t1; + let t2; + if ($[2] !== b.value) { + t2 = () => { + console.log(b.value); + }; + $[2] = b.value; + $[3] = t2; + } else { + t2 = $[3]; + } + const logB = t2; + + const hasLogged = useRef(false); + let t3; + if ($[4] !== logA || $[5] !== logB) { + t3 = () => { + if (!hasLogged.current) { + logA(); + logB(); + hasLogged.current = true; + } + }; + $[4] = logA; + $[5] = logB; + $[6] = t3; + } else { + t3 = $[6]; + } + const log = t3; + let t4; + if ($[7] !== log) { + t4 = ; + $[7] = log; + $[8] = t4; + } else { + t4 = $[8]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: { value: 1 }, b: { value: 2 } }], + sequentialRenders: [ + { a: { value: 1 }, b: { value: 2 } }, + { a: { value: 3 }, b: { value: 4 } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"log":{"kind":"Function"},"shouldInvokeFns":true}
+
{"log":{"kind":"Function"},"shouldInvokeFns":true}
+logs: [1,2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.tsx new file mode 100644 index 0000000000..746287fe60 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call-chain.tsx @@ -0,0 +1,29 @@ +import {useRef} from 'react'; +import {Stringify} from 'shared-runtime'; + +function Component({a, b}) { + const logA = () => { + console.log(a.value); + }; + const logB = () => { + console.log(b.value); + }; + const hasLogged = useRef(false); + const log = () => { + if (!hasLogged.current) { + logA(); + logB(); + hasLogged.current = true; + } + }; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {value: 1}, b: {value: 2}}], + sequentialRenders: [ + {a: {value: 1}, b: {value: 2}}, + {a: {value: 3}, b: {value: 4}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.expect.md new file mode 100644 index 0000000000..0080fd0468 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.expect.md @@ -0,0 +1,85 @@ + +## Input + +```javascript +import {useState} from 'react'; +import {useIdentity} from 'shared-runtime'; + +/** + * Assume that conditionally called functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback({obj}: {obj: {value: number}}) { + const [state, setState] = useState(0); + const cb = () => { + if (obj.value !== 0) setState(obj.value); + }; + useIdentity(null); + if (state === 0) { + cb(); + } + return {cb}; +} +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useState } from "react"; +import { useIdentity } from "shared-runtime"; + +/** + * Assume that conditionally called functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback(t0) { + const $ = _c(4); + const { obj } = t0; + const [state, setState] = useState(0); + let t1; + if ($[0] !== obj.value) { + t1 = () => { + if (obj.value !== 0) { + setState(obj.value); + } + }; + $[0] = obj.value; + $[1] = t1; + } else { + t1 = $[1]; + } + const cb = t1; + + useIdentity(null); + if (state === 0) { + cb(); + } + let t2; + if ($[2] !== cb) { + t2 = { cb }; + $[2] = cb; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{ obj: { value: 1 } }], + sequentialRenders: [{ obj: { value: 1 } }, { obj: { value: 2 } }], +}; + +``` + +### Eval output +(kind: ok) {"cb":"[[ function params=0 ]]"} +{"cb":"[[ function params=0 ]]"} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.ts new file mode 100644 index 0000000000..12d92b726f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditional-call.ts @@ -0,0 +1,23 @@ +import {useState} from 'react'; +import {useIdentity} from 'shared-runtime'; + +/** + * Assume that conditionally called functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback({obj}: {obj: {value: number}}) { + const [state, setState] = useState(0); + const cb = () => { + if (obj.value !== 0) setState(obj.value); + }; + useIdentity(null); + if (state === 0) { + cb(); + } + return {cb}; +} +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.expect.md new file mode 100644 index 0000000000..77b62bc8c2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.expect.md @@ -0,0 +1,87 @@ + +## Input + +```javascript +import {createHookWrapper} from 'shared-runtime'; + +/** + * Assume that conditionally returned functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback({ + obj, + shouldMakeCb, + setState, +}: { + obj: {value: number}; + shouldMakeCb: boolean; + setState: (newState: number) => void; +}) { + const cb = () => setState(obj.value); + if (shouldMakeCb) return cb; + else return null; +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, shouldMakeCb: true, setState}], + sequentialRenders: [ + {obj: {value: 1}, shouldMakeCb: true, setState}, + {obj: {value: 2}, shouldMakeCb: true, setState}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { createHookWrapper } from "shared-runtime"; + +/** + * Assume that conditionally returned functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback(t0) { + const $ = _c(3); + const { obj, shouldMakeCb, setState } = t0; + let t1; + if ($[0] !== obj.value || $[1] !== setState) { + t1 = () => setState(obj.value); + $[0] = obj.value; + $[1] = setState; + $[2] = t1; + } else { + t1 = $[2]; + } + const cb = t1; + if (shouldMakeCb) { + return cb; + } else { + return null; + } +} + +const setState = (arg: number) => { + "use no memo"; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{ obj: { value: 1 }, shouldMakeCb: true, setState }], + sequentialRenders: [ + { obj: { value: 1 }, shouldMakeCb: true, setState }, + { obj: { value: 2 }, shouldMakeCb: true, setState }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"result":{"kind":"Function","result":1},"shouldInvokeFns":true}
+
{"result":{"kind":"Function","result":2},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.ts new file mode 100644 index 0000000000..08dde03b03 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/conditionally-return-fn.ts @@ -0,0 +1,32 @@ +import {createHookWrapper} from 'shared-runtime'; + +/** + * Assume that conditionally returned functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback({ + obj, + shouldMakeCb, + setState, +}: { + obj: {value: number}; + shouldMakeCb: boolean; + setState: (newState: number) => void; +}) { + const cb = () => setState(obj.value); + if (shouldMakeCb) return cb; + else return null; +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, shouldMakeCb: true, setState}], + sequentialRenders: [ + {obj: {value: 1}, shouldMakeCb: true, setState}, + {obj: {value: 2}, shouldMakeCb: true, setState}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.expect.md new file mode 100644 index 0000000000..2f31be1ffe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.expect.md @@ -0,0 +1,74 @@ + +## Input + +```javascript +import {useState} from 'react'; +import {useIdentity} from 'shared-runtime'; + +function useMakeCallback({obj}: {obj: {value: number}}) { + const [state, setState] = useState(0); + const cb = () => { + if (obj.value !== state) setState(obj.value); + }; + useIdentity(); + cb(); + return [cb]; +} +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useState } from "react"; +import { useIdentity } from "shared-runtime"; + +function useMakeCallback(t0) { + const $ = _c(5); + const { obj } = t0; + const [state, setState] = useState(0); + let t1; + if ($[0] !== obj.value || $[1] !== state) { + t1 = () => { + if (obj.value !== state) { + setState(obj.value); + } + }; + $[0] = obj.value; + $[1] = state; + $[2] = t1; + } else { + t1 = $[2]; + } + const cb = t1; + + useIdentity(); + cb(); + let t2; + if ($[3] !== cb) { + t2 = [cb]; + $[3] = cb; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{ obj: { value: 1 } }], + sequentialRenders: [{ obj: { value: 1 } }, { obj: { value: 2 } }], +}; + +``` + +### Eval output +(kind: ok) ["[[ function params=0 ]]"] +["[[ function params=0 ]]"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.ts new file mode 100644 index 0000000000..c2e8292297 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/direct-call.ts @@ -0,0 +1,17 @@ +import {useState} from 'react'; +import {useIdentity} from 'shared-runtime'; + +function useMakeCallback({obj}: {obj: {value: number}}) { + const [state, setState] = useState(0); + const cb = () => { + if (obj.value !== state) setState(obj.value); + }; + useIdentity(); + cb(); + return [cb]; +} +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.expect.md new file mode 100644 index 0000000000..8301912b02 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.expect.md @@ -0,0 +1,130 @@ + +## Input + +```javascript +import {createHookWrapper} from 'shared-runtime'; + +/** + * (Given that the returned lambda is assumed to be invoked, see + * return-function) + * + * If lambda A conditionally calls lambda B, optimistically assume that property + * loads from lambda B has the same hoistability of ones from lambda A. This + * helps optimize components / hooks that create and chain many helper + * functions. + * + * Type systems and code readability encourage developers to colocate length and + * null checks values in the same function as where values are used. i.e. + * developers are unlikely to write the following code. + * ```js + * function useFoo(obj, objNotNullAndHasElements) { + * // ... + * const get0th = () => obj.arr[0].value; + * return () => objNotNullAndHasElements ? get0th : undefined; + * } + * ``` + * + * In Meta code, this assumption helps reduce the number of memo dependency + * deopts. + */ +function useMakeCallback({ + obj, + cond, + setState, +}: { + obj: {value: number}; + cond: boolean; + setState: (newState: number) => void; +}) { + const cb = () => setState(obj.value); + // cb's property loads are assumed to be hoistable to the start of this lambda + return () => (cond ? cb() : undefined); +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, cond: true, setState}], + sequentialRenders: [ + {obj: {value: 1}, cond: true, setState}, + {obj: {value: 2}, cond: true, setState}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { createHookWrapper } from "shared-runtime"; + +/** + * (Given that the returned lambda is assumed to be invoked, see + * return-function) + * + * If lambda A conditionally calls lambda B, optimistically assume that property + * loads from lambda B has the same hoistability of ones from lambda A. This + * helps optimize components / hooks that create and chain many helper + * functions. + * + * Type systems and code readability encourage developers to colocate length and + * null checks values in the same function as where values are used. i.e. + * developers are unlikely to write the following code. + * ```js + * function useFoo(obj, objNotNullAndHasElements) { + * // ... + * const get0th = () => obj.arr[0].value; + * return () => objNotNullAndHasElements ? get0th : undefined; + * } + * ``` + * + * In Meta code, this assumption helps reduce the number of memo dependency + * deopts. + */ +function useMakeCallback(t0) { + const $ = _c(6); + const { obj, cond, setState } = t0; + let t1; + if ($[0] !== obj.value || $[1] !== setState) { + t1 = () => setState(obj.value); + $[0] = obj.value; + $[1] = setState; + $[2] = t1; + } else { + t1 = $[2]; + } + const cb = t1; + let t2; + if ($[3] !== cb || $[4] !== cond) { + t2 = () => (cond ? cb() : undefined); + $[3] = cb; + $[4] = cond; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +const setState = (arg: number) => { + "use no memo"; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{ obj: { value: 1 }, cond: true, setState }], + sequentialRenders: [ + { obj: { value: 1 }, cond: true, setState }, + { obj: { value: 2 }, cond: true, setState }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"result":{"kind":"Function","result":1},"shouldInvokeFns":true}
+
{"result":{"kind":"Function","result":2},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.ts new file mode 100644 index 0000000000..b6283aa6a6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/function-with-conditional-callsite-in-another-function.ts @@ -0,0 +1,51 @@ +import {createHookWrapper} from 'shared-runtime'; + +/** + * (Given that the returned lambda is assumed to be invoked, see + * return-function) + * + * If lambda A conditionally calls lambda B, optimistically assume that property + * loads from lambda B has the same hoistability of ones from lambda A. This + * helps optimize components / hooks that create and chain many helper + * functions. + * + * Type systems and code readability encourage developers to colocate length and + * null checks values in the same function as where values are used. i.e. + * developers are unlikely to write the following code. + * ```js + * function useFoo(obj, objNotNullAndHasElements) { + * // ... + * const get0th = () => obj.arr[0].value; + * return () => objNotNullAndHasElements ? get0th : undefined; + * } + * ``` + * + * In Meta code, this assumption helps reduce the number of memo dependency + * deopts. + */ +function useMakeCallback({ + obj, + cond, + setState, +}: { + obj: {value: number}; + cond: boolean; + setState: (newState: number) => void; +}) { + const cb = () => setState(obj.value); + // cb's property loads are assumed to be hoistable to the start of this lambda + return () => (cond ? cb() : undefined); +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, cond: true, setState}], + sequentialRenders: [ + {obj: {value: 1}, cond: true, setState}, + {obj: {value: 2}, cond: true, setState}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.expect.md new file mode 100644 index 0000000000..ab8326a228 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +import {createHookWrapper, useIdentity} from 'shared-runtime'; + +/** + * Assume that functions passed hook arguments are invoked and that their + * property loads are hoistable. + */ +function useMakeCallback({ + obj, + setState, +}: { + obj: {value: number}; + setState: (newState: number) => void; +}) { + const cb = useIdentity(() => setState(obj.value)); + return cb; +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, setState}], + sequentialRenders: [ + {obj: {value: 1}, setState}, + {obj: {value: 2}, setState}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { createHookWrapper, useIdentity } from "shared-runtime"; + +/** + * Assume that functions passed hook arguments are invoked and that their + * property loads are hoistable. + */ +function useMakeCallback(t0) { + const $ = _c(3); + const { obj, setState } = t0; + let t1; + if ($[0] !== obj.value || $[1] !== setState) { + t1 = () => setState(obj.value); + $[0] = obj.value; + $[1] = setState; + $[2] = t1; + } else { + t1 = $[2]; + } + const cb = useIdentity(t1); + return cb; +} + +const setState = (arg: number) => { + "use no memo"; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{ obj: { value: 1 }, setState }], + sequentialRenders: [ + { obj: { value: 1 }, setState }, + { obj: { value: 2 }, setState }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"result":{"kind":"Function","result":1},"shouldInvokeFns":true}
+
{"result":{"kind":"Function","result":2},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.ts new file mode 100644 index 0000000000..a1ab6e18c5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/hook-call.ts @@ -0,0 +1,29 @@ +import {createHookWrapper, useIdentity} from 'shared-runtime'; + +/** + * Assume that functions passed hook arguments are invoked and that their + * property loads are hoistable. + */ +function useMakeCallback({ + obj, + setState, +}: { + obj: {value: number}; + setState: (newState: number) => void; +}) { + const cb = useIdentity(() => setState(obj.value)); + return cb; +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, setState}], + sequentialRenders: [ + {obj: {value: 1}, setState}, + {obj: {value: 2}, setState}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.expect.md new file mode 100644 index 0000000000..688901a4e2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +import {createHookWrapper} from 'shared-runtime'; + +function useFoo({arr1}) { + const cb1 = e => arr1[0].value + e.value; + const x = arr1.map(cb1); + return [x, cb1]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useFoo), + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { createHookWrapper } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(8); + const { arr1 } = t0; + let t1; + if ($[0] !== arr1[0]) { + t1 = (e) => arr1[0].value + e.value; + $[0] = arr1[0]; + $[1] = t1; + } else { + t1 = $[1]; + } + const cb1 = t1; + let t2; + if ($[2] !== arr1 || $[3] !== cb1) { + t2 = arr1.map(cb1); + $[2] = arr1; + $[3] = cb1; + $[4] = t2; + } else { + t2 = $[4]; + } + const x = t2; + let t3; + if ($[5] !== cb1 || $[6] !== x) { + t3 = [x, cb1]; + $[5] = cb1; + $[6] = x; + $[7] = t3; + } else { + t3 = $[7]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useFoo), + params: [{ arr1: [], arr2: [] }], + sequentialRenders: [ + { arr1: [], arr2: [] }, + { arr1: [], arr2: null }, + { arr1: [{ value: 1 }, { value: 2 }], arr2: [{ value: -1 }] }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"result":[[],"[[ function params=1 ]]"],"shouldInvokeFns":true}
+
{"result":[[],"[[ function params=1 ]]"],"shouldInvokeFns":true}
+
{"result":[[2,3],"[[ function params=1 ]]"],"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.ts new file mode 100644 index 0000000000..c08701022a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-and-passed.ts @@ -0,0 +1,17 @@ +import {createHookWrapper} from 'shared-runtime'; + +function useFoo({arr1}) { + const cb1 = e => arr1[0].value + e.value; + const x = arr1.map(cb1); + return [x, cb1]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useFoo), + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.expect.md new file mode 100644 index 0000000000..76228fc249 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.expect.md @@ -0,0 +1,75 @@ + +## Input + +```javascript +// @flow +import {Stringify} from 'shared-runtime'; + +/** + * Assume that functions captured directly as jsx attributes are invoked and + * that their property loads are hoistable. + */ +function useMakeCallback({ + obj, + setState, +}: { + obj: {value: number}; + setState: (newState: number) => void; +}) { + return setState(obj.value)} shouldInvokeFns={true} />; +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}, setState}], + sequentialRenders: [ + {obj: {value: 1}, setState}, + {obj: {value: 2}, setState}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +function useMakeCallback(t0) { + const $ = _c(3); + const { obj, setState } = t0; + let t1; + if ($[0] !== obj.value || $[1] !== setState) { + t1 = setState(obj.value)} shouldInvokeFns={true} />; + $[0] = obj.value; + $[1] = setState; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +const setState = (arg: number) => { + "use no memo"; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{ obj: { value: 1 }, setState }], + sequentialRenders: [ + { obj: { value: 1 }, setState }, + { obj: { value: 2 }, setState }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}
+
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.tsx new file mode 100644 index 0000000000..316a0a03fb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/jsx-function.tsx @@ -0,0 +1,29 @@ +// @flow +import {Stringify} from 'shared-runtime'; + +/** + * Assume that functions captured directly as jsx attributes are invoked and + * that their property loads are hoistable. + */ +function useMakeCallback({ + obj, + setState, +}: { + obj: {value: number}; + setState: (newState: number) => void; +}) { + return setState(obj.value)} shouldInvokeFns={true} />; +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}, setState}], + sequentialRenders: [ + {obj: {value: 1}, setState}, + {obj: {value: 2}, setState}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.expect.md new file mode 100644 index 0000000000..31e317d07e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +import {createHookWrapper} from 'shared-runtime'; + +/** + * Assume that directly returned functions are invoked and that their property + * loads are hoistable. + */ +function useMakeCallback({ + obj, + setState, +}: { + obj: {value: number}; + setState: (newState: number) => void; +}) { + return () => setState(obj.value); +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, setState}], + sequentialRenders: [ + {obj: {value: 1}, setState}, + {obj: {value: 2}, setState}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { createHookWrapper } from "shared-runtime"; + +/** + * Assume that directly returned functions are invoked and that their property + * loads are hoistable. + */ +function useMakeCallback(t0) { + const $ = _c(3); + const { obj, setState } = t0; + let t1; + if ($[0] !== obj.value || $[1] !== setState) { + t1 = () => setState(obj.value); + $[0] = obj.value; + $[1] = setState; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +const setState = (arg: number) => { + "use no memo"; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{ obj: { value: 1 }, setState }], + sequentialRenders: [ + { obj: { value: 1 }, setState }, + { obj: { value: 2 }, setState }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"result":{"kind":"Function","result":1},"shouldInvokeFns":true}
+
{"result":{"kind":"Function","result":2},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.ts new file mode 100644 index 0000000000..f0e0ac77f0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/return-function.ts @@ -0,0 +1,28 @@ +import {createHookWrapper} from 'shared-runtime'; + +/** + * Assume that directly returned functions are invoked and that their property + * loads are hoistable. + */ +function useMakeCallback({ + obj, + setState, +}: { + obj: {value: number}; + setState: (newState: number) => void; +}) { + return () => setState(obj.value); +} + +const setState = (arg: number) => { + 'use no memo'; + return arg; +}; +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useMakeCallback), + params: [{obj: {value: 1}, setState}], + sequentialRenders: [ + {obj: {value: 1}, setState}, + {obj: {value: 2}, setState}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.expect.md new file mode 100644 index 0000000000..e750b8ab84 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.expect.md @@ -0,0 +1,82 @@ + +## Input + +```javascript +import {useState, useMemo} from 'react'; +import {useIdentity} from 'shared-runtime'; + +/** + * Assume that conditionally called functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback({ + obj, + shouldSynchronizeState, +}: { + obj: {value: number}; + shouldSynchronizeState: boolean; +}) { + const [state, setState] = useState(0); + const cb = useMemo(() => { + return () => { + if (obj.value !== 0) setState(obj.value); + }; + }, [obj.value, shouldSynchronizeState]); + useIdentity(null); + return cb; +} +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useState, useMemo } from "react"; +import { useIdentity } from "shared-runtime"; + +/** + * Assume that conditionally called functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback(t0) { + const $ = _c(2); + const { obj, shouldSynchronizeState } = t0; + + const [, setState] = useState(0); + let t1; + let t2; + if ($[0] !== obj.value) { + t2 = () => { + if (obj.value !== 0) { + setState(obj.value); + } + }; + $[0] = obj.value; + $[1] = t2; + } else { + t2 = $[1]; + } + t1 = t2; + const cb = t1; + + useIdentity(null); + return cb; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{ obj: { value: 1 } }], + sequentialRenders: [{ obj: { value: 1 } }, { obj: { value: 2 } }], +}; + +``` + +### Eval output +(kind: ok) "[[ function params=0 ]]" +"[[ function params=0 ]]" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.ts new file mode 100644 index 0000000000..6cb2e44a2b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/assume-invoked/use-memo-returned.ts @@ -0,0 +1,28 @@ +import {useState, useMemo} from 'react'; +import {useIdentity} from 'shared-runtime'; + +/** + * Assume that conditionally called functions can be invoked and that their + * property loads are hoistable to the function declaration site. + */ +function useMakeCallback({ + obj, + shouldSynchronizeState, +}: { + obj: {value: number}; + shouldSynchronizeState: boolean; +}) { + const [state, setState] = useState(0); + const cb = useMemo(() => { + return () => { + if (obj.value !== 0) setState(obj.value); + }; + }, [obj.value, shouldSynchronizeState]); + useIdentity(null); + return cb; +} +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.expect.md new file mode 100644 index 0000000000..d00e71e14f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.expect.md @@ -0,0 +1,68 @@ + +## Input + +```javascript +function useFoo({arr1, arr2}) { + const cb = e => arr2[0].value + e.value; + const y = []; + for (let i = 0; i < arr1.length; i++) { + y.push(cb(arr1[i])); + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function useFoo(t0) { + const $ = _c(5); + const { arr1, arr2 } = t0; + let t1; + if ($[0] !== arr2[0].value) { + t1 = (e) => arr2[0].value + e.value; + $[0] = arr2[0].value; + $[1] = t1; + } else { + t1 = $[1]; + } + const cb = t1; + let y; + if ($[2] !== arr1 || $[3] !== cb) { + y = []; + for (let i = 0; i < arr1.length; i++) { + y.push(cb(arr1[i])); + } + $[2] = arr1; + $[3] = cb; + $[4] = y; + } else { + y = $[4]; + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ arr1: [], arr2: [] }], + sequentialRenders: [ + { arr1: [], arr2: [] }, + { arr1: [], arr2: null }, + { arr1: [{ value: 1 }, { value: 2 }], arr2: [{ value: -1 }] }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.js new file mode 100644 index 0000000000..eed7561306 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/bug-invalid-array-map-manual.js @@ -0,0 +1,18 @@ +function useFoo({arr1, arr2}) { + const cb = e => arr2[0].value + e.value; + const y = []; + for (let i = 0; i < arr1.length; i++) { + y.push(cb(arr1[i])); + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arr1: [], arr2: []}], + sequentialRenders: [ + {arr1: [], arr2: []}, + {arr1: [], arr2: null}, + {arr1: [{value: 1}, {value: 2}], arr2: [{value: -1}]}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.expect.md new file mode 100644 index 0000000000..5ccf5b5edc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +/** + * Assume that only directly returned functions or JSX attributes are invoked. + * Conservatively estimate that functions wrapped in objects or other containers + * might never be called (and therefore their property loads are not hoistable). + */ +function useMakeCallback({arr}) { + return { + getElement0: () => arr[0].value, + getElement1: () => arr[1].value, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{arr: [1, 2]}], + sequentialRenders: [{arr: [1, 2]}, {arr: []}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * Assume that only directly returned functions or JSX attributes are invoked. + * Conservatively estimate that functions wrapped in objects or other containers + * might never be called (and therefore their property loads are not hoistable). + */ +function useMakeCallback(t0) { + const $ = _c(2); + const { arr } = t0; + let t1; + if ($[0] !== arr) { + t1 = { getElement0: () => arr[0].value, getElement1: () => arr[1].value }; + $[0] = arr; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{ arr: [1, 2] }], + sequentialRenders: [{ arr: [1, 2] }, { arr: [] }], +}; + +``` + +### Eval output +(kind: ok) {"getElement0":"[[ function params=0 ]]","getElement1":"[[ function params=0 ]]"} +{"getElement0":"[[ function params=0 ]]","getElement1":"[[ function params=0 ]]"} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.js new file mode 100644 index 0000000000..6aface49f8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/return-object-of-functions.js @@ -0,0 +1,17 @@ +/** + * Assume that only directly returned functions or JSX attributes are invoked. + * Conservatively estimate that functions wrapped in objects or other containers + * might never be called (and therefore their property loads are not hoistable). + */ +function useMakeCallback({arr}) { + return { + getElement0: () => arr[0].value, + getElement1: () => arr[1].value, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useMakeCallback, + params: [{arr: [1, 2]}], + sequentialRenders: [{arr: [1, 2]}, {arr: []}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-nested-function-uncond-access-local-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-nested-function-uncond-access-local-var.expect.md index ca65ce72bc..53d3d04531 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-nested-function-uncond-access-local-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-nested-function-uncond-access-local-var.expect.md @@ -41,9 +41,9 @@ function useFoo(t0) { local = $[1]; } let t1; - if ($[2] !== local.b.c) { + if ($[2] !== local) { t1 = () => [() => local.b.c]; - $[2] = local.b.c; + $[2] = local; $[3] = t1; } else { t1 = $[3]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-object-method-uncond-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-object-method-uncond-access.expect.md index 7d75470550..f8a8af1fd4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-object-method-uncond-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-object-method-uncond-access.expect.md @@ -34,13 +34,13 @@ function useFoo(t0) { const $ = _c(4); const { a } = t0; let t1; - if ($[0] !== a.b.c) { + if ($[0] !== a) { t1 = { fn() { return identity(a.b.c); }, }; - $[0] = a.b.c; + $[0] = a; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md index ceaa350012..963024e887 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md @@ -51,7 +51,7 @@ import { identity } from "shared-runtime"; function Component(props) { const $ = _c(4); let x; - if ($[0] !== props.cond) { + if ($[0] !== props) { const f = () => { if (props.cond) { x = 1; @@ -62,7 +62,7 @@ function Component(props) { const f2 = identity(f); f2(); - $[0] = props.cond; + $[0] = props; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md index d72f34b4fd..f887870197 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md @@ -82,9 +82,9 @@ function Component(t0) { contextVar = $[2]; } let t1; - if ($[3] !== contextVar.val) { + if ($[3] !== contextVar) { t1 = { cb: () => contextVar.val * 4 }; - $[3] = contextVar.val; + $[3] = contextVar; $[4] = t1; } else { t1 = $[4]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rename-source-variables-nested-object-method.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rename-source-variables-nested-object-method.expect.md index d0f3d5dcfe..e406f3a7d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rename-source-variables-nested-object-method.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/rename-source-variables-nested-object-method.expect.md @@ -43,7 +43,7 @@ const t0 = "module_t0"; const c_0 = "module_c_0"; function useFoo(props) { const $0 = _c(2); - const c_00 = $0[0] !== props.value; + const c_00 = $0[0] !== props; let t1; if (c_00) { const a = { @@ -61,7 +61,7 @@ function useFoo(props) { }; t1 = a.foo().bar(); - $0[0] = props.value; + $0[0] = props; $0[1] = t1; } else { t1 = $0[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect-nested-lambdas.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect-nested-lambdas.expect.md index c3e115fa0d..0cce42e97a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect-nested-lambdas.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect-nested-lambdas.expect.md @@ -35,7 +35,7 @@ function Component(props) { import { c as _c } from "react/compiler-runtime"; // @enableTransitivelyFreezeFunctionExpressions:false function Component(props) { - const $ = _c(9); + const $ = _c(7); const item = useMutable(props.itemId); const dispatch = useDispatch(); useFreeze(dispatch); @@ -51,7 +51,8 @@ function Component(props) { } const exit = t0; let t1; - if ($[2] !== exit || $[3] !== item.value) { + let t2; + if ($[2] !== exit || $[3] !== item) { t1 = () => { const cleanup = GlobalEventEmitter.addListener("onInput", () => { if (item.value) { @@ -60,30 +61,24 @@ function Component(props) { }); return () => cleanup.remove(); }; + t2 = [exit, item]; $[2] = exit; - $[3] = item.value; + $[3] = item; $[4] = t1; + $[5] = t2; } else { t1 = $[4]; - } - let t2; - if ($[5] !== exit || $[6] !== item) { - t2 = [exit, item]; - $[5] = exit; - $[6] = item; - $[7] = t2; - } else { - t2 = $[7]; + t2 = $[5]; } useEffect(t1, t2); maybeMutate(item); let t3; - if ($[8] === Symbol.for("react.memo_cache_sentinel")) { + if ($[6] === Symbol.for("react.memo_cache_sentinel")) { t3 =
; - $[8] = t3; + $[6] = t3; } else { - t3 = $[8]; + t3 = $[6]; } return t3; } diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 7c487869d3..62b8a7703f 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -450,6 +450,7 @@ const skipFilter = new Set([ 'invalid-jsx-lowercase-localvar', // bugs + 'inner-function/nullable-objects/bug-invalid-array-map-manual', 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', `bug-capturing-func-maybealias-captured-mutate`, 'bug-aliased-capture-aliased-mutate', From c2a196174763e0b4f16ed1c512ed4442b062395e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 18 Mar 2025 19:20:34 -0400 Subject: [PATCH 2/3] Minor Fixes to View Transition Fixture (#32664) Follow up to #32656. Remove touchAction from SwipeRecognizer. I was under the wrong impression that this was only the touch-action applied to this particular element, but that parents would still win but in fact this blocks the parent from scrolling in the other direction. By specifying a fixed direction it also blocked rage-swiping in the other direction early on. Disable pointer-events on view-transition so that the scroll can be hit. This means that touches hit below the items animating above. This allows swiping to happen again before momentum scroll has finished. Previously they were ignored. This only works as long as the SwipeRecognizer is itself not animating. This means you can now rage-swipe in both directions quickly. --- fixtures/view-transition/src/components/Chrome.css | 7 +++++++ fixtures/view-transition/src/components/SwipeRecognizer.js | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/fixtures/view-transition/src/components/Chrome.css b/fixtures/view-transition/src/components/Chrome.css index ade2f73952..969f1e2d32 100644 --- a/fixtures/view-transition/src/components/Chrome.css +++ b/fixtures/view-transition/src/components/Chrome.css @@ -7,3 +7,10 @@ body { padding: 0; font-family: sans-serif; } + +::view-transition-group(*), +::view-transition-image-pair(*), +::view-transition-old(*), +::view-transition-new(*) { + pointer-events: none; +} diff --git a/fixtures/view-transition/src/components/SwipeRecognizer.js b/fixtures/view-transition/src/components/SwipeRecognizer.js index 8e913dfb4e..0b16967f51 100644 --- a/fixtures/view-transition/src/components/SwipeRecognizer.js +++ b/fixtures/view-transition/src/components/SwipeRecognizer.js @@ -92,7 +92,6 @@ export default function SwipeRecognizer({ width: axis === 'x' ? '100%' : null, height: axis === 'y' ? '100%' : null, overflow: 'scroll hidden', - touchAction: 'pan-' + direction, // Disable overscroll on Safari which moves the sticky content. // Unfortunately, this also means that we disable chaining. We should only disable // it if the parent is not scrollable in this axis. From 5a40bab58c80f71bdb341bc41217dbef9660e3f2 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 19 Mar 2025 11:24:29 -0400 Subject: [PATCH 3/3] [ci] Properly format commit message Using the github variable for the commit message replaces the variable inline. If the commit message contains quotes or other characters that need to be escaped, this breaks the workflow. To fix this we instead output the message as an env variable. --- .github/workflows/runtime_commit_artifacts.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/runtime_commit_artifacts.yml b/.github/workflows/runtime_commit_artifacts.yml index 5de51b2664..450e7bb773 100644 --- a/.github/workflows/runtime_commit_artifacts.yml +++ b/.github/workflows/runtime_commit_artifacts.yml @@ -252,9 +252,9 @@ jobs: git config --global user.email "${{ format('{0}@users.noreply.github.com', github.triggering_actor) }}" git config --global user.name "${{ github.triggering_actor }}" - git commit -m "${{ github.event.workflow_run.head_commit.message || format('Manual build of {0}', github.event.workflow_run.head_sha || github.sha) }} - - DiffTrain build for [${{ github.event.workflow_run.head_sha || github.sha }}](https://github.com/facebook/react/commit/${{ github.event.workflow_run.head_sha || github.sha }})" || echo "No changes to commit" + git commit -m $COMMIT_MSG || echo "No changes to commit" + env: + COMMIT_MSG: "$(git show --no-patch --pretty=format:'%B%n%nDiffTrain build for [${{ github.event.workflow_run.head_sha || github.sha }}](https://github.com/facebook/react/commit/${{ github.event.workflow_run.head_sha || github.sha}})')" - name: Push changes to branch if: inputs.dry_run == false && (inputs.force == true || steps.check_should_commit.outputs.should_commit == 'true') run: git push @@ -419,9 +419,9 @@ jobs: git config --global user.email "${{ format('{0}@users.noreply.github.com', github.triggering_actor) }}" git config --global user.name "${{ github.triggering_actor }}" - git commit -m "${{ github.event.workflow_run.head_commit.message || format('Manual build of {0}', github.event.workflow_run.head_sha || github.sha) }} - - DiffTrain build for [${{ github.event.workflow_run.head_sha || github.sha }}](https://github.com/facebook/react/commit/${{ github.event.workflow_run.head_sha || github.sha }})" || echo "No changes to commit" + git commit -m $COMMIT_MSG || echo "No changes to commit" + env: + COMMIT_MSG: "$(git show --no-patch --pretty=format:'%B%n%nDiffTrain build for [${{ github.event.workflow_run.head_sha || github.sha }}](https://github.com/facebook/react/commit/${{ github.event.workflow_run.head_sha || github.sha}})')" - name: Push changes to branch if: inputs.dry_run == false && (inputs.force == true || steps.check_should_commit.outputs.should_commit == 'true') run: git push