[compiler][autodeps/fire] Do not include fire functions in autodep arrays (#32532)

Summary: We landed on not including fire functions in dep arrays. They
aren't needed because all values returned from the useFire hook call
will read from the same ref. The linter will error if you include a
fired function in an explicit dep array.

Test Plan: yarn snap --watch

--
This commit is contained in:
Jordan Brown
2025-04-17 13:03:19 -04:00
committed by GitHub
parent 4a36d3eab7
commit b8bedc267f
6 changed files with 31 additions and 5 deletions

View File

@@ -9,6 +9,7 @@ import {Effect, ValueKind, ValueReason} from './HIR';
import {
BUILTIN_SHAPES,
BuiltInArrayId,
BuiltInFireFunctionId,
BuiltInFireId,
BuiltInMapId,
BuiltInMixedReadonlyId,
@@ -674,7 +675,12 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
{
positionalParams: [],
restParam: null,
returnType: {kind: 'Primitive'},
returnType: {
kind: 'Function',
return: {kind: 'Poly'},
shapeId: BuiltInFireFunctionId,
isConstructor: false,
},
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Frozen,
},

View File

@@ -1722,6 +1722,12 @@ export function isDispatcherType(id: Identifier): boolean {
return id.type.kind === 'Function' && id.type.shapeId === 'BuiltInDispatch';
}
export function isFireFunctionType(id: Identifier): boolean {
return (
id.type.kind === 'Function' && id.type.shapeId === 'BuiltInFireFunction'
);
}
export function isStableType(id: Identifier): boolean {
return (
isSetStateType(id) ||

View File

@@ -223,6 +223,7 @@ export const BuiltInUseContextHookId = 'BuiltInUseContextHook';
export const BuiltInUseTransitionId = 'BuiltInUseTransition';
export const BuiltInStartTransitionId = 'BuiltInStartTransition';
export const BuiltInFireId = 'BuiltInFire';
export const BuiltInFireFunctionId = 'BuiltInFireFunction';
// ShapeRegistry with default definitions for built-ins.
export const BUILTIN_SHAPES: ShapeRegistry = new Map();

View File

@@ -17,6 +17,7 @@ import {
ReactiveScopeDependencies,
isUseRefType,
isSetStateType,
isFireFunctionType,
} from '../HIR';
import {DEFAULT_EXPORT} from '../HIR/Environment';
import {
@@ -189,9 +190,10 @@ export function inferEffectDependencies(fn: HIRFunction): void {
*/
for (const dep of scopeInfo.deps) {
if (
(isUseRefType(dep.identifier) ||
((isUseRefType(dep.identifier) ||
isSetStateType(dep.identifier)) &&
!reactiveIds.has(dep.identifier.id)
!reactiveIds.has(dep.identifier.id)) ||
isFireFunctionType(dep.identifier)
) {
// exclude non-reactive hook results, which will never be in a memo block
continue;

View File

@@ -34,7 +34,11 @@ import {
} from '../HIR';
import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder';
import {getOrInsertWith} from '../Utils/utils';
import {BuiltInFireId, DefaultNonmutatingHook} from '../HIR/ObjectShape';
import {
BuiltInFireFunctionId,
BuiltInFireId,
DefaultNonmutatingHook,
} from '../HIR/ObjectShape';
import {eachInstructionOperand} from '../HIR/visitors';
import {printSourceLocationLine} from '../HIR/PrintHIR';
import {USE_FIRE_FUNCTION_NAME} from '../HIR/Environment';
@@ -633,6 +637,13 @@ class Context {
() => createTemporaryPlace(this.#env, GeneratedSource),
);
fireFunctionBinding.identifier.type = {
kind: 'Function',
shapeId: BuiltInFireFunctionId,
return: {kind: 'Poly'},
isConstructor: false,
};
this.#capturedCalleeIdentifierIds.set(callee.identifier.id, {
fireFunctionBinding,
capturedCalleeIdentifier: callee.identifier,

View File

@@ -49,7 +49,7 @@ function Component(props) {
} else {
t2 = $[4];
}
useEffect(t2, [t1, props]);
useEffect(t2, [props]);
return null;
}