Skip to content

Commit

Permalink
[compiler] Rewrite effect dep arrays that use fire
Browse files Browse the repository at this point in the history
If an effect uses a dep array, also rewrite the dep array to use the fire binding

--
  • Loading branch information
jbrown215 committed Dec 16, 2024
1 parent c8d53fc commit e4c6a1c
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
SourceLocation,
} from '..';
import {
ArrayExpression,
CallExpression,
Effect,
Environment,
Expand All @@ -38,7 +39,6 @@ import {printSourceLocationLine} from '../HIR/PrintHIR';

/*
* TODO(jmbrown):
* - rewrite dep arrays
* - traverse object methods
* - method calls
*/
Expand Down Expand Up @@ -246,6 +246,12 @@ class Context {
*/
#loadGlobalInstructionIds = new Map<IdentifierId, InstructionId>();

/*
* We keep track of array expressions so we can rewrite dependency arrays passed to useEffect
* to use the fire functions
*/
#arrayExpressions = new Map<IdentifierId, ArrayExpression>();

pushError(error: CompilerErrorDetailOptions): void {
this.#errors.push(error);
}
Expand Down Expand Up @@ -354,6 +360,14 @@ class Context {
return this.#loadGlobalInstructionIds.get(id);
}

addArrayExpression(id: IdentifierId, array: ArrayExpression): void {
this.#arrayExpressions.set(id, array);
}

getArrayExpression(id: IdentifierId): ArrayExpression | undefined {
return this.#arrayExpressions.get(id);
}

hasErrors(): boolean {
return this.#errors.hasErrors();
}
Expand Down Expand Up @@ -527,6 +541,44 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
context,
capturedCallees,
);

if (
value.args.length > 1 &&
value.args[1] != null &&
value.args[1].kind === 'Identifier'
) {
const depArray = value.args[1];
const depArrayExpression = context.getArrayExpression(
depArray.identifier.id,
);
if (depArrayExpression != null) {
for (const dependency of depArrayExpression.elements) {
if (dependency.kind === 'Identifier') {
const loadOfDependency = context.getLoadLocalInstr(
dependency.identifier.id,
);
if (loadOfDependency != null) {
const replacedDepArrayItem = capturedCallees.get(
loadOfDependency.place.identifier.id,
);
if (replacedDepArrayItem != null) {
loadOfDependency.place =
replacedDepArrayItem.fireFunctionBinding;
}
}
}
}
} else {
context.pushError({
loc: value.loc,
description:
'You must use an array literal for an effect dependency array when that effect uses `fire()`',
severity: ErrorSeverity.Invariant,
reason: CANNOT_COMPILE_FIRE,
suggestions: null,
});
}
}
}
} else if (
value.kind === 'CallExpression' &&
Expand Down Expand Up @@ -628,6 +680,8 @@ function replaceFireFunctions(fn: HIRFunction, context: Context): void {
deleteInstrs.add(instr.id);
} else if (value.kind === 'LoadGlobal') {
context.addLoadGlobalInstrId(lvalue.identifier.id, instr.id);
} else if (value.kind === 'ArrayExpression') {
context.addArrayExpression(lvalue.identifier.id, value);
}
}
block.instructions = rewriteInstructions(rewriteInstrs, block.instructions);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

## Input

```javascript
// @enableFire
import {fire} from 'react';

function Component(props) {
const foo = props => {
console.log(props);
};

const deps = [foo, props];

useEffect(() => {
fire(foo(props));
}, deps);

return null;
}

```


## Error

```
9 | const deps = [foo, props];
10 |
> 11 | useEffect(() => {
| ^^^^^^^^^^^^^^^^^
> 12 | fire(foo(props));
| ^^^^^^^^^^^^^^^^^^^^^
> 13 | }, deps);
| ^^^^^^^^^^^ Invariant: Cannot compile `fire`. You must use an array literal for an effect dependency array when that effect uses `fire()` (11:13)
14 |
15 | return null;
16 | }
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @enableFire
import {fire} from 'react';

function Component(props) {
const foo = props => {
console.log(props);
};

const deps = [foo, props];

useEffect(() => {
fire(foo(props));
}, deps);

return null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@

## Input

```javascript
// @enableFire @inferEffectDependencies
import {fire, useEffect} from 'react';

function Component(props) {
const foo = arg => {
console.log(arg, props.bar);
};
useEffect(() => {
fire(foo(props));
});

return null;
}

```

## Code

```javascript
import { useFire } from "react";
import { c as _c } from "react/compiler-runtime"; // @enableFire @inferEffectDependencies
import { fire, useEffect } from "react";

function Component(props) {
const $ = _c(5);
let t0;
if ($[0] !== props.bar) {
t0 = (arg) => {
console.log(arg, props.bar);
};
$[0] = props.bar;
$[1] = t0;
} else {
t0 = $[1];
}
const foo = t0;
const t1 = useFire(foo);
let t2;
if ($[2] !== props || $[3] !== t1) {
t2 = () => {
t1(props);
};
$[2] = props;
$[3] = t1;
$[4] = t2;
} else {
t2 = $[4];
}
useEffect(t2, [t1, props]);
return null;
}

```
### Eval output
(kind: exception) Fixture not implemented
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @enableFire @inferEffectDependencies
import {fire, useEffect} from 'react';

function Component(props) {
const foo = arg => {
console.log(arg, props.bar);
};
useEffect(() => {
fire(foo(props));
});

return null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@

## Input

```javascript
// @enableFire
import {fire} from 'react';

function Component(props) {
const foo = props => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
}, [foo, props]);

return null;
}

```

## Code

```javascript
import { useFire } from "react";
import { c as _c } from "react/compiler-runtime"; // @enableFire
import { fire } from "react";

function Component(props) {
const $ = _c(4);
const foo = _temp;
const t0 = useFire(foo);
let t1;
let t2;
if ($[0] !== props || $[1] !== t0) {
t1 = () => {
t0(props);
};
t2 = [t0, props];
$[0] = props;
$[1] = t0;
$[2] = t1;
$[3] = t2;
} else {
t1 = $[2];
t2 = $[3];
}
useEffect(t1, t2);
return null;
}
function _temp(props_0) {
console.log(props_0);
}

```
### Eval output
(kind: exception) Fixture not implemented
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @enableFire
import {fire} from 'react';

function Component(props) {
const foo = props => {
console.log(props);
};
useEffect(() => {
fire(foo(props));
}, [foo, props]);

return null;
}

0 comments on commit e4c6a1c

Please sign in to comment.