Skip to content

Commit

Permalink
Update on "compiler: treat pruned scope outputs as reactive"
Browse files Browse the repository at this point in the history
Mostly addresses the issue with non-reactive pruned scopes. Before, values from pruned scopes would not be memoized, but could still be depended upon by downstream scopes. However, those downstream scopes would assume the value could never change. This could allow the developer to observe two different versions of a value - the freshly created one (if observed outside a scope) or a cached one (if observed inside, or through) a scope which used the value but didn't depend on it.

The fix here is to consider the outputs of pruned reactive scopes as reactive. Note that this is a partial fix because of things like control variables — the full solution would be to mark these values as reactive, and then re-run InferReactivePlaces. We can do this once we've fully converted our pipeline to use HIR everywhere. For now, this should fix most issues in practice because PruneNonReactiveDependencies already does basic alias tracking (see new fixture).

[ghstack-poisoned]
  • Loading branch information
josephsavona committed Jun 7, 2024
2 parents 2fef771 + d4f4f98 commit 0a5761c
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 0a5761c

Please sign in to comment.