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 77494d0a04ea..3e1c2b5e58c5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -31,6 +31,7 @@ import { PropertyLiteral, ReactiveScopeDependency, ScopeId, + SourceLocation, TInstruction, } from './HIR'; @@ -244,6 +245,7 @@ class PropertyPathRegistry { getOrCreateIdentifier( identifier: Identifier, reactive: boolean, + loc: SourceLocation, ): PropertyPathNode { /** * Reads from a statically scoped variable are always safe in JS, @@ -260,6 +262,7 @@ class PropertyPathRegistry { identifier, reactive, path: [], + loc, }, hasOptional: false, parent: null, @@ -290,6 +293,7 @@ class PropertyPathRegistry { identifier: parent.fullPath.identifier, reactive: parent.fullPath.reactive, path: parent.fullPath.path.concat(entry), + loc: entry.loc, }, hasOptional: parent.hasOptional || entry.optional, }; @@ -304,7 +308,7 @@ class PropertyPathRegistry { * so all subpaths of a PropertyLoad should already exist * (e.g. a.b is added before a.b.c), */ - let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive); + let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive, n.loc); if (n.path.length === 0) { return currNode; } @@ -323,20 +327,21 @@ class PropertyPathRegistry { } function getMaybeNonNullInInstruction( - instr: InstructionValue, + value: InstructionValue, context: CollectHoistablePropertyLoadsContext, ): PropertyPathNode | null { let path: ReactiveScopeDependency | null = null; - if (instr.kind === 'PropertyLoad') { - path = context.temporaries.get(instr.object.identifier.id) ?? { - identifier: instr.object.identifier, - reactive: instr.object.reactive, + if (value.kind === 'PropertyLoad') { + path = context.temporaries.get(value.object.identifier.id) ?? { + identifier: value.object.identifier, + reactive: value.object.reactive, path: [], + loc: value.loc, }; - } else if (instr.kind === 'Destructure') { - path = context.temporaries.get(instr.value.identifier.id) ?? null; - } else if (instr.kind === 'ComputedLoad') { - path = context.temporaries.get(instr.object.identifier.id) ?? null; + } else if (value.kind === 'Destructure') { + path = context.temporaries.get(value.value.identifier.id) ?? null; + } else if (value.kind === 'ComputedLoad') { + path = context.temporaries.get(value.object.identifier.id) ?? null; } return path != null ? context.registry.getOrCreateProperty(path) : null; } @@ -393,7 +398,11 @@ function collectNonNullsInBlocks( ) { const identifier = fn.params[0].identifier; knownNonNullIdentifiers.add( - context.registry.getOrCreateIdentifier(identifier, true), + context.registry.getOrCreateIdentifier( + identifier, + true, + fn.params[0].loc, + ), ); } const nodes = new Map< @@ -468,6 +477,7 @@ function collectNonNullsInBlocks( identifier: dep.root.value.identifier, path: dep.path.slice(0, i), reactive: dep.root.value.reactive, + loc: dep.loc, }); assumedNonNullObjects.add(depNode); } @@ -654,17 +664,23 @@ function reduceMaybeOptionalChains( changed = false; for (const original of optionalChainNodes) { - let {identifier, path: origPath, reactive} = original.fullPath; + let { + identifier, + path: origPath, + reactive, + loc: origLoc, + } = original.fullPath; let currNode: PropertyPathNode = registry.getOrCreateIdentifier( identifier, reactive, + origLoc, ); for (let i = 0; i < origPath.length; i++) { const entry = origPath[i]; // If the base is known to be non-null, replace with a non-optional load const nextEntry: DependencyPathEntry = entry.optional && nodes.has(currNode) - ? {property: entry.property, optional: false} + ? {property: entry.property, optional: false, loc: entry.loc} : entry; currNode = PropertyPathRegistry.getOrCreatePropertyEntry( currNode, diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts index 75dad4c1bfe6..f78598ec3c6a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError} from '..'; +import {CompilerError, SourceLocation} from '..'; import {assertNonNull} from './CollectHoistablePropertyLoads'; import { BlockId, @@ -169,6 +169,7 @@ function matchOptionalTestBlock( propertyId: IdentifierId; storeLocalInstr: Instruction; consequentGoto: BlockId; + propertyLoadLoc: SourceLocation; } | null { const consequentBlock = assertNonNull(blocks.get(terminal.consequent)); if ( @@ -221,6 +222,7 @@ function matchOptionalTestBlock( propertyId: propertyLoad.lvalue.identifier.id, storeLocalInstr, consequentGoto: consequentBlock.terminal.block, + propertyLoadLoc: propertyLoad.loc, }; } return null; @@ -275,7 +277,11 @@ function traverseOptionalBlock( instrVal.kind === 'PropertyLoad' && instrVal.object.identifier.id === prevInstr.lvalue.identifier.id ) { - path.push({property: instrVal.property, optional: false}); + path.push({ + property: instrVal.property, + optional: false, + loc: instrVal.loc, + }); } else { return null; } @@ -292,6 +298,7 @@ function traverseOptionalBlock( identifier: maybeTest.instructions[0].value.place.identifier, reactive: maybeTest.instructions[0].value.place.reactive, path, + loc: maybeTest.instructions[0].value.place.loc, }; test = maybeTest.terminal; } else if (maybeTest.terminal.kind === 'optional') { @@ -390,7 +397,7 @@ function traverseOptionalBlock( loc: optional.terminal.loc, }, ); - const load = { + const load: ReactiveScopeDependency = { identifier: baseObject.identifier, reactive: baseObject.reactive, path: [ @@ -398,8 +405,10 @@ function traverseOptionalBlock( { property: matchConsequentResult.property, optional: optional.terminal.optional, + loc: matchConsequentResult.propertyLoadLoc, }, ], + loc: matchConsequentResult.propertyLoadLoc, }; context.processedInstrsInOptional.add(matchConsequentResult.storeLocalInstr); context.processedInstrsInOptional.add(test); 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 7819ab39b2c6..2850e73ca5a0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -12,6 +12,7 @@ import { Identifier, PropertyLiteral, ReactiveScopeDependency, + SourceLocation, } from '../HIR'; import {printIdentifier} from '../HIR/PrintHIR'; @@ -36,12 +37,13 @@ export class ReactiveScopeDependencyTreeHIR { * duplicates when traversing the CFG. */ constructor(hoistableObjects: Iterable) { - for (const {path, identifier, reactive} of hoistableObjects) { + for (const {path, identifier, reactive, loc} of hoistableObjects) { let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, reactive, this.#hoistableObjects, path.length > 0 && path[0].optional ? 'Optional' : 'NonNull', + loc, ); for (let i = 0; i < path.length; i++) { @@ -62,6 +64,7 @@ export class ReactiveScopeDependencyTreeHIR { nextNode = { properties: new Map(), accessType, + loc: path[i].loc, }; currNode.properties.set(path[i].property, nextNode); } @@ -75,6 +78,7 @@ export class ReactiveScopeDependencyTreeHIR { reactive: boolean, roots: Map & {reactive: boolean}>, defaultAccessType: T, + loc: SourceLocation, ): TreeNode { // roots can always be accessed unconditionally in JS let rootNode = roots.get(identifier); @@ -84,6 +88,7 @@ export class ReactiveScopeDependencyTreeHIR { properties: new Map(), reactive, accessType: defaultAccessType, + loc, }; roots.set(identifier, rootNode); } else { @@ -102,12 +107,13 @@ export class ReactiveScopeDependencyTreeHIR { * safe-to-evaluate subpath */ addDependency(dep: ReactiveScopeDependency): void { - const {identifier, reactive, path} = dep; + const {identifier, reactive, path, loc} = dep; let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, reactive, this.#deps, PropertyAccessType.UnconditionalAccess, + loc, ); /** * hoistableCursor is null if depCursor is not an object we can hoist @@ -153,6 +159,7 @@ export class ReactiveScopeDependencyTreeHIR { depCursor, entry.property, accessType, + entry.loc, ); } else if ( hoistableCursor != null && @@ -163,6 +170,7 @@ export class ReactiveScopeDependencyTreeHIR { depCursor, entry.property, PropertyAccessType.UnconditionalAccess, + entry.loc, ); } else { /** @@ -306,6 +314,7 @@ function merge( type TreeNode = { properties: Map>; accessType: T; + loc: SourceLocation; }; type HoistableNode = TreeNode<'Optional' | 'NonNull'>; type DependencyNode = TreeNode; @@ -323,7 +332,7 @@ function collectMinimalDependenciesInSubtree( results: Set, ): void { if (isDependency(node.accessType)) { - results.add({identifier: rootIdentifier, reactive, path}); + results.add({identifier: rootIdentifier, reactive, path, loc: node.loc}); } else { for (const [childName, childNode] of node.properties) { collectMinimalDependenciesInSubtree( @@ -335,6 +344,7 @@ function collectMinimalDependenciesInSubtree( { property: childName, optional: isOptional(childNode.accessType), + loc: childNode.loc, }, ], results, @@ -362,12 +372,14 @@ function makeOrMergeProperty( node: DependencyNode, property: PropertyLiteral, accessType: PropertyAccessType, + loc: SourceLocation, ): DependencyNode { let child = node.properties.get(property); if (child == null) { child = { properties: new Map(), accessType, + loc, }; node.properties.set(property, child); } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index dca41eac92fe..fa78e3d3001c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1639,6 +1639,7 @@ export function makePropertyLiteral(value: string | number): PropertyLiteral { export type DependencyPathEntry = { property: PropertyLiteral; optional: boolean; + loc: SourceLocation; }; export type DependencyPath = Array; export type ReactiveScopeDependency = { @@ -1656,6 +1657,7 @@ export type ReactiveScopeDependency = { */ reactive: boolean; path: DependencyPath; + loc: SourceLocation; }; export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean { 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 91b7712b881f..19b4fae30fda 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -31,6 +31,7 @@ import { ObjectMethod, PropertyLiteral, convertHoistedLValueKind, + SourceLocation, } from './HIR'; import { collectHoistablePropertyLoads, @@ -298,6 +299,7 @@ function collectTemporariesSidemapImpl( value.object, value.property, false, + value.loc, temporaries, ); temporaries.set(lvalue.identifier.id, property); @@ -318,6 +320,7 @@ function collectTemporariesSidemapImpl( identifier: value.place.identifier, reactive: value.place.reactive, path: [], + loc: value.loc, }); } } else if ( @@ -339,6 +342,7 @@ function getProperty( object: Place, propertyName: PropertyLiteral, optional: boolean, + loc: SourceLocation, temporaries: ReadonlyMap, ): ReactiveScopeDependency { /* @@ -371,13 +375,18 @@ function getProperty( property = { identifier: object.identifier, reactive: object.reactive, - path: [{property: propertyName, optional}], + path: [{property: propertyName, optional, loc}], + loc, }; } else { property = { identifier: resolvedDependency.identifier, reactive: resolvedDependency.reactive, - path: [...resolvedDependency.path, {property: propertyName, optional}], + path: [ + ...resolvedDependency.path, + {property: propertyName, optional, loc}, + ], + loc, }; } return property; @@ -537,6 +546,7 @@ export class DependencyCollectionContext { identifier: place.identifier, reactive: place.reactive, path: [], + loc: place.loc, }, ); } @@ -545,11 +555,13 @@ export class DependencyCollectionContext { object: Place, property: PropertyLiteral, optional: boolean, + loc: SourceLocation, ): void { const nextDependency = getProperty( object, property, optional, + loc, this.#temporaries, ); this.visitDependency(nextDependency); @@ -602,6 +614,7 @@ export class DependencyCollectionContext { identifier: maybeDependency.identifier, reactive: maybeDependency.reactive, path: [], + loc: maybeDependency.loc, }; } if (this.#checkValidDependency(maybeDependency)) { @@ -626,6 +639,7 @@ export class DependencyCollectionContext { identifier: place.identifier, reactive: place.reactive, path: [], + loc: place.loc, }) ) { currentScope.reassignments.add(place.identifier); @@ -679,7 +693,7 @@ export function handleInstruction( return; } if (value.kind === 'PropertyLoad') { - context.visitProperty(value.object, value.property, false); + context.visitProperty(value.object, value.property, false, value.loc); } else if (value.kind === 'StoreLocal') { context.visitOperand(value.value); if (value.lvalue.kind === InstructionKind.Reassign) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index 3f589bba9545..a6d680755c72 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -74,7 +74,10 @@ export function collectMaybeMemoDependencies( return { root: object.root, // TODO: determine if the access is optional - path: [...object.path, {property: value.property, optional}], + path: [ + ...object.path, + {property: value.property, optional, loc: value.loc}, + ], loc: value.loc, }; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts index bdf2f29284aa..f6cf491220c1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts @@ -470,6 +470,7 @@ function canMergeScopes( identifier: declaration.identifier, reactive: true, path: [], + loc: GeneratedSource, })), ), next.scope.dependencies, diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts index ffb25082c20f..ddc65423f38d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts @@ -22,6 +22,7 @@ import { printIdentifier, printInstructionValue, printPlace, + printSourceLocation, printType, } from '../HIR/PrintHIR'; import {assertExhaustive} from '../Utils/utils'; @@ -114,7 +115,7 @@ export function printDependency(dependency: ReactiveScopeDependency): string { const identifier = printIdentifier(dependency.identifier) + printType(dependency.identifier.type); - return `${identifier}${dependency.path.map(token => `${token.optional ? '?.' : '.'}${token.property}`).join('')}`; + return `${identifier}${dependency.path.map(token => `${token.optional ? '?.' : '.'}${token.property}`).join('')}_${printSourceLocation(dependency.loc)}`; } export function printReactiveInstructions( diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index b8647ec7c9bd..bfde19991e9f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -756,6 +756,7 @@ function collectDependencies( { optional, property: value.property, + loc: value.loc, }, ], loc: value.loc, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repeated-dependencies-more-precise.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repeated-dependencies-more-precise.expect.md new file mode 100644 index 000000000000..27da22eb0fa5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repeated-dependencies-more-precise.expect.md @@ -0,0 +1,87 @@ + +## Input + +```javascript +// @flow +import {Stringify} from 'shared-runtime'; + +/** + * Example fixture demonstrating a case where we could hoist dependencies + * and reuse them across scopes. Here we extract a temporary for `item.value` + * and reference it both in the scope for `a`. Then the scope for `c` could + * use `.inner` as its dependency, avoiding reloading + * `item.value`. + */ +function Test({item, index}: {item: {value: {inner: any}}, index: number}) { + // These scopes have the same dependency, `item.value`, and could + // share a hoisted expression to evaluate it + const a = []; + if (index) { + a.push({value: item.value, index}); + } + const b = [item.value]; + + // This dependency is more precise (nested property), the outer + // `item.value` portion could use a hoisted dep for `item.value + const c = [item.value.inner]; + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; + +function Test(t0) { + const $ = _c(11); + const { item, index } = t0; + let a; + if ($[0] !== index || $[1] !== item.value) { + a = []; + if (index) { + a.push({ value: item.value, index }); + } + $[0] = index; + $[1] = item.value; + $[2] = a; + } else { + a = $[2]; + } + let t1; + if ($[3] !== item.value) { + t1 = [item.value]; + $[3] = item.value; + $[4] = t1; + } else { + t1 = $[4]; + } + const b = t1; + let t2; + if ($[5] !== item.value.inner) { + t2 = [item.value.inner]; + $[5] = item.value.inner; + $[6] = t2; + } else { + t2 = $[6]; + } + const c = t2; + let t3; + if ($[7] !== a || $[8] !== b || $[9] !== c) { + t3 = ; + $[7] = a; + $[8] = b; + $[9] = c; + $[10] = t3; + } else { + t3 = $[10]; + } + return t3; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repeated-dependencies-more-precise.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repeated-dependencies-more-precise.js new file mode 100644 index 000000000000..63ebbdd91b7b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repeated-dependencies-more-precise.js @@ -0,0 +1,24 @@ +// @flow +import {Stringify} from 'shared-runtime'; + +/** + * Example fixture demonstrating a case where we could hoist dependencies + * and reuse them across scopes. Here we extract a temporary for `item.value` + * and reference it both in the scope for `a`. Then the scope for `c` could + * use `.inner` as its dependency, avoiding reloading + * `item.value`. + */ +function Test({item, index}: {item: {value: {inner: any}}, index: number}) { + // These scopes have the same dependency, `item.value`, and could + // share a hoisted expression to evaluate it + const a = []; + if (index) { + a.push({value: item.value, index}); + } + const b = [item.value]; + + // This dependency is more precise (nested property), the outer + // `item.value` portion could use a hoisted dep for `item.value + const c = [item.value.inner]; + return ; +} diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index cc7f0d0c36a2..4eeac5433777 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -278,6 +278,7 @@ export function getChildHostContext( const isInAParentText = type === 'AndroidTextInput' || // Android type === 'RCTMultilineTextInputView' || // iOS + type === 'RCTSelectableText' || type === 'RCTSinglelineTextInputView' || // iOS type === 'RCTText' || type === 'RCTVirtualText'; diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index fcf356776c2e..404ae7a54a85 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -275,6 +275,7 @@ export function getChildHostContext( const isInAParentText = type === 'AndroidTextInput' || // Android type === 'RCTMultilineTextInputView' || // iOS + type === 'RCTSelectableText' || type === 'RCTSinglelineTextInputView' || // iOS type === 'RCTText' || type === 'RCTVirtualText';