Skip to content

Commit 6bcf0d2

Browse files
authored
[compiler] Empty dep arrays for globals/module-scoped values/imports (#31666)
Any LoadGlobal in the "infer deps" position can safely use an empty dep array. Globals have no reactive deps! I just keep messing up sapling. This is the revised version of #31662
1 parent b9b510d commit 6bcf0d2

File tree

2 files changed

+42
-32
lines changed

2 files changed

+42
-32
lines changed

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

+41-31
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export function inferEffectDependencies(fn: HIRFunction): void {
5555
{pruned: boolean; deps: ReactiveScopeDependencies; hasSingleInstr: boolean}
5656
>();
5757

58+
const loadGlobals = new Set<IdentifierId>();
59+
5860
/**
5961
* When inserting LoadLocals, we need to retain the reactivity of the base
6062
* identifier, as later passes e.g. PruneNonReactiveDeps take the reactivity of
@@ -87,26 +89,23 @@ export function inferEffectDependencies(fn: HIRFunction): void {
8789
lvalue.identifier.id,
8890
instr as TInstruction<FunctionExpression>,
8991
);
90-
} else if (
91-
value.kind === 'LoadGlobal' &&
92-
value.binding.kind === 'ImportSpecifier'
93-
) {
94-
const moduleTargets = autodepFnConfigs.get(value.binding.module);
95-
if (moduleTargets != null) {
96-
const numRequiredArgs = moduleTargets.get(value.binding.imported);
97-
if (numRequiredArgs != null) {
98-
autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs);
99-
}
100-
}
101-
} else if (
102-
value.kind === 'LoadGlobal' &&
103-
value.binding.kind === 'ImportDefault'
104-
) {
105-
const moduleTargets = autodepFnConfigs.get(value.binding.module);
106-
if (moduleTargets != null) {
107-
const numRequiredArgs = moduleTargets.get(DEFAULT_EXPORT);
108-
if (numRequiredArgs != null) {
109-
autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs);
92+
} else if (value.kind === 'LoadGlobal') {
93+
loadGlobals.add(lvalue.identifier.id);
94+
95+
if (
96+
value.binding.kind === 'ImportSpecifier' ||
97+
value.binding.kind === 'ImportDefault'
98+
) {
99+
const moduleTargets = autodepFnConfigs.get(value.binding.module);
100+
if (moduleTargets != null) {
101+
const importSpecifierName =
102+
value.binding.kind === 'ImportSpecifier'
103+
? value.binding.imported
104+
: DEFAULT_EXPORT;
105+
const numRequiredArgs = moduleTargets.get(importSpecifierName);
106+
if (numRequiredArgs != null) {
107+
autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs);
108+
}
110109
}
111110
}
112111
} else if (
@@ -117,8 +116,19 @@ export function inferEffectDependencies(fn: HIRFunction): void {
117116
autodepFnLoads.get(value.callee.identifier.id) === value.args.length &&
118117
value.args[0].kind === 'Identifier'
119118
) {
119+
const effectDeps: Array<Place> = [];
120+
const newInstructions: Array<Instruction> = [];
121+
const deps: ArrayExpression = {
122+
kind: 'ArrayExpression',
123+
elements: effectDeps,
124+
loc: GeneratedSource,
125+
};
126+
const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
127+
depsPlace.effect = Effect.Read;
128+
120129
const fnExpr = fnExpressions.get(value.args[0].identifier.id);
121130
if (fnExpr != null) {
131+
// We have a function expression, so we can infer its dependencies
122132
const scopeInfo =
123133
fnExpr.lvalue.identifier.scope != null
124134
? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
@@ -140,14 +150,12 @@ export function inferEffectDependencies(fn: HIRFunction): void {
140150
}
141151

142152
/**
143-
* Step 1: write new instructions to insert a dependency array
153+
* Step 1: push dependencies to the effect deps array
144154
*
145155
* Note that it's invalid to prune non-reactive deps in this pass, see
146156
* the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
147157
* explanation.
148158
*/
149-
const effectDeps: Array<Place> = [];
150-
const newInstructions: Array<Instruction> = [];
151159
for (const dep of scopeInfo.deps) {
152160
const {place, instructions} = writeDependencyToInstructions(
153161
dep,
@@ -158,14 +166,6 @@ export function inferEffectDependencies(fn: HIRFunction): void {
158166
newInstructions.push(...instructions);
159167
effectDeps.push(place);
160168
}
161-
const deps: ArrayExpression = {
162-
kind: 'ArrayExpression',
163-
elements: effectDeps,
164-
loc: GeneratedSource,
165-
};
166-
167-
const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
168-
depsPlace.effect = Effect.Read;
169169

170170
newInstructions.push({
171171
id: makeInstructionId(0),
@@ -177,6 +177,16 @@ export function inferEffectDependencies(fn: HIRFunction): void {
177177
// Step 2: push the inferred deps array as an argument of the useEffect
178178
value.args.push({...depsPlace, effect: Effect.Freeze});
179179
rewriteInstrs.set(instr.id, newInstructions);
180+
} else if (loadGlobals.has(value.args[0].identifier.id)) {
181+
// Global functions have no reactive dependencies, so we can insert an empty array
182+
newInstructions.push({
183+
id: makeInstructionId(0),
184+
loc: GeneratedSource,
185+
lvalue: {...depsPlace, effect: Effect.Mutate},
186+
value: deps,
187+
});
188+
value.args.push({...depsPlace, effect: Effect.Freeze});
189+
rewriteInstrs.set(instr.id, newInstructions);
180190
}
181191
}
182192
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/outlined-function.expect.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { print } from "shared-runtime";
3434
* before OutlineFunctions
3535
*/
3636
function OutlinedFunctionInEffect() {
37-
useEffect(_temp);
37+
useEffect(_temp, []);
3838
}
3939
function _temp() {
4040
return print("hello world!");

0 commit comments

Comments
 (0)