mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
[compiler] Repro for missing memoization due to inferred mutation
This fixture bails out on ValidatePreserveExistingMemo but would ideally memoize since the original memoization is safe. It's trivial to make it pass by commenting out the commented line (`LogEvent.log(() => object)`). I would expect the compiler to infer this as possible mutation of `logData`, since `object` captures a reference to `logData`. But somehow `logData` is getting memoized successfully, but we still infer the callback, `setCurrentIndex`, as having a mutable range that extends to the `setCurrentIndex()` call after the useCallback. [ghstack-poisoned]
This commit is contained in:
+9
-3
@@ -5,7 +5,7 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {CompilerError, Effect, ErrorSeverity} from '..';
|
||||
import {CompilerError, Effect, ErrorSeverity, printReactiveFunction} from '..';
|
||||
import {
|
||||
DeclarationId,
|
||||
GeneratedSource,
|
||||
@@ -23,7 +23,11 @@ import {
|
||||
ScopeId,
|
||||
SourceLocation,
|
||||
} from '../HIR';
|
||||
import {printManualMemoDependency} from '../HIR/PrintHIR';
|
||||
import {
|
||||
printFunction,
|
||||
printIdentifier,
|
||||
printManualMemoDependency,
|
||||
} from '../HIR/PrintHIR';
|
||||
import {eachInstructionValueOperand} from '../HIR/visitors';
|
||||
import {collectMaybeMemoDependencies} from '../Inference/DropManualMemoization';
|
||||
import {
|
||||
@@ -537,7 +541,9 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
|
||||
state.errors.push({
|
||||
reason:
|
||||
'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.',
|
||||
description: null,
|
||||
description: DEBUG
|
||||
? `${printIdentifier(identifier)} was not memoized`
|
||||
: null,
|
||||
severity: ErrorSeverity.CannotPreserveMemoization,
|
||||
loc,
|
||||
suggestions: null,
|
||||
|
||||
+83
@@ -0,0 +1,83 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @flow @validatePreserveExistingMemoizationGuarantees
|
||||
import {useFragment} from 'react-relay';
|
||||
import LogEvent from 'LogEvent';
|
||||
import {useCallback, useMemo} from 'react';
|
||||
|
||||
component Component(id) {
|
||||
const {data} = useFragment();
|
||||
const items = data.items.edges;
|
||||
|
||||
const [prevId, setPrevId] = useState(id);
|
||||
const [index, setIndex] = useState(0);
|
||||
|
||||
const logData = useMemo(() => {
|
||||
const item = items[index];
|
||||
return {
|
||||
key: item.key ?? '',
|
||||
};
|
||||
}, [index, items]);
|
||||
|
||||
const setCurrentIndex = useCallback(
|
||||
(index: number) => {
|
||||
const object = {
|
||||
tracking: logData.key,
|
||||
};
|
||||
// We infer that this may mutate `object`, which in turn aliases
|
||||
// data from `logData`, such that `logData` may be mutated.
|
||||
LogEvent.log(() => object);
|
||||
setIndex(index);
|
||||
},
|
||||
[index, logData, items]
|
||||
);
|
||||
|
||||
if (prevId !== id) {
|
||||
setPrevId(id);
|
||||
setCurrentIndex(0);
|
||||
}
|
||||
|
||||
return (
|
||||
<Foo
|
||||
index={index}
|
||||
items={items}
|
||||
current={mediaList[index]}
|
||||
setCurrentIndex={setCurrentIndex}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
19 |
|
||||
20 | const setCurrentIndex = useCallback(
|
||||
> 21 | (index: number) => {
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
> 22 | const object = {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 23 | tracking: logData.key,
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 24 | };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 25 | // We infer that this may mutate `object`, which in turn aliases
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 26 | // data from `logData`, such that `logData` may be mutated.
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 27 | LogEvent.log(() => object);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 28 | setIndex(index);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
> 29 | },
|
||||
| ^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. (21:29)
|
||||
30 | [index, logData, items]
|
||||
31 | );
|
||||
32 |
|
||||
```
|
||||
|
||||
|
||||
+46
@@ -0,0 +1,46 @@
|
||||
// @flow @validatePreserveExistingMemoizationGuarantees
|
||||
import {useFragment} from 'react-relay';
|
||||
import LogEvent from 'LogEvent';
|
||||
import {useCallback, useMemo} from 'react';
|
||||
|
||||
component Component(id) {
|
||||
const {data} = useFragment();
|
||||
const items = data.items.edges;
|
||||
|
||||
const [prevId, setPrevId] = useState(id);
|
||||
const [index, setIndex] = useState(0);
|
||||
|
||||
const logData = useMemo(() => {
|
||||
const item = items[index];
|
||||
return {
|
||||
key: item.key ?? '',
|
||||
};
|
||||
}, [index, items]);
|
||||
|
||||
const setCurrentIndex = useCallback(
|
||||
(index: number) => {
|
||||
const object = {
|
||||
tracking: logData.key,
|
||||
};
|
||||
// We infer that this may mutate `object`, which in turn aliases
|
||||
// data from `logData`, such that `logData` may be mutated.
|
||||
LogEvent.log(() => object);
|
||||
setIndex(index);
|
||||
},
|
||||
[index, logData, items]
|
||||
);
|
||||
|
||||
if (prevId !== id) {
|
||||
setPrevId(id);
|
||||
setCurrentIndex(0);
|
||||
}
|
||||
|
||||
return (
|
||||
<Foo
|
||||
index={index}
|
||||
items={items}
|
||||
current={mediaList[index]}
|
||||
setCurrentIndex={setCurrentIndex}
|
||||
/>
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user