-
Notifications
You must be signed in to change notification settings - Fork 46.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[compiler] Repros for missing memoization due to lack of phi type inf…
…erence This is a complex case: we not only need phi type inference but also need to be able infer the union of `MixedReadonly | Array`. ghstack-source-id: 935088910dd8c210b3253cf8ff1f4b935f5081b7 Pull Request resolved: #30793
- Loading branch information
1 parent
ee7f675
commit 039c5c0
Showing
4 changed files
with
191 additions
and
0 deletions.
There are no files selected for viewing
42 changes: 42 additions & 0 deletions
42
...tures/compiler/error.todo-repro-missing-memoization-lack-of-phi-types.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,42 @@ | ||
|
||
## Input | ||
|
||
```javascript | ||
// @flow @validatePreserveExistingMemoizationGuarantees | ||
import {useMemo} from 'react'; | ||
import {useFragment} from 'shared-runtime'; | ||
|
||
function Component() { | ||
const data = useFragment(); | ||
const nodes = data.nodes ?? []; | ||
const flatMap = nodes.flatMap(node => node.items); | ||
const filtered = flatMap.filter(item => item != null); | ||
const map = useMemo(() => filtered.map(), [filtered]); | ||
const index = filtered.findIndex(x => x === null); | ||
|
||
return ( | ||
<div> | ||
{map} | ||
{index} | ||
</div> | ||
); | ||
} | ||
|
||
``` | ||
## Error | ||
``` | ||
8 | const flatMap = nodes.flatMap(node => node.items); | ||
9 | const filtered = flatMap.filter(item => item != null); | ||
> 10 | const map = useMemo(() => filtered.map(), [filtered]); | ||
| ^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly (10:10) | ||
|
||
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. (10:10) | ||
11 | const index = filtered.findIndex(x => x === null); | ||
12 | | ||
13 | return ( | ||
``` | ||
19 changes: 19 additions & 0 deletions
19
...src/__tests__/fixtures/compiler/error.todo-repro-missing-memoization-lack-of-phi-types.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,19 @@ | ||
// @flow @validatePreserveExistingMemoizationGuarantees | ||
import {useMemo} from 'react'; | ||
import {useFragment} from 'shared-runtime'; | ||
|
||
function Component() { | ||
const data = useFragment(); | ||
const nodes = data.nodes ?? []; | ||
const flatMap = nodes.flatMap(node => node.items); | ||
const filtered = flatMap.filter(item => item != null); | ||
const map = useMemo(() => filtered.map(), [filtered]); | ||
const index = filtered.findIndex(x => x === null); | ||
|
||
return ( | ||
<div> | ||
{map} | ||
{index} | ||
</div> | ||
); | ||
} |
108 changes: 108 additions & 0 deletions
108
...s/compiler/repro-missing-memoization-lack-of-phi-types-explicit-types.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,108 @@ | ||
|
||
## Input | ||
|
||
```javascript | ||
// @flow @validatePreserveExistingMemoizationGuarantees @enableUseTypeAnnotations | ||
import {useMemo} from 'react'; | ||
import {useFragment} from 'shared-runtime'; | ||
|
||
// This is a version of error.todo-repro-missing-memoization-lack-of-phi-types | ||
// with explicit type annotations and using enableUseTypeAnnotations to demonstrate | ||
// that type information is sufficient to preserve memoization in this example | ||
function Component() { | ||
const data = useFragment(); | ||
const nodes: Array<any> = data.nodes ?? []; | ||
const flatMap: Array<any> = nodes.flatMap(node => node.items); | ||
const filtered: Array<any> = flatMap.filter(item => item != null); | ||
const map: Array<any> = useMemo(() => filtered.map(), [filtered]); | ||
const index: Array<any> = filtered.findIndex(x => x === null); | ||
|
||
return ( | ||
<div> | ||
{map} | ||
{index} | ||
</div> | ||
); | ||
} | ||
|
||
``` | ||
## Code | ||
```javascript | ||
import { c as _c } from "react/compiler-runtime"; | ||
import { useMemo } from "react"; | ||
import { useFragment } from "shared-runtime"; | ||
|
||
function Component() { | ||
const $ = _c(11); | ||
const data = useFragment(); | ||
let t0; | ||
if ($[0] !== data.nodes) { | ||
t0 = data.nodes ?? []; | ||
$[0] = data.nodes; | ||
$[1] = t0; | ||
} else { | ||
t0 = $[1]; | ||
} | ||
const nodes = t0; | ||
let t1; | ||
if ($[2] !== nodes) { | ||
t1 = nodes.flatMap(_temp); | ||
$[2] = nodes; | ||
$[3] = t1; | ||
} else { | ||
t1 = $[3]; | ||
} | ||
const flatMap = t1; | ||
let t2; | ||
if ($[4] !== flatMap) { | ||
t2 = flatMap.filter(_temp2); | ||
$[4] = flatMap; | ||
$[5] = t2; | ||
} else { | ||
t2 = $[5]; | ||
} | ||
const filtered = t2; | ||
let t3; | ||
let t4; | ||
if ($[6] !== filtered) { | ||
t4 = filtered.map(); | ||
$[6] = filtered; | ||
$[7] = t4; | ||
} else { | ||
t4 = $[7]; | ||
} | ||
t3 = t4; | ||
const map = t3; | ||
const index = filtered.findIndex(_temp3); | ||
let t5; | ||
if ($[8] !== map || $[9] !== index) { | ||
t5 = ( | ||
<div> | ||
{map} | ||
{index} | ||
</div> | ||
); | ||
$[8] = map; | ||
$[9] = index; | ||
$[10] = t5; | ||
} else { | ||
t5 = $[10]; | ||
} | ||
return t5; | ||
} | ||
function _temp3(x) { | ||
return x === null; | ||
} | ||
function _temp2(item) { | ||
return item != null; | ||
} | ||
function _temp(node) { | ||
return node.items; | ||
} | ||
|
||
``` | ||
### Eval output | ||
(kind: exception) Fixture not implemented |
22 changes: 22 additions & 0 deletions
22
...__tests__/fixtures/compiler/repro-missing-memoization-lack-of-phi-types-explicit-types.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,22 @@ | ||
// @flow @validatePreserveExistingMemoizationGuarantees @enableUseTypeAnnotations | ||
import {useMemo} from 'react'; | ||
import {useFragment} from 'shared-runtime'; | ||
|
||
// This is a version of error.todo-repro-missing-memoization-lack-of-phi-types | ||
// with explicit type annotations and using enableUseTypeAnnotations to demonstrate | ||
// that type information is sufficient to preserve memoization in this example | ||
function Component() { | ||
const data = useFragment(); | ||
const nodes: Array<any> = data.nodes ?? []; | ||
const flatMap: Array<any> = nodes.flatMap(node => node.items); | ||
const filtered: Array<any> = flatMap.filter(item => item != null); | ||
const map: Array<any> = useMemo(() => filtered.map(), [filtered]); | ||
const index: Array<any> = filtered.findIndex(x => x === null); | ||
|
||
return ( | ||
<div> | ||
{map} | ||
{index} | ||
</div> | ||
); | ||
} |