-
Notifications
You must be signed in to change notification settings - Fork 47.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
InferReactivePlaces understands setState type
I realized this while working on Forest. When computing the dependencies of a reactive scope we can omit setState functions in the general case (exception described below). Currently that's implemented in PruneNonReactiveDependencies. However, this causes us to miss some optimizations — a value isn't reactive if its only dependency is a setState, and that may allow further downstreams values to become non-reactive. We lose out on that by only filtering out setStates in PruneNonReactiveDependencies — this logic really belongs in InferReactivePlaces. So this PR moves the check for setState types to that pass. The updated fixtures show that this already uncovers some wins. The _new_ fixtures covers the exception. It's possible for a value to be typed as being a setState function, but to still be reactive: if its a local that is conditionally assigned different setState function values. Currently this test happens to work because our phi type inference is incomplete (see #2296). I'm adding the test now though to prevent regressions when we fix phi type inference.
- Loading branch information
1 parent
9be2efd
commit fa8f47e
Showing
8 changed files
with
172 additions
and
29 deletions.
There are no files selected for viewing
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
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
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
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
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
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
111 changes: 111 additions & 0 deletions
111
...sts__/fixtures/compiler/reactive-control-dependency-phi-setState-type.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,111 @@ | ||
|
|
||
| ## Input | ||
|
|
||
| ```javascript | ||
| import invariant from "invariant"; | ||
| import { useState } from "react"; | ||
|
|
||
| function Component(props) { | ||
| const [x, setX] = useState(false); | ||
| const [y, setY] = useState(false); | ||
| let setState; | ||
| if (props.cond) { | ||
| setState = setX; | ||
| } else { | ||
| setState = setY; | ||
| } | ||
| const setState2 = setState; | ||
| const stateObject = { setState: setState2 }; | ||
| return ( | ||
| <Foo | ||
| cond={props.cond} | ||
| setX={setX} | ||
| setY={setY} | ||
| setState={stateObject.setState} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| function Foo({ cond, setX, setY, setState }) { | ||
| if (cond) { | ||
| invariant(setState === setX, "Expected the correct setState function"); | ||
| } else { | ||
| invariant(setState === setY, "Expected the correct setState function"); | ||
| } | ||
| return "ok"; | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: Component, | ||
| // TODO: run this function with {cond:true}, {cond: false} | ||
| params: [{ cond: true }], | ||
| }; | ||
|
|
||
| ``` | ||
|
|
||
| ## Code | ||
|
|
||
| ```javascript | ||
| import invariant from "invariant"; | ||
| import { useState, unstable_useMemoCache as useMemoCache } from "react"; | ||
|
|
||
| function Component(props) { | ||
| const $ = useMemoCache(5); | ||
| const [x, setX] = useState(false); | ||
| const [y, setY] = useState(false); | ||
| let setState; | ||
| if (props.cond) { | ||
| setState = setX; | ||
| } else { | ||
| setState = setY; | ||
| } | ||
|
|
||
| const setState2 = setState; | ||
| let t0; | ||
| if ($[0] !== setState2) { | ||
| t0 = { setState: setState2 }; | ||
| $[0] = setState2; | ||
| $[1] = t0; | ||
| } else { | ||
| t0 = $[1]; | ||
| } | ||
| const stateObject = t0; | ||
| let t1; | ||
| if ($[2] !== props.cond || $[3] !== stateObject.setState) { | ||
| t1 = ( | ||
| <Foo | ||
| cond={props.cond} | ||
| setX={setX} | ||
| setY={setY} | ||
| setState={stateObject.setState} | ||
| /> | ||
| ); | ||
| $[2] = props.cond; | ||
| $[3] = stateObject.setState; | ||
| $[4] = t1; | ||
| } else { | ||
| t1 = $[4]; | ||
| } | ||
| return t1; | ||
| } | ||
|
|
||
| function Foo(t21) { | ||
| const { cond, setX, setY, setState } = t21; | ||
| if (cond) { | ||
| invariant(setState === setX, "Expected the correct setState function"); | ||
| } else { | ||
| invariant(setState === setY, "Expected the correct setState function"); | ||
| } | ||
| return "ok"; | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: Component, | ||
| // TODO: run this function with {cond:true}, {cond: false} | ||
| params: [{ cond: true }], | ||
| }; | ||
|
|
||
| ``` | ||
| ### Eval output | ||
| (kind: ok) ok |
38 changes: 38 additions & 0 deletions
38
...t-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-phi-setState-type.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,38 @@ | ||
| import invariant from "invariant"; | ||
| import { useState } from "react"; | ||
|
|
||
| function Component(props) { | ||
| const [x, setX] = useState(false); | ||
| const [y, setY] = useState(false); | ||
| let setState; | ||
| if (props.cond) { | ||
| setState = setX; | ||
| } else { | ||
| setState = setY; | ||
| } | ||
| const setState2 = setState; | ||
| const stateObject = { setState: setState2 }; | ||
| return ( | ||
| <Foo | ||
| cond={props.cond} | ||
| setX={setX} | ||
| setY={setY} | ||
| setState={stateObject.setState} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| function Foo({ cond, setX, setY, setState }) { | ||
| if (cond) { | ||
| invariant(setState === setX, "Expected the correct setState function"); | ||
| } else { | ||
| invariant(setState === setY, "Expected the correct setState function"); | ||
| } | ||
| return "ok"; | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: Component, | ||
| // TODO: run this function with {cond:true}, {cond: false} | ||
| params: [{ cond: true }], | ||
| }; |