Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
);
}
const autodepFnLoads = new Map<IdentifierId, number>();
const autodepModuleLoads = new Map<IdentifierId, Map<string, number>>();

const scopeInfos = new Map<
ScopeId,
Expand Down Expand Up @@ -89,9 +90,34 @@ export function inferEffectDependencies(fn: HIRFunction): void {
lvalue.identifier.id,
instr as TInstruction<FunctionExpression>,
);
} else if (value.kind === 'PropertyLoad') {
if (
typeof value.property === 'string' &&
autodepModuleLoads.has(value.object.identifier.id)
) {
const moduleTargets = autodepModuleLoads.get(
value.object.identifier.id,
)!;
const propertyName = value.property;
const numRequiredArgs = moduleTargets.get(propertyName);
if (numRequiredArgs != null) {
autodepFnLoads.set(lvalue.identifier.id, numRequiredArgs);
}
}
} else if (value.kind === 'LoadGlobal') {
loadGlobals.add(lvalue.identifier.id);

/*
* TODO: Handle properties on default exports, like
* import React from 'react';
* React.useEffect(...);
*/
if (value.binding.kind === 'ImportNamespace') {
const moduleTargets = autodepFnConfigs.get(value.binding.module);
if (moduleTargets != null) {
autodepModuleLoads.set(lvalue.identifier.id, moduleTargets);
}
}
if (
value.binding.kind === 'ImportSpecifier' ||
value.binding.kind === 'ImportDefault'
Expand All @@ -109,84 +135,88 @@ export function inferEffectDependencies(fn: HIRFunction): void {
}
}
} else if (
/*
* TODO: Handle method calls
*/
value.kind === 'CallExpression' &&
autodepFnLoads.get(value.callee.identifier.id) === value.args.length &&
value.args[0].kind === 'Identifier'
value.kind === 'CallExpression' ||
value.kind === 'MethodCall'
) {
const effectDeps: Array<Place> = [];
const newInstructions: Array<Instruction> = [];
const deps: ArrayExpression = {
kind: 'ArrayExpression',
elements: effectDeps,
loc: GeneratedSource,
};
const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
depsPlace.effect = Effect.Read;
const callee =
value.kind === 'CallExpression' ? value.callee : value.property;
if (
value.args.length === autodepFnLoads.get(callee.identifier.id) &&
value.args[0].kind === 'Identifier'
) {
// We have a useEffect call with no deps array, so we need to infer the deps
const effectDeps: Array<Place> = [];
const newInstructions: Array<Instruction> = [];
const deps: ArrayExpression = {
kind: 'ArrayExpression',
elements: effectDeps,
loc: GeneratedSource,
};
const depsPlace = createTemporaryPlace(fn.env, GeneratedSource);
depsPlace.effect = Effect.Read;

const fnExpr = fnExpressions.get(value.args[0].identifier.id);
if (fnExpr != null) {
// We have a function expression, so we can infer its dependencies
const scopeInfo =
fnExpr.lvalue.identifier.scope != null
? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
: null;
CompilerError.invariant(scopeInfo != null, {
reason: 'Expected function expression scope to exist',
loc: value.loc,
});
if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) {
/**
* TODO: retry pipeline that ensures effect function expressions
* are placed into their own scope
*/
CompilerError.throwTodo({
reason:
'[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction',
loc: fnExpr.loc,
});
}

const fnExpr = fnExpressions.get(value.args[0].identifier.id);
if (fnExpr != null) {
// We have a function expression, so we can infer its dependencies
const scopeInfo =
fnExpr.lvalue.identifier.scope != null
? scopeInfos.get(fnExpr.lvalue.identifier.scope.id)
: null;
CompilerError.invariant(scopeInfo != null, {
reason: 'Expected function expression scope to exist',
loc: value.loc,
});
if (scopeInfo.pruned || !scopeInfo.hasSingleInstr) {
/**
* TODO: retry pipeline that ensures effect function expressions
* are placed into their own scope
* Step 1: push dependencies to the effect deps array
*
* Note that it's invalid to prune non-reactive deps in this pass, see
* the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
* explanation.
*/
CompilerError.throwTodo({
reason:
'[InferEffectDependencies] Expected effect function to have non-pruned scope and its scope to have exactly one instruction',
loc: fnExpr.loc,
for (const dep of scopeInfo.deps) {
const {place, instructions} = writeDependencyToInstructions(
dep,
reactiveIds.has(dep.identifier.id),
fn.env,
fnExpr.loc,
);
newInstructions.push(...instructions);
effectDeps.push(place);
}

newInstructions.push({
id: makeInstructionId(0),
loc: GeneratedSource,
lvalue: {...depsPlace, effect: Effect.Mutate},
value: deps,
});
}

/**
* Step 1: push dependencies to the effect deps array
*
* Note that it's invalid to prune non-reactive deps in this pass, see
* the `infer-effect-deps/pruned-nonreactive-obj` fixture for an
* explanation.
*/
for (const dep of scopeInfo.deps) {
const {place, instructions} = writeDependencyToInstructions(
dep,
reactiveIds.has(dep.identifier.id),
fn.env,
fnExpr.loc,
);
newInstructions.push(...instructions);
effectDeps.push(place);
// Step 2: push the inferred deps array as an argument of the useEffect
value.args.push({...depsPlace, effect: Effect.Freeze});
rewriteInstrs.set(instr.id, newInstructions);
} else if (loadGlobals.has(value.args[0].identifier.id)) {
// Global functions have no reactive dependencies, so we can insert an empty array
newInstructions.push({
id: makeInstructionId(0),
loc: GeneratedSource,
lvalue: {...depsPlace, effect: Effect.Mutate},
value: deps,
});
value.args.push({...depsPlace, effect: Effect.Freeze});
rewriteInstrs.set(instr.id, newInstructions);
}

newInstructions.push({
id: makeInstructionId(0),
loc: GeneratedSource,
lvalue: {...depsPlace, effect: Effect.Mutate},
value: deps,
});

// Step 2: push the inferred deps array as an argument of the useEffect
value.args.push({...depsPlace, effect: Effect.Freeze});
rewriteInstrs.set(instr.id, newInstructions);
} else if (loadGlobals.has(value.args[0].identifier.id)) {
// Global functions have no reactive dependencies, so we can insert an empty array
newInstructions.push({
id: makeInstructionId(0),
loc: GeneratedSource,
lvalue: {...depsPlace, effect: Effect.Mutate},
value: deps,
});
value.args.push({...depsPlace, effect: Effect.Freeze});
rewriteInstrs.set(instr.id, newInstructions);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@

## Input

```javascript
// @inferEffectDependencies
import * as React from 'react';
import * as SharedRuntime from 'shared-runtime';

function NonReactiveDepInEffect() {
const obj = makeObject_Primitives();
React.useEffect(() => print(obj));
SharedRuntime.useSpecialEffect(() => print(obj), [obj]);
}

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
import * as React from "react";
import * as SharedRuntime from "shared-runtime";

function NonReactiveDepInEffect() {
const $ = _c(4);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = makeObject_Primitives();
$[0] = t0;
} else {
t0 = $[0];
}
const obj = t0;
let t1;
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = () => print(obj);
$[1] = t1;
} else {
t1 = $[1];
}
React.useEffect(t1, [obj]);
let t2;
let t3;
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
t2 = () => print(obj);
t3 = [obj];
$[2] = t2;
$[3] = t3;
} else {
t2 = $[2];
t3 = $[3];
}
SharedRuntime.useSpecialEffect(t2, t3, [obj]);
}

```

### Eval output
(kind: exception) Fixture not implemented
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// @inferEffectDependencies
import * as React from 'react';
import * as SharedRuntime from 'shared-runtime';

function NonReactiveDepInEffect() {
const obj = makeObject_Primitives();
React.useEffect(() => print(obj));
SharedRuntime.useSpecialEffect(() => print(obj), [obj]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@

```javascript
// @inferEffectDependencies
import * as React from 'react';
import React from 'react';

/**
* TODO: recognize import namespace
*/
function NonReactiveDepInEffect() {
const obj = makeObject_Primitives();
React.useEffect(() => print(obj));
Expand All @@ -19,11 +16,8 @@ function NonReactiveDepInEffect() {

```javascript
import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
import * as React from "react";
import React from "react";

/**
* TODO: recognize import namespace
*/
function NonReactiveDepInEffect() {
const $ = _c(2);
let t0;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// @inferEffectDependencies
import * as React from 'react';
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh makes sense, thanks for checking the ImportDefault case!


/**
* TODO: recognize import namespace
*/
function NonReactiveDepInEffect() {
const obj = makeObject_Primitives();
React.useEffect(() => print(obj));
Expand Down
Loading