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 c47a41145157..eb862de2221c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -167,9 +167,21 @@ type CollectHoistablePropertyLoadsContext = { 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) + * not be called, e.g. the 0th argument of Array.map). + * + * Split into two categories: + * - syncInvoked: Functions called synchronously during render (direct calls). + * Property accesses from these can safely prove non-nullness at the outer scope. + * - deferredInvoked: Functions invoked later (JSX event handlers, hook callbacks, + * returned functions). Property accesses from these should NOT prove non-nullness + * at the outer scope because they don't execute during render. */ - assumedInvokedFns: ReadonlySet; + assumedInvokedFns: AssumedInvokedFunctions; +}; + +type AssumedInvokedFunctions = { + syncInvoked: ReadonlySet; + deferredInvoked: ReadonlySet; }; function collectHoistablePropertyLoadsImpl( fn: HIRFunction, @@ -209,6 +221,13 @@ export function keyByScopeId( export type BlockInfo = { block: BasicBlock; assumedNonNullObjects: ReadonlySet; + /** + * Property paths from deferred closures (JSX event handlers, hook callbacks, + * returned functions). These cannot prove non-nullness during render, but are + * tracked separately so that downstream cache key generation can use optional + * access (e.g. `user?.name`) instead of truncating to just the base object. + */ + deferredNonNullObjects: ReadonlySet; }; /** @@ -406,12 +425,14 @@ function collectNonNullsInBlocks( { block: BasicBlock; assumedNonNullObjects: Set; + deferredNonNullObjects: Set; } >(); for (const [_, block] of fn.body.blocks) { const assumedNonNullObjects = new Set( knownNonNullIdentifiers, ); + const deferredNonNullObjects = new Set(); const maybeOptionalChain = context.hoistableFromOptionals.get(block.id); if (maybeOptionalChain != null) { @@ -429,7 +450,11 @@ function collectNonNullsInBlocks( } if (instr.value.kind === 'FunctionExpression') { const innerFn = instr.value.loweredFunc; - if (context.assumedInvokedFns.has(innerFn)) { + const isSyncInvoked = + context.assumedInvokedFns.syncInvoked.has(innerFn); + const isDeferredInvoked = + context.assumedInvokedFns.deferredInvoked.has(innerFn); + if (isSyncInvoked || isDeferredInvoked) { const innerHoistableMap = collectHoistablePropertyLoadsImpl( innerFn.func, { @@ -445,11 +470,27 @@ function collectNonNullsInBlocks( ), }, ); + /** + * Only propagate assumedNonNullObjects from synchronously-invoked + * inner functions (direct calls during render). Deferred functions + * (JSX event handlers, hook callbacks, returned functions) don't + * execute during render, so their property accesses cannot prove + * non-nullness at render time. + * + * See: https://github.com/facebook/react/issues/34752 + * https://github.com/facebook/react/issues/35762 + */ const innerHoistables = assertNonNull( innerHoistableMap.get(innerFn.func.body.entry), ); - for (const entry of innerHoistables.assumedNonNullObjects) { - assumedNonNullObjects.add(entry); + if (isSyncInvoked) { + for (const entry of innerHoistables.assumedNonNullObjects) { + assumedNonNullObjects.add(entry); + } + } else if (isDeferredInvoked) { + for (const entry of innerHoistables.assumedNonNullObjects) { + deferredNonNullObjects.add(entry); + } } } } else if ( @@ -485,6 +526,7 @@ function collectNonNullsInBlocks( nodes.set(block.id, { block, assumedNonNullObjects, + deferredNonNullObjects, }); } return nodes; @@ -571,10 +613,11 @@ function propagateNonNull( * it's not safe to assume they can be filtered out (e.g. not included in * the intersection) */ + const doneNeighbors = Array.from(neighbors).filter( + n => traversalState.get(n) === 'done', + ); const neighborAccesses = Set_intersect( - Array.from(neighbors) - .filter(n => traversalState.get(n) === 'done') - .map(n => assertNonNull(nodes.get(n)).assumedNonNullObjects), + doneNeighbors.map(n => assertNonNull(nodes.get(n)).assumedNonNullObjects), ); const prevObjects = assertNonNull(nodes.get(nodeId)).assumedNonNullObjects; @@ -582,6 +625,18 @@ function propagateNonNull( reduceMaybeOptionalChains(mergedObjects, registry); assertNonNull(nodes.get(nodeId)).assumedNonNullObjects = mergedObjects; + + // Also propagate deferred non-null objects + const neighborDeferred = Set_intersect( + doneNeighbors.map( + n => assertNonNull(nodes.get(n)).deferredNonNullObjects, + ), + ); + const prevDeferred = + assertNonNull(nodes.get(nodeId)).deferredNonNullObjects; + const mergedDeferred = Set_union(prevDeferred, neighborDeferred); + assertNonNull(nodes.get(nodeId)).deferredNonNullObjects = mergedDeferred; + traversalState.set(nodeId, 'done'); /** * Note that it's not sufficient to compare set sizes since @@ -589,7 +644,9 @@ function propagateNonNull( * unconditional loads. This could in turn change `assumedNonNullObjects` of * downstream blocks and backedges. */ - changed ||= !Set_equal(prevObjects, mergedObjects); + changed ||= + !Set_equal(prevObjects, mergedObjects) || + !Set_equal(prevDeferred, mergedDeferred); return changed; } const traversalState = new Map(); @@ -700,8 +757,9 @@ function getAssumedInvokedFunctions( IdentifierId, {fn: LoweredFunction; mayInvoke: Set} > = new Map(), -): ReadonlySet { - const hoistableFunctions = new Set(); +): AssumedInvokedFunctions { + const syncInvoked = new Set(); + const deferredInvoked = new Set(); /** * Step 1: Conservatively collect identifier to function expression mappings */ @@ -734,6 +792,13 @@ function getAssumedInvokedFunctions( * 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: () => {...}})`). + * + * Functions are classified into two categories: + * - syncInvoked: Called synchronously during render (direct function calls). + * Their property accesses can safely prove non-nullness at the call site. + * - deferredInvoked: Called later, not during render (JSX event handlers, + * hook callbacks, returned functions). Their property accesses should NOT + * be used to prove non-nullness during render. */ for (const block of fn.body.blocks.values()) { for (const {lvalue, value} of block.instructions) { @@ -742,24 +807,29 @@ function getAssumedInvokedFunctions( const maybeHook = getHookKind(fn.env, callee.identifier); const maybeLoweredFunc = temporaries.get(callee.identifier.id); if (maybeLoweredFunc != null) { - // Direct calls - hoistableFunctions.add(maybeLoweredFunc.fn); + // Direct calls execute synchronously during render + syncInvoked.add(maybeLoweredFunc.fn); } else if (maybeHook != null) { /** - * Assume arguments to all hooks are safe to invoke + * Hook arguments are classified as deferred — they may or may not + * be invoked during render. Note: useMemo/useCallback are already + * lowered to StartMemoize/FinishMemoize before this pass runs + * (dropManualMemoization is always enabled), so they won't appear + * as hook CallExpressions here. */ for (const arg of value.args) { if (arg.kind === 'Identifier') { const maybeLoweredFunc = temporaries.get(arg.identifier.id); if (maybeLoweredFunc != null) { - hoistableFunctions.add(maybeLoweredFunc.fn); + deferredInvoked.add(maybeLoweredFunc.fn); } } } } } else if (value.kind === 'JsxExpression') { /** - * Assume JSX attributes and children are safe to invoke + * JSX attributes and children are deferred — event handlers like + * onClick don't execute during render. */ for (const attr of value.props) { if (attr.kind === 'JsxSpreadAttribute') { @@ -767,13 +837,13 @@ function getAssumedInvokedFunctions( } const maybeLoweredFunc = temporaries.get(attr.place.identifier.id); if (maybeLoweredFunc != null) { - hoistableFunctions.add(maybeLoweredFunc.fn); + deferredInvoked.add(maybeLoweredFunc.fn); } } for (const child of value.children ?? []) { const maybeLoweredFunc = temporaries.get(child.identifier.id); if (maybeLoweredFunc != null) { - hoistableFunctions.add(maybeLoweredFunc.fn); + deferredInvoked.add(maybeLoweredFunc.fn); } } } else if (value.kind === 'FunctionExpression') { @@ -792,7 +862,10 @@ function getAssumedInvokedFunctions( ); const maybeLoweredFunc = temporaries.get(lvalue.identifier.id); if (maybeLoweredFunc != null) { - for (const called of lambdasCalled) { + for (const called of lambdasCalled.syncInvoked) { + maybeLoweredFunc.mayInvoke.add(called); + } + for (const called of lambdasCalled.deferredInvoked) { maybeLoweredFunc.mayInvoke.add(called); } } @@ -800,23 +873,39 @@ function getAssumedInvokedFunctions( } if (block.terminal.kind === 'return') { /** - * Assume directly returned functions are safe to call + * Returned functions are deferred — they execute in the caller's + * context, not during this render. */ const maybeLoweredFunc = temporaries.get( block.terminal.value.identifier.id, ); if (maybeLoweredFunc != null) { - hoistableFunctions.add(maybeLoweredFunc.fn); + deferredInvoked.add(maybeLoweredFunc.fn); } } } + /** + * Step 3: Transitive closure. If a function is assumed invoked, then all + * functions it may invoke are also assumed invoked. The category (sync vs + * deferred) propagates from the parent: if a sync function calls another + * function, that function is also sync. If a deferred function calls + * another function, that function is also deferred. If a function appears + * in both sets, sync wins (it IS called synchronously via at least one path). + */ for (const [_, {fn, mayInvoke}] of temporaries) { - if (hoistableFunctions.has(fn)) { + if (syncInvoked.has(fn)) { for (const called of mayInvoke) { - hoistableFunctions.add(called); + syncInvoked.add(called); + } + } + if (deferredInvoked.has(fn)) { + for (const called of mayInvoke) { + if (!syncInvoked.has(called)) { + deferredInvoked.add(called); + } } } } - return hoistableFunctions; + return {syncInvoked, deferredInvoked}; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts index 2850e73ca5a0..b56a460c530c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -28,6 +28,16 @@ export class ReactiveScopeDependencyTreeHIR { */ #hoistableObjects: Map = new Map(); + /** + * Paths from deferred closures (event handlers, hook callbacks, returned + * functions). These cannot prove non-nullness at render time, but can be + * used to preserve fine-grained cache keys via optional access. When a + * dependency path would normally be truncated, we check this tree as a + * fallback — if the path exists here, we emit optional access (e.g. + * `user?.name`) instead of truncating to just `user`. + */ + #deferredObjects: Map = + new Map(); #deps: Map = new Map(); /** @@ -35,13 +45,34 @@ export class ReactiveScopeDependencyTreeHIR { * PropertyLoads. Note that we expect these to not contain duplicates (e.g. * both `a?.b` and `a.b`) only because CollectHoistablePropertyLoads merges * duplicates when traversing the CFG. + * @param deferredObjects a set of property paths from deferred closures, + * used as a fallback for optional cache key generation. */ - constructor(hoistableObjects: Iterable) { - for (const {path, identifier, reactive, loc} of hoistableObjects) { + constructor( + hoistableObjects: Iterable, + deferredObjects?: Iterable, + ) { + ReactiveScopeDependencyTreeHIR.#buildTree( + hoistableObjects, + this.#hoistableObjects, + ); + if (deferredObjects != null) { + ReactiveScopeDependencyTreeHIR.#buildTree( + deferredObjects, + this.#deferredObjects, + ); + } + } + + static #buildTree( + objects: Iterable, + tree: Map, + ): void { + for (const {path, identifier, reactive, loc} of objects) { let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, reactive, - this.#hoistableObjects, + tree, path.length > 0 && path[0].optional ? 'Optional' : 'NonNull', loc, ); @@ -122,10 +153,18 @@ export class ReactiveScopeDependencyTreeHIR { */ let hoistableCursor: HoistableNode | undefined = this.#hoistableObjects.get(identifier); + /** + * deferredCursor tracks the same path in the deferred objects tree. + * When the main hoistable tree doesn't have a path (would truncate), + * we check the deferred tree as a fallback to emit optional access. + */ + let deferredCursor: HoistableNode | undefined = + this.#deferredObjects.get(identifier); // All properties read 'on the way' to a dependency are marked as 'access' for (const entry of path) { let nextHoistableCursor: HoistableNode | undefined; + let nextDeferredCursor: HoistableNode | undefined; let nextDepCursor: DependencyNode; if (entry.optional) { /** @@ -136,6 +175,9 @@ export class ReactiveScopeDependencyTreeHIR { if (hoistableCursor != null) { nextHoistableCursor = hoistableCursor?.properties.get(entry.property); } + if (deferredCursor != null) { + nextDeferredCursor = deferredCursor.properties.get(entry.property); + } let accessType; if ( @@ -166,20 +208,40 @@ export class ReactiveScopeDependencyTreeHIR { hoistableCursor.accessType === 'NonNull' ) { nextHoistableCursor = hoistableCursor.properties.get(entry.property); + if (deferredCursor != null) { + nextDeferredCursor = deferredCursor.properties.get(entry.property); + } nextDepCursor = makeOrMergeProperty( depCursor, entry.property, PropertyAccessType.UnconditionalAccess, entry.loc, ); + } else if (deferredCursor != null) { + /** + * The main hoistable tree would truncate here, but the deferred + * tree has this path — emit optional access to preserve fine-grained + * cache keys. For example, `user.name` inside an onClick handler + * becomes `user?.name` in the cache key instead of truncating to + * just `user`. + */ + nextDeferredCursor = deferredCursor.properties.get(entry.property); + nextDepCursor = makeOrMergeProperty( + depCursor, + entry.property, + PropertyAccessType.OptionalAccess, + entry.loc, + ); } else { /** - * Break to truncate the dependency on its first non-optional entry that PropertyLoads are not hoistable from + * Break to truncate the dependency on its first non-optional entry + * that PropertyLoads are not hoistable from */ break; } depCursor = nextDepCursor; hoistableCursor = nextHoistableCursor; + deferredCursor = nextDeferredCursor; } // mark the final node as a dependency depCursor.accessType = merge( diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 19b4fae30fda..9f5ae400fcab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -94,6 +94,7 @@ export function propagateScopeDependenciesHIR(fn: HIRFunction): void { */ const tree = new ReactiveScopeDependencyTreeHIR( [...hoistables.assumedNonNullObjects].map(o => o.fullPath), + [...hoistables.deferredNonNullObjects].map(o => o.fullPath), ); for (const dep of deps) { tree.addDependency({...dep}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-assert-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-assert-function.expect.md new file mode 100644 index 000000000000..84107ecad0d8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-assert-function.expect.md @@ -0,0 +1,90 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: TypeScript assertion function pattern. `planPeriod.id` after the + * assertion is used as a cache key, crashing when planPeriod is null. + * The compiler doesn't understand that the assertion narrows the type. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function assertIsNotEmpty( + value: TValue | null | undefined +): asserts value is TValue { + if (value == null) throw new Error('assertion failure'); +} + +function Component({ + planPeriod, +}: { + planPeriod: {id: string} | null; +}) { + const callback = () => { + assertIsNotEmpty(planPeriod?.id); + console.log(planPeriod.id); + }; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{planPeriod: {id: 'p1'}}], + sequentialRenders: [{planPeriod: {id: 'p1'}}, {planPeriod: {id: 'p2'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: TypeScript assertion function pattern. `planPeriod.id` after the + * assertion is used as a cache key, crashing when planPeriod is null. + * The compiler doesn't understand that the assertion narrows the type. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function assertIsNotEmpty(value) { + if (value == null) { + throw new Error("assertion failure"); + } +} + +function Component(t0) { + const $ = _c(2); + const { planPeriod } = t0; + let t1; + if ($[0] !== planPeriod?.id) { + const callback = () => { + assertIsNotEmpty(planPeriod?.id); + console.log(planPeriod.id); + }; + t1 = ; + $[0] = planPeriod?.id; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ planPeriod: { id: "p1" } }], + sequentialRenders: [ + { planPeriod: { id: "p1" } }, + { planPeriod: { id: "p2" } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]"}
+
{"onClick":"[[ function params=0 ]]"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-assert-function.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-assert-function.tsx new file mode 100644 index 000000000000..821546e59657 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-assert-function.tsx @@ -0,0 +1,32 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: TypeScript assertion function pattern. `planPeriod.id` after the + * assertion is used as a cache key, crashing when planPeriod is null. + * The compiler doesn't understand that the assertion narrows the type. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function assertIsNotEmpty( + value: TValue | null | undefined +): asserts value is TValue { + if (value == null) throw new Error('assertion failure'); +} + +function Component({ + planPeriod, +}: { + planPeriod: {id: string} | null; +}) { + const callback = () => { + assertIsNotEmpty(planPeriod?.id); + console.log(planPeriod.id); + }; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{planPeriod: {id: 'p1'}}], + sequentialRenders: [{planPeriod: {id: 'p1'}}, {planPeriod: {id: 'p2'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-conditional-render-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-conditional-render-access.expect.md new file mode 100644 index 000000000000..cc380ec15d80 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-conditional-render-access.expect.md @@ -0,0 +1,90 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Optional chain `user?.name` in render + unconditional `user.email` + * in closure. The closure's `user.email` makes the compiler think `user` is + * non-null, converting the render's `user?.name` to `user.name` in cache keys. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component({ + user, +}: { + user: {name: string; email: string} | null; +}) { + const sendEmail = () => { + console.log(user.email); + }; + return {user?.name}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice', email: 'alice@example.com'}}], + sequentialRenders: [ + {user: {name: 'Alice', email: 'alice@example.com'}}, + {user: {name: 'Bob', email: 'bob@example.com'}}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: Optional chain `user?.name` in render + unconditional `user.email` + * in closure. The closure's `user.email` makes the compiler think `user` is + * non-null, converting the render's `user?.name` to `user.name` in cache keys. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component(t0) { + const $ = _c(5); + const { user } = t0; + let t1; + if ($[0] !== user?.email) { + t1 = () => { + console.log(user.email); + }; + $[0] = user?.email; + $[1] = t1; + } else { + t1 = $[1]; + } + const sendEmail = t1; + + const t2 = user?.name; + let t3; + if ($[2] !== sendEmail || $[3] !== t2) { + t3 = {t2}; + $[2] = sendEmail; + $[3] = t2; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ user: { name: "Alice", email: "alice@example.com" } }], + sequentialRenders: [ + { user: { name: "Alice", email: "alice@example.com" } }, + { user: { name: "Bob", email: "bob@example.com" } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]","children":"Alice"}
+
{"onClick":"[[ function params=0 ]]","children":"Bob"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-conditional-render-access.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-conditional-render-access.tsx new file mode 100644 index 000000000000..80fd5f04ea87 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-conditional-render-access.tsx @@ -0,0 +1,28 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Optional chain `user?.name` in render + unconditional `user.email` + * in closure. The closure's `user.email` makes the compiler think `user` is + * non-null, converting the render's `user?.name` to `user.name` in cache keys. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component({ + user, +}: { + user: {name: string; email: string} | null; +}) { + const sendEmail = () => { + console.log(user.email); + }; + return {user?.name}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice', email: 'alice@example.com'}}], + sequentialRenders: [ + {user: {name: 'Alice', email: 'alice@example.com'}}, + {user: {name: 'Bob', email: 'bob@example.com'}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-hook-argument.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-hook-argument.expect.md new file mode 100644 index 000000000000..34c4736ee850 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-hook-argument.expect.md @@ -0,0 +1,82 @@ + +## Input + +```javascript +import {Stringify, useIdentity} from 'shared-runtime'; + +/** + * Bug: Function passed as argument to a custom hook. The compiler assumes + * hook arguments are invoked and hoists `item.id` from the closure into + * a cache key that crashes when item is null. + * + * Related: https://github.com/facebook/react/issues/34194 + */ +function Component({item}: {item: {id: string} | null}) { + const processItem = () => { + return item.id; + }; + useIdentity(processItem); + if (!item) return null; + return {item.id}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{item: {id: 'abc'}}], + sequentialRenders: [{item: {id: 'abc'}}, {item: {id: 'def'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify, useIdentity } from "shared-runtime"; + +/** + * Bug: Function passed as argument to a custom hook. The compiler assumes + * hook arguments are invoked and hoists `item.id` from the closure into + * a cache key that crashes when item is null. + * + * Related: https://github.com/facebook/react/issues/34194 + */ +function Component(t0) { + const $ = _c(4); + const { item } = t0; + let t1; + if ($[0] !== item?.id) { + t1 = () => item.id; + $[0] = item?.id; + $[1] = t1; + } else { + t1 = $[1]; + } + const processItem = t1; + + useIdentity(processItem); + if (!item) { + return null; + } + let t2; + if ($[2] !== item.id) { + t2 = {item.id}; + $[2] = item.id; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ item: { id: "abc" } }], + sequentialRenders: [{ item: { id: "abc" } }, { item: { id: "def" } }], +}; + +``` + +### Eval output +(kind: ok)
{"children":"abc"}
+
{"children":"def"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-hook-argument.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-hook-argument.tsx new file mode 100644 index 000000000000..3b9193b85d56 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-hook-argument.tsx @@ -0,0 +1,23 @@ +import {Stringify, useIdentity} from 'shared-runtime'; + +/** + * Bug: Function passed as argument to a custom hook. The compiler assumes + * hook arguments are invoked and hoists `item.id` from the closure into + * a cache key that crashes when item is null. + * + * Related: https://github.com/facebook/react/issues/34194 + */ +function Component({item}: {item: {id: string} | null}) { + const processItem = () => { + return item.id; + }; + useIdentity(processItem); + if (!item) return null; + return {item.id}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{item: {id: 'abc'}}], + sequentialRenders: [{item: {id: 'abc'}}, {item: {id: 'def'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-mixed-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-mixed-optional-chain.expect.md new file mode 100644 index 000000000000..ad917b15b935 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-mixed-optional-chain.expect.md @@ -0,0 +1,75 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: `user?.company.name` inside a closure — the compiler strips the `?.` + * when computing cache keys, producing `user.company.name` which crashes + * when user is null. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component({user}: {user: {company: {name: string}} | null}) { + const handleClick = () => { + console.log(user?.company.name); + }; + return Click; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {company: {name: 'Acme'}}}], + sequentialRenders: [ + {user: {company: {name: 'Acme'}}}, + {user: {company: {name: 'Corp'}}}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: `user?.company.name` inside a closure — the compiler strips the `?.` + * when computing cache keys, producing `user.company.name` which crashes + * when user is null. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component(t0) { + const $ = _c(2); + const { user } = t0; + let t1; + if ($[0] !== user?.company?.name) { + const handleClick = () => { + console.log(user?.company.name); + }; + t1 = Click; + $[0] = user?.company?.name; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ user: { company: { name: "Acme" } } }], + sequentialRenders: [ + { user: { company: { name: "Acme" } } }, + { user: { company: { name: "Corp" } } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]","children":"Click"}
+
{"onClick":"[[ function params=0 ]]","children":"Click"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-mixed-optional-chain.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-mixed-optional-chain.tsx new file mode 100644 index 000000000000..1709614bded4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-mixed-optional-chain.tsx @@ -0,0 +1,24 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: `user?.company.name` inside a closure — the compiler strips the `?.` + * when computing cache keys, producing `user.company.name` which crashes + * when user is null. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component({user}: {user: {company: {name: string}} | null}) { + const handleClick = () => { + console.log(user?.company.name); + }; + return Click; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {company: {name: 'Acme'}}}], + sequentialRenders: [ + {user: {company: {name: 'Acme'}}}, + {user: {company: {name: 'Corp'}}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-multiple-closures.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-multiple-closures.expect.md new file mode 100644 index 000000000000..7cdc1f705710 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-multiple-closures.expect.md @@ -0,0 +1,124 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Multiple closures accessing different nullable props. Both `user.name` + * and `post.title` are hoisted as cache keys that crash when either is null. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component({ + user, + post, +}: { + user: {name: string} | null; + post: {title: string} | null; +}) { + const handleUser = () => { + console.log(user.name); + }; + const handlePost = () => { + console.log(post.title); + }; + if (!user || !post) return null; + return ( + + {user.name} + {post.title} + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice'}, post: {title: 'Hello'}}], + sequentialRenders: [ + {user: {name: 'Alice'}, post: {title: 'Hello'}}, + {user: {name: 'Bob'}, post: {title: 'World'}}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: Multiple closures accessing different nullable props. Both `user.name` + * and `post.title` are hoisted as cache keys that crash when either is null. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component(t0) { + const $ = _c(9); + const { user, post } = t0; + let t1; + if ($[0] !== user?.name) { + t1 = () => { + console.log(user.name); + }; + $[0] = user?.name; + $[1] = t1; + } else { + t1 = $[1]; + } + const handleUser = t1; + let t2; + if ($[2] !== post?.title) { + t2 = () => { + console.log(post.title); + }; + $[2] = post?.title; + $[3] = t2; + } else { + t2 = $[3]; + } + const handlePost = t2; + + if (!user || !post) { + return null; + } + let t3; + if ( + $[4] !== handlePost || + $[5] !== handleUser || + $[6] !== post.title || + $[7] !== user.name + ) { + t3 = ( + + {user.name} + {post.title} + + ); + $[4] = handlePost; + $[5] = handleUser; + $[6] = post.title; + $[7] = user.name; + $[8] = t3; + } else { + t3 = $[8]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ user: { name: "Alice" }, post: { title: "Hello" } }], + sequentialRenders: [ + { user: { name: "Alice" }, post: { title: "Hello" } }, + { user: { name: "Bob" }, post: { title: "World" } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"onUser":"[[ function params=0 ]]","onPost":"[[ function params=0 ]]","children":["Alice","Hello"]}
+
{"onUser":"[[ function params=0 ]]","onPost":"[[ function params=0 ]]","children":["Bob","World"]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-multiple-closures.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-multiple-closures.tsx new file mode 100644 index 000000000000..18977caf664a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-multiple-closures.tsx @@ -0,0 +1,38 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Multiple closures accessing different nullable props. Both `user.name` + * and `post.title` are hoisted as cache keys that crash when either is null. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component({ + user, + post, +}: { + user: {name: string} | null; + post: {title: string} | null; +}) { + const handleUser = () => { + console.log(user.name); + }; + const handlePost = () => { + console.log(post.title); + }; + if (!user || !post) return null; + return ( + + {user.name} + {post.title} + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice'}, post: {title: 'Hello'}}], + sequentialRenders: [ + {user: {name: 'Alice'}, post: {title: 'Hello'}}, + {user: {name: 'Bob'}, post: {title: 'World'}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-nested-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-nested-property.expect.md new file mode 100644 index 000000000000..c000a02e3986 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-nested-property.expect.md @@ -0,0 +1,97 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Deep property access on nullable base inside closure. The compiler + * hoists `post.author.profile.avatar` as a cache key that crashes when + * post is null, even though the early return guard prevents rendering. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component({ + post, +}: { + post: {author: {profile: {avatar: string}}} | null; +}) { + const handleClick = () => { + console.log(post.author.profile.avatar); + }; + if (!post) return null; + return ( + {post.author.profile.avatar} + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{post: {author: {profile: {avatar: 'pic.jpg'}}}}], + sequentialRenders: [ + {post: {author: {profile: {avatar: 'pic.jpg'}}}}, + {post: {author: {profile: {avatar: 'new.jpg'}}}}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: Deep property access on nullable base inside closure. The compiler + * hoists `post.author.profile.avatar` as a cache key that crashes when + * post is null, even though the early return guard prevents rendering. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component(t0) { + const $ = _c(5); + const { post } = t0; + let t1; + if ($[0] !== post?.author?.profile?.avatar) { + t1 = () => { + console.log(post.author.profile.avatar); + }; + $[0] = post?.author?.profile?.avatar; + $[1] = t1; + } else { + t1 = $[1]; + } + const handleClick = t1; + + if (!post) { + return null; + } + let t2; + if ($[2] !== handleClick || $[3] !== post.author.profile.avatar) { + t2 = ( + {post.author.profile.avatar} + ); + $[2] = handleClick; + $[3] = post.author.profile.avatar; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ post: { author: { profile: { avatar: "pic.jpg" } } } }], + sequentialRenders: [ + { post: { author: { profile: { avatar: "pic.jpg" } } } }, + { post: { author: { profile: { avatar: "new.jpg" } } } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]","children":"pic.jpg"}
+
{"onClick":"[[ function params=0 ]]","children":"new.jpg"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-nested-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-nested-property.tsx new file mode 100644 index 000000000000..14b7945d78da --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-nested-property.tsx @@ -0,0 +1,31 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Deep property access on nullable base inside closure. The compiler + * hoists `post.author.profile.avatar` as a cache key that crashes when + * post is null, even though the early return guard prevents rendering. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component({ + post, +}: { + post: {author: {profile: {avatar: string}}} | null; +}) { + const handleClick = () => { + console.log(post.author.profile.avatar); + }; + if (!post) return null; + return ( + {post.author.profile.avatar} + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{post: {author: {profile: {avatar: 'pic.jpg'}}}}], + sequentialRenders: [ + {post: {author: {profile: {avatar: 'pic.jpg'}}}}, + {post: {author: {profile: {avatar: 'new.jpg'}}}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-property-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-property-access.expect.md new file mode 100644 index 000000000000..cff3e41790b7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-property-access.expect.md @@ -0,0 +1,89 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: The compiler hoists `user.name` from the onClick closure into a cache + * key check that runs during render. When `user` is null, this crashes with + * TypeError even though the source code guards with an early return. + * + * The compiled output should use `user?.name` (optional) in the cache key, + * not `user.name` (non-optional). + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component({user}: {user: {name: string} | null}) { + const handleClick = () => { + console.log(user.name); + }; + if (!user) return null; + return {user.name}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice'}}], + sequentialRenders: [{user: {name: 'Alice'}}, {user: {name: 'Bob'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: The compiler hoists `user.name` from the onClick closure into a cache + * key check that runs during render. When `user` is null, this crashes with + * TypeError even though the source code guards with an early return. + * + * The compiled output should use `user?.name` (optional) in the cache key, + * not `user.name` (non-optional). + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component(t0) { + const $ = _c(5); + const { user } = t0; + let t1; + if ($[0] !== user?.name) { + t1 = () => { + console.log(user.name); + }; + $[0] = user?.name; + $[1] = t1; + } else { + t1 = $[1]; + } + const handleClick = t1; + + if (!user) { + return null; + } + let t2; + if ($[2] !== handleClick || $[3] !== user.name) { + t2 = {user.name}; + $[2] = handleClick; + $[3] = user.name; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ user: { name: "Alice" } }], + sequentialRenders: [{ user: { name: "Alice" } }, { user: { name: "Bob" } }], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]","children":"Alice"}
+
{"onClick":"[[ function params=0 ]]","children":"Bob"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-property-access.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-property-access.tsx new file mode 100644 index 000000000000..c46f20929818 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-property-access.tsx @@ -0,0 +1,25 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: The compiler hoists `user.name` from the onClick closure into a cache + * key check that runs during render. When `user` is null, this crashes with + * TypeError even though the source code guards with an early return. + * + * The compiled output should use `user?.name` (optional) in the cache key, + * not `user.name` (non-optional). + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function Component({user}: {user: {name: string} | null}) { + const handleClick = () => { + console.log(user.name); + }; + if (!user) return null; + return {user.name}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice'}}], + sequentialRenders: [{user: {name: 'Alice'}}, {user: {name: 'Bob'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-returned-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-returned-function.expect.md new file mode 100644 index 000000000000..f2ab2407a906 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-returned-function.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +import {createHookWrapper} from 'shared-runtime'; + +/** + * Bug: Returned function accessing nullable prop. The compiler classifies + * returned functions as "assumed invoked" and hoists `item.id` as a cache + * key that crashes when item is null. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function useHandler({item}: {item: {id: string} | null}) { + const handler = () => { + console.log(item.id); + }; + return handler; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useHandler), + params: [{item: {id: 'abc'}}], + sequentialRenders: [{item: {id: 'abc'}}, {item: {id: 'def'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { createHookWrapper } from "shared-runtime"; + +/** + * Bug: Returned function accessing nullable prop. The compiler classifies + * returned functions as "assumed invoked" and hoists `item.id` as a cache + * key that crashes when item is null. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function useHandler(t0) { + const $ = _c(2); + const { item } = t0; + let t1; + if ($[0] !== item?.id) { + t1 = () => { + console.log(item.id); + }; + $[0] = item?.id; + $[1] = t1; + } else { + t1 = $[1]; + } + const handler = t1; + + return handler; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useHandler), + params: [{ item: { id: "abc" } }], + sequentialRenders: [{ item: { id: "abc" } }, { item: { id: "def" } }], +}; + +``` + +### Eval output +(kind: ok)
{"result":{"kind":"Function"},"shouldInvokeFns":true}
+
{"result":{"kind":"Function"},"shouldInvokeFns":true}
+logs: ['abc','def'] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-returned-function.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-returned-function.tsx new file mode 100644 index 000000000000..3fbac77bd5d7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-returned-function.tsx @@ -0,0 +1,21 @@ +import {createHookWrapper} from 'shared-runtime'; + +/** + * Bug: Returned function accessing nullable prop. The compiler classifies + * returned functions as "assumed invoked" and hoists `item.id` as a cache + * key that crashes when item is null. + * + * Related: https://github.com/facebook/react/issues/35762 + */ +function useHandler({item}: {item: {id: string} | null}) { + const handler = () => { + console.log(item.id); + }; + return handler; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createHookWrapper(useHandler), + params: [{item: {id: 'abc'}}], + sequentialRenders: [{item: {id: 'abc'}}, {item: {id: 'def'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-ts-non-null-assertion.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-ts-non-null-assertion.expect.md new file mode 100644 index 000000000000..35bc8b9cdcfc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-ts-non-null-assertion.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: TypeScript non-null assertion (!) is transparent to the compiler. + * `data!.id` inside the closure is lowered as `data.id`, causing the compiler + * to hoist `data.id` as a cache key that crashes when data is undefined. + * + * Related: https://github.com/facebook/react/issues/34194 + */ +function Component({data}: {data: {id: string} | undefined}) { + const handleClick = () => { + console.log(data!.id); + }; + if (!data) return null; + return {data.id}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{data: {id: 'abc'}}], + sequentialRenders: [{data: {id: 'abc'}}, {data: {id: 'def'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: TypeScript non-null assertion (!) is transparent to the compiler. + * `data!.id` inside the closure is lowered as `data.id`, causing the compiler + * to hoist `data.id` as a cache key that crashes when data is undefined. + * + * Related: https://github.com/facebook/react/issues/34194 + */ +function Component(t0) { + const $ = _c(5); + const { data } = t0; + let t1; + if ($[0] !== data?.id) { + t1 = () => { + console.log(data.id); + }; + $[0] = data?.id; + $[1] = t1; + } else { + t1 = $[1]; + } + const handleClick = t1; + + if (!data) { + return null; + } + let t2; + if ($[2] !== data.id || $[3] !== handleClick) { + t2 = {data.id}; + $[2] = data.id; + $[3] = handleClick; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ data: { id: "abc" } }], + sequentialRenders: [{ data: { id: "abc" } }, { data: { id: "def" } }], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]","children":"abc"}
+
{"onClick":"[[ function params=0 ]]","children":"def"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-ts-non-null-assertion.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-ts-non-null-assertion.tsx new file mode 100644 index 000000000000..4faed5411159 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-ts-non-null-assertion.tsx @@ -0,0 +1,22 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: TypeScript non-null assertion (!) is transparent to the compiler. + * `data!.id` inside the closure is lowered as `data.id`, causing the compiler + * to hoist `data.id` as a cache key that crashes when data is undefined. + * + * Related: https://github.com/facebook/react/issues/34194 + */ +function Component({data}: {data: {id: string} | undefined}) { + const handleClick = () => { + console.log(data!.id); + }; + if (!data) return null; + return {data.id}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{data: {id: 'abc'}}], + sequentialRenders: [{data: {id: 'abc'}}, {data: {id: 'def'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-use-effect-event.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-use-effect-event.expect.md new file mode 100644 index 000000000000..8cba5adf57ab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-use-effect-event.expect.md @@ -0,0 +1,81 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Closure passed as JSX prop accesses nullable state. The compiler hoists + * `data.value` into a cache key that crashes because data could be null. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component({data}: {data: {value: string} | null}) { + const onData = () => { + console.log(data.value); + }; + return {data?.value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{data: {value: 'hello'}}], + sequentialRenders: [{data: {value: 'hello'}}, {data: {value: 'world'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Bug: Closure passed as JSX prop accesses nullable state. The compiler hoists + * `data.value` into a cache key that crashes because data could be null. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component(t0) { + const $ = _c(5); + const { data } = t0; + let t1; + if ($[0] !== data?.value) { + t1 = () => { + console.log(data.value); + }; + $[0] = data?.value; + $[1] = t1; + } else { + t1 = $[1]; + } + const onData = t1; + + const t2 = data?.value; + let t3; + if ($[2] !== onData || $[3] !== t2) { + t3 = {t2}; + $[2] = onData; + $[3] = t2; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ data: { value: "hello" } }], + sequentialRenders: [ + { data: { value: "hello" } }, + { data: { value: "world" } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"onData":"[[ function params=0 ]]","children":"hello"}
+
{"onData":"[[ function params=0 ]]","children":"world"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-use-effect-event.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-use-effect-event.tsx new file mode 100644 index 000000000000..27a03a9cdbe2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-hoisted-nullable-closure-use-effect-event.tsx @@ -0,0 +1,20 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Bug: Closure passed as JSX prop accesses nullable state. The compiler hoists + * `data.value` into a cache key that crashes because data could be null. + * + * Related: https://github.com/facebook/react/issues/34752 + */ +function Component({data}: {data: {value: string} | null}) { + const onData = () => { + console.log(data.value); + }; + return {data?.value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{data: {value: 'hello'}}], + sequentialRenders: [{data: {value: 'hello'}}, {data: {value: 'world'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-member-expr-call.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-member-expr-call.expect.md index cab9c9a500b9..04192e43a7f5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-member-expr-call.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-member-expr-call.expect.md @@ -35,11 +35,11 @@ function component(t0) { } const poke = t1; let t2; - if ($[2] !== mutator.user) { + if ($[2] !== mutator?.user) { t2 = () => { mutator.user.hide(); }; - $[2] = mutator.user; + $[2] = mutator?.user; $[3] = t2; } else { t2 = $[3]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-member-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-member-expr.expect.md index 943533a63d12..3e34766ee08f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-member-expr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-member-expr.expect.md @@ -34,11 +34,11 @@ function component(a) { } const z = t0; let t1; - if ($[2] !== z.a) { + if ($[2] !== z?.a) { t1 = function () { console.log(z.a); }; - $[2] = z.a; + $[2] = z?.a; $[3] = t1; } else { t1 = $[3]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr-in-nested-func.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr-in-nested-func.expect.md index 9e95af2b0d59..cbf30a145cb3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr-in-nested-func.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr-in-nested-func.expect.md @@ -25,29 +25,21 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function component(a) { - const $ = _c(4); + const $ = _c(2); let t0; if ($[0] !== a) { - t0 = { a: { a } }; - $[0] = a; - $[1] = t0; - } else { - t0 = $[1]; - } - const z = t0; - let t1; - if ($[2] !== z.a.a) { - t1 = function () { + const z = { a: { a } }; + t0 = function () { (function () { console.log(z.a.a); })(); }; - $[2] = z.a.a; - $[3] = t1; + $[0] = a; + $[1] = t0; } else { - t1 = $[3]; + t0 = $[1]; } - const x = t1; + const x = t0; return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr.expect.md index 13c122750e9b..0817c3d6f36c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-nested-member-expr.expect.md @@ -34,11 +34,11 @@ function component(a) { } const z = t0; let t1; - if ($[2] !== z.a.a) { + if ($[2] !== z?.a?.a) { t1 = function () { console.log(z.a.a); }; - $[2] = z.a.a; + $[2] = z?.a?.a; $[3] = t1; } else { t1 = $[3]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md index 3a8f9e84cac1..31c7893f4487 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md @@ -96,14 +96,14 @@ function Component(props) { } const urls = t5; let t6; - if ($[6] !== comments.length) { + if ($[6] !== comments?.length) { t6 = (e) => { if (!comments.length) { return; } console.log(comments.length); }; - $[6] = comments.length; + $[6] = comments?.length; $[7] = t6; } else { t6 = $[7]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md index fbb6e50871dc..3360794ecd04 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md @@ -55,14 +55,14 @@ function Component(props) { const allUrls = []; const { media, comments, urls } = post; let t1; - if ($[2] !== comments.length) { + if ($[2] !== comments?.length) { t1 = (e) => { if (!comments.length) { return; } console.log(comments.length); }; - $[2] = comments.length; + $[2] = comments?.length; $[3] = t1; } else { t1 = $[3]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md index 2558d10d19cb..06695c3b7fa3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md @@ -35,7 +35,7 @@ Found 1 error: Compilation Skipped: Existing memoization could not be preserved -React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `Ref.current`, but the source dependencies were []. Inferred dependency not present in source. +React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `Ref?.current`, but the source dependencies were []. Inferred dependency not present in source. error.ref-like-name-not-Ref.ts:11:30 9 | const Ref = useCustomRef(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md index 2c2f725ec84b..642cad49f081 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-a-ref.expect.md @@ -35,7 +35,7 @@ Found 1 error: Compilation Skipped: Existing memoization could not be preserved -React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `notaref.current`, but the source dependencies were []. Inferred dependency not present in source. +React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `notaref?.current`, but the source dependencies were []. Inferred dependency not present in source. error.ref-like-name-not-a-ref.ts:11:30 9 | const notaref = useCustomRef(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-call-freezes-captured-memberexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-call-freezes-captured-memberexpr.expect.md index 957919516d09..4c6e9c0f14d5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-call-freezes-captured-memberexpr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-call-freezes-captured-memberexpr.expect.md @@ -45,9 +45,9 @@ function Foo(t0) { } const x = t1; let t2; - if ($[2] !== x.inner) { + if ($[2] !== x?.inner) { t2 = () => x.inner; - $[2] = x.inner; + $[2] = x?.inner; $[3] = t2; } else { t2 = $[3]; 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 index c1a6dfb3eae1..778fbcab8ec4 100644 --- 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 @@ -60,9 +60,9 @@ function useFoo(t0) { const $ = _c(13); const { arr1, arr2 } = t0; let t1; - if ($[0] !== arr1[0]) { + if ($[0] !== arr1?.[0]) { t1 = (e) => arr1[0].value + e.value; - $[0] = arr1[0]; + $[0] = arr1?.[0]; $[1] = t1; } else { t1 = $[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 index 4622beeb0eb8..ce2c33a22356 100644 --- 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 @@ -45,22 +45,22 @@ function Component(t0) { const $ = _c(7); const { a, b } = t0; let t1; - if ($[0] !== a.value) { + if ($[0] !== a?.value) { t1 = () => { console.log(a.value); }; - $[0] = a.value; + $[0] = a?.value; $[1] = t1; } else { t1 = $[1]; } const logA = t1; let t2; - if ($[2] !== b.value) { + if ($[2] !== b?.value) { t2 = () => { console.log(b.value); }; - $[2] = b.value; + $[2] = b?.value; $[3] = t2; } else { t2 = $[3]; 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 index 77b62bc8c24b..511e4749f198 100644 --- 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 @@ -51,9 +51,9 @@ function useMakeCallback(t0) { const $ = _c(3); const { obj, shouldMakeCb, setState } = t0; let t1; - if ($[0] !== obj.value || $[1] !== setState) { + if ($[0] !== obj?.value || $[1] !== setState) { t1 = () => setState(obj.value); - $[0] = obj.value; + $[0] = obj?.value; $[1] = setState; $[2] = t1; } else { 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 index 8301912b024c..de72dba2c14a 100644 --- 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 @@ -89,9 +89,9 @@ function useMakeCallback(t0) { const $ = _c(6); const { obj, cond, setState } = t0; let t1; - if ($[0] !== obj.value || $[1] !== setState) { + if ($[0] !== obj?.value || $[1] !== setState) { t1 = () => setState(obj.value); - $[0] = obj.value; + $[0] = obj?.value; $[1] = setState; $[2] = t1; } else { 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 index ab8326a2286d..fc48b4e5e528 100644 --- 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 @@ -48,9 +48,9 @@ function useMakeCallback(t0) { const $ = _c(3); const { obj, setState } = t0; let t1; - if ($[0] !== obj.value || $[1] !== setState) { + if ($[0] !== obj?.value || $[1] !== setState) { t1 = () => setState(obj.value); - $[0] = obj.value; + $[0] = obj?.value; $[1] = setState; $[2] = t1; } else { 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 index 76228fc24911..94d905ebd29f 100644 --- 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 @@ -44,9 +44,9 @@ function useMakeCallback(t0) { const $ = _c(3); const { obj, setState } = t0; let t1; - if ($[0] !== obj.value || $[1] !== setState) { + if ($[0] !== obj?.value || $[1] !== setState) { t1 = setState(obj.value)} shouldInvokeFns={true} />; - $[0] = obj.value; + $[0] = obj?.value; $[1] = setState; $[2] = t1; } else { 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 index 31e317d07e99..3c6767ad25b8 100644 --- 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 @@ -47,9 +47,9 @@ function useMakeCallback(t0) { const $ = _c(3); const { obj, setState } = t0; let t1; - if ($[0] !== obj.value || $[1] !== setState) { + if ($[0] !== obj?.value || $[1] !== setState) { t1 = () => setState(obj.value); - $[0] = obj.value; + $[0] = obj?.value; $[1] = setState; $[2] = t1; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-named-callback-cross-context.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-named-callback-cross-context.expect.md index 7bc2c193cf7c..4300b06a281c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-named-callback-cross-context.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-named-callback-cross-context.expect.md @@ -61,9 +61,9 @@ function useFoo(t0) { const $ = _c(13); const { arr1, arr2 } = t0; let t1; - if ($[0] !== arr1[0]) { + if ($[0] !== arr1?.[0]) { t1 = (e) => arr1[0].value + e.value; - $[0] = arr1[0]; + $[0] = arr1?.[0]; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-direct-call-is-safe.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-direct-call-is-safe.expect.md new file mode 100644 index 000000000000..41b55c05619e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-direct-call-is-safe.expect.md @@ -0,0 +1,73 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Correctness guard: When a closure is directly called during render, + * it executes synchronously and its property accesses prove non-nullness. + * The cache key should remain `obj.value` (non-optional). This fixture + * must NOT change after the nullable-closure fix. + */ +function Component({obj}: {obj: {value: number}}) { + const getValue = () => obj.value; + const value = getValue(); + return {value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Correctness guard: When a closure is directly called during render, + * it executes synchronously and its property accesses prove non-nullness. + * The cache key should remain `obj.value` (non-optional). This fixture + * must NOT change after the nullable-closure fix. + */ +function Component(t0) { + const $ = _c(4); + const { obj } = t0; + let t1; + if ($[0] !== obj.value) { + const getValue = () => obj.value; + t1 = getValue(); + $[0] = obj.value; + $[1] = t1; + } else { + t1 = $[1]; + } + const value = t1; + let t2; + if ($[2] !== value) { + t2 = {value}; + $[2] = value; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ obj: { value: 1 } }], + sequentialRenders: [{ obj: { value: 1 } }, { obj: { value: 2 } }], +}; + +``` + +### Eval output +(kind: ok)
{"children":1}
+
{"children":2}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-direct-call-is-safe.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-direct-call-is-safe.tsx new file mode 100644 index 000000000000..507fd7679df4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-direct-call-is-safe.tsx @@ -0,0 +1,19 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Correctness guard: When a closure is directly called during render, + * it executes synchronously and its property accesses prove non-nullness. + * The cache key should remain `obj.value` (non-optional). This fixture + * must NOT change after the nullable-closure fix. + */ +function Component({obj}: {obj: {value: number}}) { + const getValue = () => obj.value; + const value = getValue(); + return {value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{obj: {value: 1}}], + sequentialRenders: [{obj: {value: 1}}, {obj: {value: 2}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-with-render-access-is-safe.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-with-render-access-is-safe.expect.md new file mode 100644 index 000000000000..eeececdb2d3b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-with-render-access-is-safe.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; + +/** + * Correctness guard: When there is a render-time property access (user.name) + * in the outer function body, it proves non-nullness at that point. The cache + * key should remain `user.name` (non-optional). This fixture must NOT change + * after the nullable-closure fix. + */ +function Component({user}: {user: {name: string}}) { + const name = user.name; + const handleClick = () => { + console.log(user.name); + }; + return {name}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice'}}], + sequentialRenders: [{user: {name: 'Alice'}}, {user: {name: 'Bob'}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +/** + * Correctness guard: When there is a render-time property access (user.name) + * in the outer function body, it proves non-nullness at that point. The cache + * key should remain `user.name` (non-optional). This fixture must NOT change + * after the nullable-closure fix. + */ +function Component(t0) { + const $ = _c(5); + const { user } = t0; + const name = user.name; + let t1; + if ($[0] !== user.name) { + t1 = () => { + console.log(user.name); + }; + $[0] = user.name; + $[1] = t1; + } else { + t1 = $[1]; + } + const handleClick = t1; + let t2; + if ($[2] !== handleClick || $[3] !== name) { + t2 = {name}; + $[2] = handleClick; + $[3] = name; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ user: { name: "Alice" } }], + sequentialRenders: [{ user: { name: "Alice" } }, { user: { name: "Bob" } }], +}; + +``` + +### Eval output +(kind: ok)
{"onClick":"[[ function params=0 ]]","children":"Alice"}
+
{"onClick":"[[ function params=0 ]]","children":"Bob"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-with-render-access-is-safe.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-with-render-access-is-safe.tsx new file mode 100644 index 000000000000..b0b052e9a467 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nullable-closure-with-render-access-is-safe.tsx @@ -0,0 +1,21 @@ +import {Stringify} from 'shared-runtime'; + +/** + * Correctness guard: When there is a render-time property access (user.name) + * in the outer function body, it proves non-nullness at that point. The cache + * key should remain `user.name` (non-optional). This fixture must NOT change + * after the nullable-closure fix. + */ +function Component({user}: {user: {name: string}}) { + const name = user.name; + const handleClick = () => { + console.log(user.name); + }; + return {name}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {name: 'Alice'}}], + sequentialRenders: [{user: {name: 'Alice'}}, {user: {name: 'Bob'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-conditional-access-noAlloc.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-conditional-access-noAlloc.expect.md index 075458831b8d..150efdc3bc23 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-conditional-access-noAlloc.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-conditional-access-noAlloc.expect.md @@ -29,7 +29,7 @@ Found 1 error: Compilation Skipped: Existing memoization could not be preserved -React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `propB?.x.y`, but the source dependencies were [propA, propB.x.y]. Inferred different dependency than source. +React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `propB?.x?.y`, but the source dependencies were [propA, propB.x.y]. Inferred different dependency than source. error.useCallback-conditional-access-noAlloc.ts:5:21 3 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md index 7e57f294927a..69e4b30f7cc5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-infer-more-specific.expect.md @@ -37,9 +37,9 @@ import { useCallback } from "react"; function useHook(x) { const $ = _c(2); let t0; - if ($[0] !== x.y.z) { + if ($[0] !== x?.y?.z) { t0 = () => [x.y.z]; - $[0] = x.y.z; + $[0] = x?.y?.z; $[1] = t0; } else { t0 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoisted.expect.md index 1ddc7495bc3e..d36e07616d39 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoisted.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoisted.expect.md @@ -29,9 +29,9 @@ function useFoo(t0) { const $ = _c(2); const { a } = t0; let t1; - if ($[0] !== a.b.c) { + if ($[0] !== a?.b?.c) { t1 = a.b.c} shouldInvokeFns={true} />; - $[0] = a.b.c; + $[0] = a?.b?.c; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoists-other-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoists-other-dep.expect.md index d82956e4a0db..a06e78af4304 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoists-other-dep.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoists-other-dep.expect.md @@ -51,12 +51,12 @@ function Foo(t0) { const fn = t1; useIdentity(null); let x; - if ($[2] !== a.b.c || $[3] !== cond) { + if ($[2] !== a?.b?.c || $[3] !== cond) { x = makeArray(); if (cond) { x.push(identity(a.b.c)); } - $[2] = a.b.c; + $[2] = a?.b?.c; $[3] = cond; $[4] = x; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-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-function-uncond-access-local-var.expect.md index 41bab7ccc9d6..b632c964d389 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-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-function-uncond-access-local-var.expect.md @@ -41,10 +41,10 @@ function useFoo(t0) { local = $[1]; } let t1; - if ($[2] !== local.b.c) { + if ($[2] !== local?.b?.c) { const fn = () => local.b.c; t1 = ; - $[2] = local.b.c; + $[2] = local?.b?.c; $[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-function-uncond-optional-hoists-other-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-optional-hoists-other-dep.expect.md index c81e59eceaf9..e3e805f362ca 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-optional-hoists-other-dep.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-optional-hoists-other-dep.expect.md @@ -50,12 +50,12 @@ function Foo(t0) { const fn = t1; useIdentity(null); let arr; - if ($[2] !== a.b?.c.e || $[3] !== cond) { + if ($[2] !== a?.b?.c?.e || $[3] !== cond) { arr = makeArray(); if (cond) { arr.push(identity(a.b?.c.e)); } - $[2] = a.b?.c.e; + $[2] = a?.b?.c?.e; $[3] = cond; $[4] = arr; } else { 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.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.expect.md index 77f5ae4db3c4..443b1863bd37 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.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.expect.md @@ -34,10 +34,10 @@ function useFoo(t0) { const $ = _c(2); const { a } = t0; let t1; - if ($[0] !== a.b.c) { + if ($[0] !== a) { const fn = () => () => ({ value: a.b.c }); t1 = ; - $[0] = a.b.c; + $[0] = a; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md index ed56ff068113..5e36a1f60f36 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md @@ -34,9 +34,9 @@ function useFoo(t0) { const $ = _c(2); const { a } = t0; let t1; - if ($[0] !== a.b?.c.d?.e) { + if ($[0] !== a?.b?.c?.d?.e) { t1 = a.b?.c.d?.e} shouldInvokeFns={true} />; - $[0] = a.b?.c.d?.e; + $[0] = a?.b?.c?.d?.e; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-invariant.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-invariant.expect.md index 73df2b615b9e..7d6507eaf11f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-invariant.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-invariant.expect.md @@ -29,9 +29,9 @@ function Foo(t0) { const $ = _c(5); const { data } = t0; let t1; - if ($[0] !== data.a.d) { + if ($[0] !== data.a?.d) { t1 = () => data.a.d; - $[0] = data.a.d; + $[0] = data.a?.d; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md index bb99a5d90fe2..bd091f9e334b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md @@ -31,9 +31,9 @@ function useFoo(t0) { const $ = _c(2); const { a } = t0; let t1; - if ($[0] !== a.b?.c.d?.e) { + if ($[0] !== a?.b?.c?.d?.e) { t1 = a.b?.c.d?.e} shouldInvokeFns={true} />; - $[0] = a.b?.c.d?.e; + $[0] = a?.b?.c?.d?.e; $[1] = t1; } else { t1 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-context-var-reassign-no-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-context-var-reassign-no-scope.expect.md index cacd56492214..9d95aec322f9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-context-var-reassign-no-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-context-var-reassign-no-scope.expect.md @@ -55,7 +55,7 @@ function Content() { } const [users, setUsers] = useState(t0); let t1; - if ($[1] !== users.length) { + if ($[1] !== users?.length) { t1 = () => { if (users.length === 2) { let removedUserName = ""; @@ -69,7 +69,7 @@ function Content() { setAnnouncement(`Removed user (${removedUserName})`); } }; - $[1] = users.length; + $[1] = users?.length; $[2] = t1; } else { t1 = $[2]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-maybe-mutate-context-in-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-maybe-mutate-context-in-callback.expect.md index 0525a0f7cdd9..1aeea2d8fdd2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-maybe-mutate-context-in-callback.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-maybe-mutate-context-in-callback.expect.md @@ -41,11 +41,11 @@ function Component(props) { const $ = _c(5); const Foo = useContext(FooContext); let t0; - if ($[0] !== Foo.current) { + if ($[0] !== Foo?.current) { t0 = () => { mutate(Foo.current); }; - $[0] = Foo.current; + $[0] = Foo?.current; $[1] = t0; } else { t0 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-read-context-in-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-read-context-in-callback.expect.md index e805b7f400e3..60d2c0aa67cd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-read-context-in-callback.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useContext-read-context-in-callback.expect.md @@ -34,11 +34,11 @@ function Component(props) { const $ = _c(5); const foo = useContext(FooContext); let t0; - if ($[0] !== foo.current) { + if ($[0] !== foo?.current) { t0 = () => { console.log(foo.current); }; - $[0] = foo.current; + $[0] = foo?.current; $[1] = t0; } else { t0 = $[1];