Compare commits

...

8 Commits

Author SHA1 Message Date
Mike Vitousek
a14020c203 Update base for Update on "[compiler] In change detection, assign reactive scopes to values that may have changed between renders"
Summary:
Change detection is desgined to determine whether rules of react have been violated, and to do so better we need to loosen Forgets assumptions about what kinds of values don't need to be memoized. For example, the compiler typically doesn't think of o.x as something that needs to be memoized, because it does not allocate. However, we want to compare o.x in the current render with o.x in a previous one, so we now insert a "memoization" (comparison, really) block around this value.

The structure of this work is that we now add a reactive scope for identifiers if they originate from any instruction that could read from state that could have mutated between renders. This means that `LoadProperty` is always going to get a reactive scope; `LoadGlobal` will get a scope if we're not reading from something mutable, and `PrefixUpdate` won't (because the variable being incremented would have already been loaded and checked).

Supersedes #30180.

[ghstack-poisoned]
2024-07-26 15:52:16 -07:00
Mike Vitousek
e13113a352 Update base for Update on "[compiler] In change detection, assign reactive scopes to values that may have changed between renders"
Summary:
Change detection is desgined to determine whether rules of react have been violated, and to do so better we need to loosen Forgets assumptions about what kinds of values don't need to be memoized. For example, the compiler typically doesn't think of o.x as something that needs to be memoized, because it does not allocate. However, we want to compare o.x in the current render with o.x in a previous one, so we now insert a "memoization" (comparison, really) block around this value.

The structure of this work is that we now add a reactive scope for identifiers if they originate from any instruction that could read from state that could have mutated between renders. This means that `LoadProperty` is always going to get a reactive scope; `LoadGlobal` will get a scope if we're not reading from something mutable, and `PrefixUpdate` won't (because the variable being incremented would have already been loaded and checked).

Supersedes #30180.

[ghstack-poisoned]
2024-07-26 15:16:25 -07:00
Mike Vitousek
f148e71f27 Update base for Update on "[compiler] In change detection, assign reactive scopes to values that may have changed between renders"
Summary:
Change detection is desgined to determine whether rules of react have been violated, and to do so better we need to loosen Forgets assumptions about what kinds of values don't need to be memoized. For example, the compiler typically doesn't think of o.x as something that needs to be memoized, because it does not allocate. However, we want to compare o.x in the current render with o.x in a previous one, so we now insert a "memoization" (comparison, really) block around this value.

The structure of this work is that we now add a reactive scope for identifiers if they originate from any instruction that could read from state that could have mutated between renders. This means that `LoadProperty` is always going to get a reactive scope; `LoadGlobal` will get a scope if we're not reading from something mutable, and `PrefixUpdate` won't (because the variable being incremented would have already been loaded and checked).

Supersedes #30180.

[ghstack-poisoned]
2024-07-26 14:39:20 -07:00
Mike Vitousek
f969e4f1af Update base for Update on "[compiler] In change detection, assign reactive scopes to values that may have changed between renders"
Summary:
Change detection is desgined to determine whether rules of react have been violated, and to do so better we need to loosen Forgets assumptions about what kinds of values don't need to be memoized. For example, the compiler typically doesn't think of o.x as something that needs to be memoized, because it does not allocate. However, we want to compare o.x in the current render with o.x in a previous one, so we now insert a "memoization" (comparison, really) block around this value.

The structure of this work is that we now add a reactive scope for identifiers if they originate from any instruction that could read from state that could have mutated between renders. This means that `LoadProperty` is always going to get a reactive scope; `LoadGlobal` will get a scope if we're not reading from something mutable, and `PrefixUpdate` won't (because the variable being incremented would have already been loaded and checked).

Supersedes #30180.

[ghstack-poisoned]
2024-07-18 16:14:17 -07:00
Mike Vitousek
d27679d2a6 Update base for Update on "[compiler] In change detection, assign reactive scopes to values that may have changed between renders"
Summary:
Change detection is desgined to determine whether rules of react have been violated, and to do so better we need to loosen Forgets assumptions about what kinds of values don't need to be memoized. For example, the compiler typically doesn't think of o.x as something that needs to be memoized, because it does not allocate. However, we want to compare o.x in the current render with o.x in a previous one, so we now insert a "memoization" (comparison, really) block around this value.

The structure of this work is that we now add a reactive scope for identifiers if they originate from any instruction that could read from state that could have mutated between renders. This means that `LoadProperty` is always going to get a reactive scope; `LoadGlobal` will get a scope if we're not reading from something mutable, and `PrefixUpdate` won't (because the variable being incremented would have already been loaded and checked).

Supersedes #30180.

[ghstack-poisoned]
2024-07-18 13:30:40 -07:00
Mike Vitousek
12fef0b5fa Update base for Update on "[compiler] In change detection, assign reactive scopes to values that may have changed between renders"
Summary:
Change detection is desgined to determine whether rules of react have been violated, and to do so better we need to loosen Forgets assumptions about what kinds of values don't need to be memoized. For example, the compiler typically doesn't think of o.x as something that needs to be memoized, because it does not allocate. However, we want to compare o.x in the current render with o.x in a previous one, so we now insert a "memoization" (comparison, really) block around this value.

The structure of this work is that we now add a reactive scope for identifiers if they originate from any instruction that could read from state that could have mutated between renders. This means that `LoadProperty` is always going to get a reactive scope; `LoadGlobal` will get a scope if we're not reading from something mutable, and `PrefixUpdate` won't (because the variable being incremented would have already been loaded and checked).

Supersedes #30180.

[ghstack-poisoned]
2024-07-18 09:41:57 -07:00
Mike Vitousek
ffd72dde5f [compiler] Make compiler aware of whether a local binding is constlike or not
Summary:
Minor change which supports the changeDetection work later on. We can know whether an individual local binding is const or constlike.

[ghstack-poisoned]
2024-07-18 09:17:57 -07:00
Mike Vitousek
26a95b10a1 [compiler] When preserving user-level useMemos, don't memoize arguments
Summary:
This work enhances the don't-drop-memoization mode to not memoize the arguments to useMemo and useCallback (or technically, to memoize them in the same block as the hook call -- but since hooks can't be memoized, this is equivalent to never memoizing them). The advantage here is that we preserve the source-level code structure around memoization callbacks more, and avoid the compiler bug of having function expression dependencies be unconditional when their actual semantics are conditional.

This conceptually replaces some of the work of #30177: some of that was motivated by avoiding the deps bug, and the rest of the motivation was to better exercise the compiler's semantics, which was really happening as much as I'd hoped, as @mofeiZ explained to me in that PR and offline!

[ghstack-poisoned]
2024-07-18 09:17:50 -07:00
7 changed files with 28 additions and 48 deletions

View File

@@ -155,11 +155,7 @@ function* runWithEnvironment(
validateContextVariableLValues(hir);
validateUseMemo(hir);
if (
!env.config.enablePreserveExistingManualUseMemo &&
!env.config.disableMemoizationForDebugging &&
!env.config.enableChangeDetectionForDebugging
) {
if (!env.preserveManualMemo()) {
dropManualMemoization(hir);
yield log({kind: 'hir', name: 'DropManualMemoization', value: hir});
}

View File

@@ -765,6 +765,14 @@ export class Environment {
return DefaultMutatingHook;
}
}
preserveManualMemo(): boolean {
return (
this.config.enablePreserveExistingManualUseMemo ||
this.config.disableMemoizationForDebugging ||
this.config.enableChangeDetectionForDebugging != null
);
}
}
// From https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js#LL18C1-L23C2

View File

@@ -1168,7 +1168,7 @@ export type NonLocalBinding =
imported: string;
}
// let, const, function, etc declared in the module but outside the current component/hook
| {kind: 'ModuleLocal'; name: string}
| {kind: 'ModuleLocal'; name: string, immutable: boolean}
// an unresolved binding
| {kind: 'Global'; name: string};

View File

@@ -271,9 +271,14 @@ export default class HIRBuilder {
module: importDeclaration.node.source.value,
};
} else {
const immutable =
(path.isVariableDeclaration() && path.node.kind === "const") ||
path.isClassDeclaration() ||
path.isClassExpression();
return {
kind: 'ModuleLocal',
name: originalName,
immutable,
};
}
}

View File

@@ -45,6 +45,7 @@ export function memoizeFbtAndMacroOperandsInSameScope(
const fbtMacroTags = new Set([
...FBT_TAGS,
...(fn.env.config.customMacros ?? []),
...(fn.env.preserveManualMemo() ? ["useMemo", "useCallback"] : []),
]);
const fbtValues: Set<IdentifierId> = new Set();
while (true) {

View File

@@ -25,33 +25,18 @@ import { c as _c } from "react/compiler-runtime"; // @disableMemoizationForDebug
import { useMemo } from "react";
function Component(t0) {
const $ = _c(5);
const $ = _c(2);
const { a } = t0;
const x = useMemo(() => [a], []);
let t1;
if ($[0] !== a || true) {
t1 = () => [a];
$[0] = a;
if ($[0] !== x || true) {
t1 = <div>{x}</div>;
$[0] = x;
$[1] = t1;
} else {
t1 = $[1];
}
let t2;
if ($[2] === Symbol.for("react.memo_cache_sentinel") || true) {
t2 = [];
$[2] = t2;
} else {
t2 = $[2];
}
const x = useMemo(t1, t2);
let t3;
if ($[3] !== x || true) {
t3 = <div>{x}</div>;
$[3] = x;
$[4] = t3;
} else {
t3 = $[4];
}
return t3;
return t1;
}
export const FIXTURE_ENTRYPOINT = {

View File

@@ -25,33 +25,18 @@ import { c as _c } from "react/compiler-runtime"; // @enablePreserveExistingManu
import { useMemo } from "react";
function Component(t0) {
const $ = _c(5);
const $ = _c(2);
const { a } = t0;
const x = useMemo(() => [a], []);
let t1;
if ($[0] !== a) {
t1 = () => [a];
$[0] = a;
if ($[0] !== x) {
t1 = <div>{x}</div>;
$[0] = x;
$[1] = t1;
} else {
t1 = $[1];
}
let t2;
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
t2 = [];
$[2] = t2;
} else {
t2 = $[2];
}
const x = useMemo(t1, t2);
let t3;
if ($[3] !== x) {
t3 = <div>{x}</div>;
$[3] = x;
$[4] = t3;
} else {
t3 = $[4];
}
return t3;
return t1;
}
export const FIXTURE_ENTRYPOINT = {