[compiler] Recognize that refs are stable #30679
Merged
Conversation
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
…n dependency is a ref Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. [ghstack-poisoned]
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
mvitousek
added a commit
that referenced
this pull request
Aug 13, 2024
…n dependency is a ref Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. ghstack-source-id: 5cd2aa1f0a9b06348b46ee5383b26a35c4e81db9 Pull Request resolved: #30679
| @@ -258,6 +260,13 @@ function validateInferredDep( | |||
| } | |||
| } | |||
| let errorDiagnostic: CompareDependencyResult | null = null; | |||
There was a problem hiding this comment.
nit: move the let errorDiagnostic ... down since it's unrelated to the new if block
There was a problem hiding this comment.
One thing to note is that it's possible to have a reactive ref:
const ref1 = useRef(..);
const ref2 = useRef(..);
const ref = props.cond ? ref1 : ref2;
...in which case we wouldn't want to prune ref as dependency in a subsequent reactive-scope/memo-block. In practice this is very rare, but unless we guard against that case the change here isn't 100% safe.
…ization when dependency is a ref" Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. [ghstack-poisoned]
mvitousek
added a commit
that referenced
this pull request
Aug 13, 2024
…n dependency is a ref Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. ghstack-source-id: a8e56b5c79715b53443a7be1fbe17ca5a2af23f6 Pull Request resolved: #30679
|
@josephsavona Good catch! I pulled this thread a little more and realized that we already have the technology to mark useRef results as non-reactive at their point of creation (the |
…ization when dependency is a ref" Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. [ghstack-poisoned]
mvitousek
added a commit
that referenced
this pull request
Aug 13, 2024
…n dependency is a ref Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. ghstack-source-id: 01f0078c5c3f15174fcba4af2115844527a79ff4 Pull Request resolved: #30679
…ization when dependency is a ref" Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. [ghstack-poisoned]
mvitousek
added a commit
that referenced
this pull request
Aug 13, 2024
…n dependency is a ref Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. ghstack-source-id: 685d859d1eed5d1e19dbbbfadc75be3875ddb6ea Pull Request resolved: #30679
…ization when dependency is a ref" Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. [ghstack-poisoned]
mvitousek
changed the title
mvitousek
added a commit
that referenced
this pull request
Aug 14, 2024
…n dependency is a ref Summary: In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source. ghstack-source-id: 685d859d1eed5d1e19dbbbfadc75be3875ddb6ea Pull Request resolved: #30679
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Summary:
In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. We can mark useRef as producing a stable value to get better memoization