Skip to content

Commit

Permalink
[compiler] Allow different dependencies from explicit memoization whe…
Browse files Browse the repository at this point in the history 
…n dependency is a ref

Summary:
In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source.

ghstack-source-id: 685d859d1eed5d1e19dbbbfadc75be3875ddb6ea
Pull Request resolved: facebook#30679
  • Loading branch information
@mvitousek
mvitousek committed Aug 14, 2024
1 parent 5c9243d commit 179197a
Show file tree
Hide file tree
Showing 29 changed files with 306 additions and 310 deletions.
7 changes: 6 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,12 @@ export function isDispatcherType(id: Identifier): boolean {
}

export function isStableType(id: Identifier): boolean {
return isSetStateType(id) || isSetActionStateType(id) || isDispatcherType(id);
return (
isSetStateType(id) ||
isSetActionStateType(id) ||
isDispatcherType(id) ||
isUseRefType(id)
);
}

export function isUseEffectHookType(id: Identifier): boolean {
Expand Down
53 changes: 14 additions & 39 deletions ...xtures/compiler/allow-mutating-ref-in-callback-passed-to-jsx-indirect.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,62 +40,37 @@ import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRen
import { useRef } from "react";

function Component() {
const $ = _c(10);
const $ = _c(2);
const ref = useRef(null);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
const setRef = () => {
if (ref.current !== null) {
ref.current = "";
}
};

t0 = () => {
setRef();
};
$[0] = t0;
} else {
t0 = $[0];
}
const setRef = t0;
const onClick = 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 = (
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = (
<>
{t2}
{t3}
<input ref={ref} />
<button onClick={onClick} />
</>
);
$[7] = t2;
$[8] = t3;
$[9] = t4;
$[1] = t1;
} else {
t4 = $[9];
t1 = $[1];
}
return t4;
return t1;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
34 changes: 8 additions & 26 deletions ...ests__/fixtures/compiler/allow-mutating-ref-in-callback-passed-to-jsx.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRen
import { useRef } from "react";

function Component() {
const $ = _c(8);
const $ = _c(2);
const ref = useRef(null);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
Expand All @@ -51,36 +51,18 @@ function Component() {
}
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 = (
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = (
<>
{t1}
{t2}
<input ref={ref} />
<button onClick={onClick} />
</>
);
$[5] = t1;
$[6] = t2;
$[7] = t3;
$[1] = t1;
} else {
t3 = $[7];
t1 = $[1];
}
return t3;
return t1;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
53 changes: 14 additions & 39 deletions ...mpiler/allow-mutating-ref-property-in-callback-passed-to-jsx-indirect.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,62 +40,37 @@ import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRen
import { useRef } from "react";

function Component() {
const $ = _c(10);
const $ = _c(2);
const ref = useRef(null);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = () => {
const setRef = () => {
if (ref.current !== null) {
ref.current.value = "";
}
};

t0 = () => {
setRef();
};
$[0] = t0;
} else {
t0 = $[0];
}
const setRef = t0;
const onClick = 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 = (
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = (
<>
{t2}
{t3}
<input ref={ref} />
<button onClick={onClick} />
</>
);
$[7] = t2;
$[8] = t3;
$[9] = t4;
$[1] = t1;
} else {
t4 = $[9];
t1 = $[1];
}
return t4;
return t1;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
34 changes: 8 additions & 26 deletions ...xtures/compiler/allow-mutating-ref-property-in-callback-passed-to-jsx.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRen
import { useRef } from "react";

function Component() {
const $ = _c(8);
const $ = _c(2);
const ref = useRef(null);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
Expand All @@ -51,36 +51,18 @@ function Component() {
}
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 = (
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = (
<>
{t1}
{t2}
<input ref={ref} />
<button onClick={onClick} />
</>
);
$[5] = t1;
$[6] = t2;
$[7] = t3;
$[1] = t1;
} else {
t3 = $[7];
t1 = $[1];
}
return t3;
return t1;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
9 changes: 4 additions & 5 deletions ...-compiler/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@ function Component(props) {
```javascript
import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(2);
const $ = _c(1);
const ref = useRef(null);
let t0;
if ($[0] !== ref) {
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = <Foo ref={ref} />;
$[0] = ref;
$[1] = t0;
$[0] = t0;
} else {
t0 = $[1];
t0 = $[0];
}
return t0;
}
Expand Down
37 changes: 16 additions & 21 deletions ...r/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRen
import { useCallback, useEffect, useRef, useState } from "react";

function Component() {
const $ = _c(9);
const $ = _c(7);
const ref = useRef(null);
const [state, setState] = useState(false);
let t0;
Expand All @@ -60,47 +60,42 @@ function Component() {
}
const setRef = t0;
let t1;
if ($[1] !== setRef) {
let t2;
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
t1 = () => {
setRef();
};
$[1] = setRef;
$[2] = t1;
} else {
t1 = $[2];
}
let t2;
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
t2 = [];
$[3] = t2;
$[1] = t1;
$[2] = t2;
} else {
t2 = $[3];
t1 = $[1];
t2 = $[2];
}
useEffect(t1, t2);
let t3;
let t4;
if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
t3 = () => {
setState(true);
};
t4 = [];
$[4] = t3;
$[5] = t4;
$[3] = t3;
$[4] = t4;
} else {
t3 = $[4];
t4 = $[5];
t3 = $[3];
t4 = $[4];
}
useEffect(t3, t4);

const t5 = String(state);
let t6;
if ($[6] !== t5 || $[7] !== ref) {
if ($[5] !== t5) {
t6 = <Child key={t5} ref={ref} />;
$[6] = t5;
$[7] = ref;
$[8] = t6;
$[5] = t5;
$[6] = t6;
} else {
t6 = $[8];
t6 = $[6];
}
return t6;
}
Expand Down
9 changes: 4 additions & 5 deletions ...t-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRen
import { useEffect, useRef, useState } from "react";

function Component() {
const $ = _c(7);
const $ = _c(6);
const ref = useRef(null);
const [state, setState] = useState(false);
let t0;
Expand Down Expand Up @@ -76,13 +76,12 @@ function Component() {

const t4 = String(state);
let t5;
if ($[4] !== t4 || $[5] !== ref) {
if ($[4] !== t4) {
t5 = <Child key={t4} ref={ref} />;
$[4] = t4;
$[5] = ref;
$[6] = t5;
$[5] = t5;
} else {
t5 = $[6];
t5 = $[5];
}
return t5;
}
Expand Down
Loading

0 comments on commit 179197a

Please sign in to comment.