-
Notifications
You must be signed in to change notification settings - Fork 47.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[compiler] Repro of missing memoization due to capturing w/o mutation
If you have a function expression which _captures_ a mutable value (but does not mutate it), and that function is invoked during render, we infer the invocation as a mutation of the captured value. But in some circumstances we can prove that the captured value cannot have been mutated, and could in theory avoid inferring a mutation. ghstack-source-id: 47664e48ce8c51a6edf4d714d1acd1ec4781df80 Pull Request resolved: #30783
- Loading branch information
1 parent
689c6bd
commit 8a20fc3
Showing
2 changed files
with
87 additions
and
0 deletions.
There are no files selected for viewing
54 changes: 54 additions & 0 deletions
54
...sed-memoization-from-capture-in-invoked-function-inferred-as-mutation.expect.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
|
||
## Input | ||
|
||
```javascript | ||
// @flow @validatePreserveExistingMemoizationGuarantees | ||
import {useMemo} from 'react'; | ||
import {logValue, useFragment, useHook, typedLog} from 'shared-runtime'; | ||
|
||
component Component() { | ||
const data = useFragment(); | ||
|
||
const getIsEnabled = () => { | ||
if (data != null) { | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
}; | ||
|
||
// We infer that getIsEnabled returns a mutable value, such that | ||
// isEnabled is mutable | ||
const isEnabled = useMemo(() => getIsEnabled(), [getIsEnabled]); | ||
|
||
// We then infer getLoggingData as capturing that mutable value, | ||
// so any calls to this function are then inferred as extending | ||
// the mutable range of isEnabled | ||
const getLoggingData = () => { | ||
return { | ||
isEnabled, | ||
}; | ||
}; | ||
|
||
// The call here is then inferred as an indirect mutation of isEnabled | ||
useHook(getLoggingData()); | ||
|
||
return <div onClick={() => typedLog(getLoggingData())} />; | ||
} | ||
|
||
``` | ||
|
||
|
||
## Error | ||
|
||
``` | ||
16 | // We infer that getIsEnabled returns a mutable value, such that | ||
17 | // isEnabled is mutable | ||
> 18 | const isEnabled = useMemo(() => getIsEnabled(), [getIsEnabled]); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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. (18:18) | ||
19 | | ||
20 | // We then infer getLoggingData as capturing that mutable value, | ||
21 | // so any calls to this function are then inferred as extending | ||
``` | ||
33 changes: 33 additions & 0 deletions
33
...or.todo-repro-missed-memoization-from-capture-in-invoked-function-inferred-as-mutation.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// @flow @validatePreserveExistingMemoizationGuarantees | ||
import {useMemo} from 'react'; | ||
import {logValue, useFragment, useHook, typedLog} from 'shared-runtime'; | ||
|
||
component Component() { | ||
const data = useFragment(); | ||
|
||
const getIsEnabled = () => { | ||
if (data != null) { | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
}; | ||
|
||
// We infer that getIsEnabled returns a mutable value, such that | ||
// isEnabled is mutable | ||
const isEnabled = useMemo(() => getIsEnabled(), [getIsEnabled]); | ||
|
||
// We then infer getLoggingData as capturing that mutable value, | ||
// so any calls to this function are then inferred as extending | ||
// the mutable range of isEnabled | ||
const getLoggingData = () => { | ||
return { | ||
isEnabled, | ||
}; | ||
}; | ||
|
||
// The call here is then inferred as an indirect mutation of isEnabled | ||
useHook(getLoggingData()); | ||
|
||
return <div onClick={() => typedLog(getLoggingData())} />; | ||
} |