Skip to content

Commit b8bedc2

Browse files
authored
[compiler][autodeps/fire] Do not include fire functions in autodep arrays (facebook#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 --
1 parent 4a36d3e commit b8bedc2

File tree

6 files changed

+31
-5
lines changed

6 files changed

+31
-5
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {Effect, ValueKind, ValueReason} from './HIR';
99
import {
1010
BUILTIN_SHAPES,
1111
BuiltInArrayId,
12+
BuiltInFireFunctionId,
1213
BuiltInFireId,
1314
BuiltInMapId,
1415
BuiltInMixedReadonlyId,
@@ -674,7 +675,12 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
674675
{
675676
positionalParams: [],
676677
restParam: null,
677-
returnType: {kind: 'Primitive'},
678+
returnType: {
679+
kind: 'Function',
680+
return: {kind: 'Poly'},
681+
shapeId: BuiltInFireFunctionId,
682+
isConstructor: false,
683+
},
678684
calleeEffect: Effect.Read,
679685
returnValueKind: ValueKind.Frozen,
680686
},

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,6 +1722,12 @@ export function isDispatcherType(id: Identifier): boolean {
17221722
return id.type.kind === 'Function' && id.type.shapeId === 'BuiltInDispatch';
17231723
}
17241724

1725+
export function isFireFunctionType(id: Identifier): boolean {
1726+
return (
1727+
id.type.kind === 'Function' && id.type.shapeId === 'BuiltInFireFunction'
1728+
);
1729+
}
1730+
17251731
export function isStableType(id: Identifier): boolean {
17261732
return (
17271733
isSetStateType(id) ||

compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ export const BuiltInUseContextHookId = 'BuiltInUseContextHook';
223223
export const BuiltInUseTransitionId = 'BuiltInUseTransition';
224224
export const BuiltInStartTransitionId = 'BuiltInStartTransition';
225225
export const BuiltInFireId = 'BuiltInFire';
226+
export const BuiltInFireFunctionId = 'BuiltInFireFunction';
226227

227228
// ShapeRegistry with default definitions for built-ins.
228229
export const BUILTIN_SHAPES: ShapeRegistry = new Map();

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
ReactiveScopeDependencies,
1818
isUseRefType,
1919
isSetStateType,
20+
isFireFunctionType,
2021
} from '../HIR';
2122
import {DEFAULT_EXPORT} from '../HIR/Environment';
2223
import {
@@ -189,9 +190,10 @@ export function inferEffectDependencies(fn: HIRFunction): void {
189190
*/
190191
for (const dep of scopeInfo.deps) {
191192
if (
192-
(isUseRefType(dep.identifier) ||
193+
((isUseRefType(dep.identifier) ||
193194
isSetStateType(dep.identifier)) &&
194-
!reactiveIds.has(dep.identifier.id)
195+
!reactiveIds.has(dep.identifier.id)) ||
196+
isFireFunctionType(dep.identifier)
195197
) {
196198
// exclude non-reactive hook results, which will never be in a memo block
197199
continue;

compiler/packages/babel-plugin-react-compiler/src/Transform/TransformFire.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ import {
3434
} from '../HIR';
3535
import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder';
3636
import {getOrInsertWith} from '../Utils/utils';
37-
import {BuiltInFireId, DefaultNonmutatingHook} from '../HIR/ObjectShape';
37+
import {
38+
BuiltInFireFunctionId,
39+
BuiltInFireId,
40+
DefaultNonmutatingHook,
41+
} from '../HIR/ObjectShape';
3842
import {eachInstructionOperand} from '../HIR/visitors';
3943
import {printSourceLocationLine} from '../HIR/PrintHIR';
4044
import {USE_FIRE_FUNCTION_NAME} from '../HIR/Environment';
@@ -633,6 +637,13 @@ class Context {
633637
() => createTemporaryPlace(this.#env, GeneratedSource),
634638
);
635639

640+
fireFunctionBinding.identifier.type = {
641+
kind: 'Function',
642+
shapeId: BuiltInFireFunctionId,
643+
return: {kind: 'Poly'},
644+
isConstructor: false,
645+
};
646+
636647
this.#capturedCalleeIdentifierIds.set(callee.identifier.id, {
637648
fireFunctionBinding,
638649
capturedCalleeIdentifier: callee.identifier,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/fire-and-autodeps.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function Component(props) {
4949
} else {
5050
t2 = $[4];
5151
}
52-
useEffect(t2, [t1, props]);
52+
useEffect(t2, [props]);
5353
return null;
5454
}
5555

0 commit comments

Comments
 (0)