From a44559b0a5f6fc01f9963d2f3b216a2dd7179fd4 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 26 Apr 2024 12:40:35 -0400 Subject: [PATCH] [hir] Re-implement mergeOverlappingReactiveScopes (+ bugfix) ghstack-source-id: 06d49edffe5ae3c31eb6ef642078752c056c617c Pull Request resolved: https://github.com/facebook/react-forget/pull/2852 --- .../src/Entrypoint/Pipeline.ts | 8 + .../HIR/MergeOverlappingReactiveScopesHIR.ts | 291 ++++++++++++++++++ .../src/HIR/index.ts | 1 + .../AlignReactiveScopesToBlockScopesHIR.ts | 131 -------- ...opes-begin-same-instr-valueblock.expect.md | 66 ++++ ...sted-scopes-begin-same-instr-valueblock.ts | 19 ++ ...valid-scope-merging-value-blocks.expect.md | 81 +++++ ...epro-invalid-scope-merging-value-blocks.ts | 32 ++ ...onditional-ternary-with-mutation.expect.md | 45 ++- ...ing-unconditional-ternary-with-mutation.js | 13 +- .../packages/snap/src/SproutTodoFilter.ts | 1 - 11 files changed, 541 insertions(+), 147 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/HIR/MergeOverlappingReactiveScopesHIR.ts create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.ts create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.ts diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts index cf5bfd9fd0..7fb6f037cb 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts @@ -18,6 +18,7 @@ import { assertValidMutableRanges, lower, mergeConsecutiveBlocks, + mergeOverlappingReactiveScopesHIR, pruneUnusedLabelsHIR, } from "../HIR"; import { @@ -252,6 +253,13 @@ function* runWithEnvironment( value: hir, }); + mergeOverlappingReactiveScopesHIR(hir); + yield log({ + kind: "hir", + name: "MergeOverlappingReactiveScopesHIR", + value: hir, + }); + assertValidBlockNesting(hir); } diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/MergeOverlappingReactiveScopesHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/MergeOverlappingReactiveScopesHIR.ts new file mode 100644 index 0000000000..6ce0c101fb --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/MergeOverlappingReactiveScopesHIR.ts @@ -0,0 +1,291 @@ +import { + HIRFunction, + InstructionId, + Place, + ReactiveScope, + makeInstructionId, +} from "."; +import { getPlaceScope } from "../ReactiveScopes/BuildReactiveBlocks"; +import { isMutable } from "../ReactiveScopes/InferReactiveScopeVariables"; +import DisjointSet from "../Utils/DisjointSet"; +import { getOrInsertDefault } from "../Utils/utils"; +import { + eachInstructionLValue, + eachInstructionOperand, + eachTerminalOperand, +} from "./visitors"; + +/** + * While previous passes ensure that reactive scopes span valid sets of program + * blocks, pairs of reactive scopes may still be inconsistent with respect to + * each other. + * + * (a) Reactive scopes ranges must form valid blocks in the resulting javascript + * program. Any two scopes must either be entirely disjoint or one scope must be + * nested within the other. + * ```js + * // Scopes 1:3 and 3:5 are valid because they contain no common instructions + * [1] ⌝ + * [2] ⌟ + * [3] ⌝ + * [4] ⌟ + * // Scopes 1:3 and 1:5 are valid because the former is nested within the other + * [1] ⌝ ⌝ + * [2] ⌟ | + * [3] | + * [4] ⌟ + * // Scopes 1:4 and 2:5 are invalid because we cannot produce if-else memo + * // blocks representing these scopes in the output program. + * [1] ⌝ + * [2] | ⌝ + * [3] ⌟ | + * [4] ⌟ + * ``` + * + * (b) A scope's own instructions may only mutate that scope. + * For each reactive scope, we currently produce exactly one if-block which + * spans the instruction range of the scope. In this simple example, instr [2] + * does not mutate any values but is included within scope @0. + * ```js + * // IR instructions + * [1] (writes to scope @0's values) + * [2] (does not mutate anything) + * [3] (writes to scope @0's values) + * + * // javascript output + * if (( scope @0's dependencies changed )) { + * [1] + * [2] + * [3] + * } + * ``` + * Nested scopes may be modeled as a tree in which child scopes are contained + * within parent scopes. This corresponds to nested if-else memo blocks in the + * output program). An instruction may only mutate its own "active" scope. + * ```js + * // Active scopes for a simple program + * scope @0 { + * [0] (active scope=@0) + * scope @1 { + * [1] (active scope=@1) + * [2] (active scope=@1) + * } + * [3] (active scope=@0) + * } + * [4] (no active scope) + * + * // In this example, scopes @0 and @1 must be merged because instr [2]'s + * // active scope is scope@1 but it mutates scope@0. + * scope @0, produces x { + * [0] x = [] + * scope @1, produces y { + * [1] y = [] + * [2] x.push(2) + * [3] y.push(3) + * } + * [3] x.push(1) + * } + * ``` + * + * As mentioned, these constraints arise entirely from the current design of + * compiler output. + * - instruction ordering is preserved (otherwise, disjoint ranges for scopes + * may be produced by reordering their mutating instructions) + * - exactly one if-else block per scope, which does not allow the composition + * of a reactive scope from disconnected instruction ranges. + */ + +export function mergeOverlappingReactiveScopesHIR(fn: HIRFunction): void { + /** + * Collect all scopes eagerly because some scopes begin before the first + * instruction that references them (due to alignReactiveScopesToBlocks) + */ + const scopesInfo = collectScopeInfo(fn); + + /** + * Iterate through scopes and instructions to find which should be merged + */ + const joinedScopes = getOverlappingReactiveScopes(fn, scopesInfo); + + /** + * Merge scopes and rewrite all references + */ + joinedScopes.forEach((scope, groupScope) => { + if (scope !== groupScope) { + groupScope.range.start = makeInstructionId( + Math.min(groupScope.range.start, scope.range.start) + ); + groupScope.range.end = makeInstructionId( + Math.max(groupScope.range.end, scope.range.end) + ); + } + }); + for (const [place, originalScope] of scopesInfo.placeScopes) { + const nextScope = joinedScopes.find(originalScope); + if (nextScope !== null && nextScope !== originalScope) { + place.identifier.scope = nextScope; + } + } +} + +type ScopeInfo = { + scopeStarts: Array<{ id: InstructionId; scopes: Set }>; + scopeEnds: Array<{ id: InstructionId; scopes: Set }>; + placeScopes: Map; +}; + +type TraversalState = { + joined: DisjointSet; + activeScopes: Array; +}; + +function collectScopeInfo(fn: HIRFunction): ScopeInfo { + const scopeStarts: Map> = new Map(); + const scopeEnds: Map> = new Map(); + const placeScopes: Map = new Map(); + + function collectPlaceScope(place: Place): void { + const scope = place.identifier.scope; + if (scope != null) { + placeScopes.set(place, scope); + if (scope.range.start !== scope.range.end) { + getOrInsertDefault(scopeStarts, scope.range.start, new Set()).add( + scope + ); + getOrInsertDefault(scopeEnds, scope.range.end, new Set()).add(scope); + } + } + } + + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + for (const operand of eachInstructionLValue(instr)) { + collectPlaceScope(operand); + } + for (const operand of eachInstructionOperand(instr)) { + collectPlaceScope(operand); + } + } + for (const operand of eachTerminalOperand(block.terminal)) { + collectPlaceScope(operand); + } + } + + return { + scopeStarts: [...scopeStarts.entries()] + .map(([id, scopes]) => ({ id, scopes })) + .sort((a, b) => b.id - a.id), + scopeEnds: [...scopeEnds.entries()] + .map(([id, scopes]) => ({ id, scopes })) + .sort((a, b) => b.id - a.id), + placeScopes, + }; +} + +function visitInstructionId( + id: InstructionId, + { scopeEnds, scopeStarts }: ScopeInfo, + { activeScopes, joined }: TraversalState +): void { + /** + * Handle all scopes that end at this instruction. + */ + const scopeEndTop = scopeEnds.at(-1); + if (scopeEndTop != null && scopeEndTop.id <= id) { + scopeEnds.pop(); + + /** + * Match scopes that end at this instruction with our stack of active + * scopes (from traversal state). We need to sort these in descending + * order of start IDs because the scopes stack is ordered as such + */ + const scopesSortedStartDescending = [...scopeEndTop.scopes].sort( + (a, b) => b.range.start - a.range.start + ); + for (const scope of scopesSortedStartDescending) { + const idx = activeScopes.indexOf(scope); + if (idx !== -1) { + /** + * Detect and merge all overlapping scopes. `activeScopes` is ordered + * by scope start, so every active scope between a completed scope s + * and the top of the stack (1) started later than s and (2) completes after s. + */ + if (idx !== activeScopes.length - 1) { + joined.union([scope, ...activeScopes.slice(idx + 1)]); + } + activeScopes.splice(idx, 1); + } + } + } + + /** + * Handle all scopes that begin at this instruction by adding them + * to the scopes stack + */ + const scopeStartTop = scopeStarts.at(-1); + if (scopeStartTop != null && scopeStartTop.id <= id) { + scopeStarts.pop(); + + const scopesSortedEndDescending = [...scopeStartTop.scopes].sort( + (a, b) => b.range.end - a.range.end + ); + activeScopes.push(...scopesSortedEndDescending); + /** + * Merge all identical scopes (ones with the same start and end), + * as they end up with the same reactive block + */ + for (let i = 1; i < scopesSortedEndDescending.length; i++) { + const prev = scopesSortedEndDescending[i - 1]; + const curr = scopesSortedEndDescending[i]; + if (prev.range.end === curr.range.end) { + joined.union([prev, curr]); + } + } + } +} + +function visitPlace( + id: InstructionId, + place: Place, + { activeScopes, joined }: TraversalState +): void { + /** + * If an instruction mutates an outer scope, flatten all scopes from the top + * of the stack to the mutated outer scope. + */ + const placeScope = getPlaceScope(id, place); + if (placeScope != null && isMutable({ id } as any, place)) { + const placeScopeIdx = activeScopes.indexOf(placeScope); + if (placeScopeIdx !== -1 && placeScopeIdx !== activeScopes.length - 1) { + joined.union([placeScope, ...activeScopes.slice(placeScopeIdx + 1)]); + } + } +} + +function getOverlappingReactiveScopes( + fn: HIRFunction, + context: ScopeInfo +): DisjointSet { + const state: TraversalState = { + joined: new DisjointSet(), + activeScopes: [], + }; + + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + visitInstructionId(instr.id, context, state); + for (const place of eachInstructionOperand(instr)) { + visitPlace(instr.id, place, state); + } + for (const place of eachInstructionLValue(instr)) { + visitPlace(instr.id, place, state); + } + } + visitInstructionId(block.terminal.id, context, state); + for (const place of eachTerminalOperand(block.terminal)) { + visitPlace(block.terminal.id, place, state); + } + } + + return state.joined; +} diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/index.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/index.ts index 6a5ee85bb9..5fea244ca2 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/index.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/index.ts @@ -27,5 +27,6 @@ export { reversePostorderBlocks, } from "./HIRBuilder"; export { mergeConsecutiveBlocks } from "./MergeConsecutiveBlocks"; +export { mergeOverlappingReactiveScopesHIR } from "./MergeOverlappingReactiveScopesHIR"; export { printFunction, printHIR } from "./PrintHIR"; export { pruneUnusedLabelsHIR } from "./PruneUnusedLabelsHIR"; diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts index cfcdaa31ed..3e451c23c7 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts @@ -22,8 +22,6 @@ import { mapTerminalSuccessors, terminalFallthrough, } from "../HIR/visitors"; -import DisjointSet from "../Utils/DisjointSet"; -import { retainWhere } from "../Utils/utils"; import { getPlaceScope } from "./BuildReactiveBlocks"; /* @@ -237,46 +235,6 @@ export function alignReactiveScopesToBlockScopesHIR(fn: HIRFunction): void { } // console.log(_debug(rootNode)); - - const joinedScopes: DisjointSet = - mergeOverlappingScopes(rootNode); - - /** - * Join scopes that begin and end at the same instructions - */ - { - const allScopes = [...new Set(placeScopes.values())].sort( - (a, b) => a.range.start - b.range.start - ); - for (let i = 1; i < allScopes.length; i++) { - const prev = allScopes[i - 1]; - const curr = allScopes[i]; - if ( - prev.range.start === curr.range.start && - prev.range.end === curr.range.end - ) { - joinedScopes.union([prev, curr]); - } - } - } - - joinedScopes.forEach((scope, groupScope) => { - if (scope !== groupScope) { - groupScope.range.start = makeInstructionId( - Math.min(groupScope.range.start, scope.range.start) - ); - groupScope.range.end = makeInstructionId( - Math.max(groupScope.range.end, scope.range.end) - ); - } - }); - - for (const [place, originalScope] of placeScopes) { - const nextScope = joinedScopes.find(originalScope); - if (nextScope !== null && nextScope !== originalScope) { - place.identifier.scope = nextScope; - } - } } type BlockNode = { @@ -318,92 +276,3 @@ function _printNode( out.push(`${prefix}]`); } } - -type ScopeItem = { - scope: ReactiveScope; - shadowedBy: ReactiveScope | null; -}; -class BlockItem { - seen: Set = new Set(); - scopes: Array = []; -} - -function mergeOverlappingScopes(root: BlockNode): DisjointSet { - const seen = new Set(); - const joined = new DisjointSet(); - - function visit(node: BlockNode, stack: Array): void { - const currentBlock = stack.at(-1)!; - child: for (const child of node.children) { - retainWhere(currentBlock.scopes, (item) => { - if (item.scope.range.end > child.id) { - return true; - } else { - currentBlock.seen.delete(item.scope); - return false; - } - }); - if (child.kind === "node") { - visit(child, [...stack, new BlockItem()]); - } else { - const scope = child.scope; - if (!seen.has(scope)) { - seen.add(scope); - currentBlock.seen.add(scope); - currentBlock.scopes.push({ shadowedBy: null, scope }); - continue; - } - - let index = stack.length - 1; - let nextBlock = currentBlock; - while (!nextBlock.seen.has(scope)) { - joined.union([scope, ...nextBlock.scopes.map((s) => s.scope)]); - index--; - if (index < 0) { - currentBlock.seen.add(scope); - currentBlock.scopes.push({ shadowedBy: null, scope }); - continue child; - } - nextBlock = stack[index]!; - } - - // Handle interleaving within a given block scope - let found = false; - for (let i = 0; i < nextBlock.scopes.length; i++) { - const current = nextBlock.scopes[i]!; - if (current.scope.id === scope.id) { - found = true; - if (current.shadowedBy !== null) { - joined.union([current.shadowedBy, current.scope]); - } - } else if (found && current.shadowedBy === null) { - // `scope` is shadowing `current` and may interleave - current.shadowedBy = scope; - if (current.scope.range.end > scope.range.end) { - /* - * Current is shadowed by `scope`, and we know that `current` will mutate - * again (per its range), so the scopes are already known to interleave. - * - * Eagerly extend the ranges of the scopes so that we don't prematurely end - * a scope relative to its eventual post-merge mutable range - */ - const end = makeInstructionId( - Math.max(current.scope.range.end, scope.range.end) - ); - current.scope.range.end = end; - scope.range.end = end; - joined.union([current.scope, scope]); - } - } - } - if (!currentBlock.seen.has(scope)) { - currentBlock.seen.add(scope); - currentBlock.scopes.push({ shadowedBy: null, scope }); - } - } - } - } - - visit(root, [new BlockItem()]); - return joined; -} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.expect.md new file mode 100644 index 0000000000..bed4eb1509 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.expect.md @@ -0,0 +1,66 @@ + +## Input + +```javascript +import { identity, mutate } from "shared-runtime"; + +function Foo({ cond }) { + const x = identity(identity(cond)) ? { a: 2 } : { b: 2 }; + + mutate(x); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: false }], + sequentialRenders: [ + { cond: false }, + { cond: false }, + { cond: true }, + { cond: true }, + ], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +import { identity, mutate } from "shared-runtime"; + +function Foo(t0) { + const $ = useMemoCache(2); + const { cond } = t0; + let x; + if ($[0] !== cond) { + x = identity(identity(cond)) ? { a: 2 } : { b: 2 }; + + mutate(x); + $[0] = cond; + $[1] = x; + } else { + x = $[1]; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: false }], + sequentialRenders: [ + { cond: false }, + { cond: false }, + { cond: true }, + { cond: true }, + ], +}; + +``` + +### Eval output +(kind: ok) {"b":2,"wat0":"joe"} +{"b":2,"wat0":"joe"} +{"a":2,"wat0":"joe"} +{"a":2,"wat0":"joe"} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.ts new file mode 100644 index 0000000000..11aabbc9f5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-scopes-begin-same-instr-valueblock.ts @@ -0,0 +1,19 @@ +import { identity, mutate } from "shared-runtime"; + +function Foo({ cond }) { + const x = identity(identity(cond)) ? { a: 2 } : { b: 2 }; + + mutate(x); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: false }], + sequentialRenders: [ + { cond: false }, + { cond: false }, + { cond: true }, + { cond: true }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.expect.md new file mode 100644 index 0000000000..c58986bbc8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.expect.md @@ -0,0 +1,81 @@ + +## Input + +```javascript +import { + CONST_TRUE, + identity, + makeObject_Primitives, + mutateAndReturn, + useHook, +} from "shared-runtime"; + +/** + * value and `mutateAndReturn(value)` should end up in the same reactive scope. + * (1) `value = makeObject` and `(temporary) = mutateAndReturn(value)` should be assigned + * the same scope id (on their identifiers) + * (2) alignScopesToBlockScopes should expand the scopes of both `(temporary) = identity(1)` + * and `(temporary) = mutateAndReturn(value)` to the outermost value block boundaries + * (3) mergeOverlappingScopes should merge the scopes of the above two instructions + */ +function Component({}) { + const value = makeObject_Primitives(); + useHook(); + const mutatedValue = + identity(1) && CONST_TRUE ? mutateAndReturn(value) : null; + const result = []; + useHook(); + result.push(value, mutatedValue); + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}, {}], +}; + +``` + +## Code + +```javascript +import { + CONST_TRUE, + identity, + makeObject_Primitives, + mutateAndReturn, + useHook, +} from "shared-runtime"; + +/** + * value and `mutateAndReturn(value)` should end up in the same reactive scope. + * (1) `value = makeObject` and `(temporary) = mutateAndReturn(value)` should be assigned + * the same scope id (on their identifiers) + * (2) alignScopesToBlockScopes should expand the scopes of both `(temporary) = identity(1)` + * and `(temporary) = mutateAndReturn(value)` to the outermost value block boundaries + * (3) mergeOverlappingScopes should merge the scopes of the above two instructions + */ +function Component(t0) { + const value = makeObject_Primitives(); + useHook(); + const mutatedValue = + identity(1) && CONST_TRUE ? mutateAndReturn(value) : null; + const result = []; + useHook(); + result.push(value, mutatedValue); + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}, {}], +}; + +``` + +### Eval output +(kind: ok) [{"a":0,"b":"value1","c":true,"wat0":"joe"},"[[ cyclic ref *1 ]]"] +[{"a":0,"b":"value1","c":true,"wat0":"joe"},"[[ cyclic ref *1 ]]"] +[{"a":0,"b":"value1","c":true,"wat0":"joe"},"[[ cyclic ref *1 ]]"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.ts new file mode 100644 index 0000000000..dec95f512c --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-invalid-scope-merging-value-blocks.ts @@ -0,0 +1,32 @@ +import { + CONST_TRUE, + identity, + makeObject_Primitives, + mutateAndReturn, + useHook, +} from "shared-runtime"; + +/** + * value and `mutateAndReturn(value)` should end up in the same reactive scope. + * (1) `value = makeObject` and `(temporary) = mutateAndReturn(value)` should be assigned + * the same scope id (on their identifiers) + * (2) alignScopesToBlockScopes should expand the scopes of both `(temporary) = identity(1)` + * and `(temporary) = mutateAndReturn(value)` to the outermost value block boundaries + * (3) mergeOverlappingScopes should merge the scopes of the above two instructions + */ +function Component({}) { + const value = makeObject_Primitives(); + useHook(); + const mutatedValue = + identity(1) && CONST_TRUE ? mutateAndReturn(value) : null; + const result = []; + useHook(); + result.push(value, mutatedValue); + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}, {}], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md index 1b76a89a8b..3f1454b4d0 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md @@ -2,39 +2,42 @@ ## Input ```javascript +import { arrayPush } from "shared-runtime"; function foo(props) { let x = []; x.push(props.bar); props.cond ? ((x = {}), (x = []), x.push(props.foo)) : ((x = []), (x = []), x.push(props.bar)); - mut(x); + arrayPush(x, 4); return x; } +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ cond: false, foo: 2, bar: 55 }], + sequentialRenders: [ + { cond: false, foo: 2, bar: 55 }, + { cond: false, foo: 3, bar: 55 }, + { cond: true, foo: 3, bar: 55 }, + ], +}; + ``` ## Code ```javascript import { unstable_useMemoCache as useMemoCache } from "react"; +import { arrayPush } from "shared-runtime"; function foo(props) { - const $ = useMemoCache(5); + const $ = useMemoCache(2); let x; if ($[0] !== props) { x = []; x.push(props.bar); - if ($[2] !== props || $[3] !== x) { - props.cond - ? ((x = []), x.push(props.foo)) - : ((x = []), x.push(props.bar)); - mut(x); - $[2] = props; - $[3] = x; - $[4] = x; - } else { - x = $[4]; - } + props.cond ? ((x = []), x.push(props.foo)) : ((x = []), x.push(props.bar)); + arrayPush(x, 4); $[0] = props; $[1] = x; } else { @@ -43,5 +46,19 @@ function foo(props) { return x; } +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ cond: false, foo: 2, bar: 55 }], + sequentialRenders: [ + { cond: false, foo: 2, bar: 55 }, + { cond: false, foo: 3, bar: 55 }, + { cond: true, foo: 3, bar: 55 }, + ], +}; + ``` - \ No newline at end of file + +### Eval output +(kind: ok) [55,4] +[55,4] +[3,4] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.js index 5877120405..56530eb640 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.js @@ -1,9 +1,20 @@ +import { arrayPush } from "shared-runtime"; function foo(props) { let x = []; x.push(props.bar); props.cond ? ((x = {}), (x = []), x.push(props.foo)) : ((x = []), (x = []), x.push(props.bar)); - mut(x); + arrayPush(x, 4); return x; } + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [{ cond: false, foo: 2, bar: 55 }], + sequentialRenders: [ + { cond: false, foo: 2, bar: 55 }, + { cond: false, foo: 3, bar: 55 }, + { cond: true, foo: 3, bar: 55 }, + ], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 7a9d8c7781..70780c2064 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -334,7 +334,6 @@ const skipFilter = new Set([ "ssa-property-alias-mutate-inside-if", "ssa-renaming-ternary-destruction-with-mutation", "ssa-renaming-ternary-with-mutation", - "ssa-renaming-unconditional-ternary-with-mutation", "ssa-renaming-unconditional-with-mutation", "ssa-renaming-via-destructuring-with-mutation", "ssa-renaming-with-mutation",