Skip to content

Commit

Permalink
Update base for Update on "compiler: Promote pruned scope declaration…
Browse files Browse the repository at this point in the history
…s to temporaries if used in a later scope"

There's a category of bug currently where pruned reactive scopes whose outputs are non-reactive can have their code end up inlining into another scope, moving the location of the instruction. Any value that had a scope assigned has to have its order of evaluation preserved, despite the fact that it got pruned, so naively we could just force every pruned scope to have its declarations promoted to named variables.

However, that ends up assigning names to _tons_ of scope declarations that don't really need to be promoted. For example, a scope with just a hook call ends up with:

```
const x = useFoo();

=>

scope {
 $t0 = Call read useFoo$ (...);
}
$t1 = StoreLocal 'x' = read $t0;
```

Where t0 doesn't need to be promoted since it's used immediately to assign to another value which is a non-temporary.

So the idea of this PR is that we can track outputs of pruned scopes which are directly referenced from inside a later scope. This fixes one of the two cases of the above pattern. We'll also likely have to consider values from pruned scopes as always reactive, i'll do that in the next PR.

[ghstack-poisoned]
  • Loading branch information
josephsavona committed Jun 7, 2024
2 parents 4eb4628 + a0a435d commit efa8e6a
Show file tree
Hide file tree
Showing 29 changed files with 813 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import {
isArrayType,
isMutableEffect,
isObjectType,
isRefValueType,
isUseRefType,
} from "../HIR/HIR";
import { FunctionSignature } from "../HIR/ObjectShape";
import {
Expand Down Expand Up @@ -521,7 +523,12 @@ class InferenceState {
break;
}
case Effect.Mutate: {
if (valueKind.kind === ValueKind.Context) {
if (
isRefValueType(place.identifier) ||
isUseRefType(place.identifier)
) {
// no-op: refs are validate via ValidateNoRefAccessInRender
} else if (valueKind.kind === ValueKind.Context) {
functionEffect = {
kind: "ContextMutation",
loc: place.loc,
Expand Down Expand Up @@ -560,7 +567,12 @@ class InferenceState {
break;
}
case Effect.Store: {
if (valueKind.kind === ValueKind.Context) {
if (
isRefValueType(place.identifier) ||
isUseRefType(place.identifier)
) {
// no-op: refs are validate via ValidateNoRefAccessInRender
} else if (valueKind.kind === ValueKind.Context) {
functionEffect = {
kind: "ContextMutation",
loc: place.loc,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@

## Input

```javascript
// @validateRefAccessDuringRender
import { useRef } from "react";

function Component() {
const ref = useRef(null);

const setRef = () => {
if (ref.current !== null) {
ref.current = "";
}
};

const onClick = () => {
setRef();
};

return (
<>
<input ref={ref} />
<button onClick={onClick} />
</>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender
import { useRef } from "react";

function Component() {
const $ = _c(10);
const ref = useRef(null);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
if (ref.current !== null) {
ref.current = "";
}
};
$[0] = t0;
} else {
t0 = $[0];
}
const setRef = t0;
let t1;
if ($[1] !== setRef) {
t1 = () => {
setRef();
};
$[1] = setRef;
$[2] = t1;
} else {
t1 = $[2];
}
const onClick = t1;
let t2;
if ($[3] !== ref) {
t2 = <input ref={ref} />;
$[3] = ref;
$[4] = t2;
} else {
t2 = $[4];
}
let t3;
if ($[5] !== onClick) {
t3 = <button onClick={onClick} />;
$[5] = onClick;
$[6] = t3;
} else {
t3 = $[6];
}
let t4;
if ($[7] !== t2 || $[8] !== t3) {
t4 = (
<>
{t2}
{t3}
</>
);
$[7] = t2;
$[8] = t3;
$[9] = t4;
} else {
t4 = $[9];
}
return t4;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};

```
### Eval output
(kind: ok) <input><button></button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// @validateRefAccessDuringRender
import { useRef } from "react";

function Component() {
const ref = useRef(null);

const setRef = () => {
if (ref.current !== null) {
ref.current = "";
}
};

const onClick = () => {
setRef();
};

return (
<>
<input ref={ref} />
<button onClick={onClick} />
</>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@

## Input

```javascript
// @validateRefAccessDuringRender
import { useRef } from "react";

function Component() {
const ref = useRef(null);

const onClick = () => {
if (ref.current !== null) {
ref.current = "";
}
};

return (
<>
<input ref={ref} />
<button onClick={onClick} />
</>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender
import { useRef } from "react";

function Component() {
const $ = _c(8);
const ref = useRef(null);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
if (ref.current !== null) {
ref.current = "";
}
};
$[0] = t0;
} else {
t0 = $[0];
}
const onClick = t0;
let t1;
if ($[1] !== ref) {
t1 = <input ref={ref} />;
$[1] = ref;
$[2] = t1;
} else {
t1 = $[2];
}
let t2;
if ($[3] !== onClick) {
t2 = <button onClick={onClick} />;
$[3] = onClick;
$[4] = t2;
} else {
t2 = $[4];
}
let t3;
if ($[5] !== t1 || $[6] !== t2) {
t3 = (
<>
{t1}
{t2}
</>
);
$[5] = t1;
$[6] = t2;
$[7] = t3;
} else {
t3 = $[7];
}
return t3;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};

```
### Eval output
(kind: ok) <input><button></button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// @validateRefAccessDuringRender
import { useRef } from "react";

function Component() {
const ref = useRef(null);

const onClick = () => {
if (ref.current !== null) {
ref.current = "";
}
};

return (
<>
<input ref={ref} />
<button onClick={onClick} />
</>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};
Loading

0 comments on commit efa8e6a

Please sign in to comment.