[WIP] Avoid forcing useless renders #1569
Draft
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
Ldoppea
reviewed
Dec 10, 2024
| @@ -1193,7 +1193,7 @@ client.query(Q('io.cozy.bills'))`) | |||
| if (queryDef instanceof QueryDefinition) { | |||
| definitions.push(queryDef) | |||
| } else { | |||
| documents.push(queryDef) | |||
| documents.push(doc) | |||
There was a problem hiding this comment.
Is this part of the fix? I don't understand the impact compared to the previous implementation
We used to hydrate any document with the relationships existing in the provided schema, even though the relationship does not exist on the document. We now hydrate only if the relationship is set in the document. Note we can still force the hydratation through a `forceHydration` option, to ease migrations on apps with many relations. BREAKING CHANGE: the relationship hydration is made only if the relationship exists in the document, so the developer should not assume a `document.relationshipName` is always defined, anymore. As an alternative, it is now possible to pass `forceHydration` on cozy-client options to ease migration. However, please not this has performance impact, as it forces extra-check on store queries evaluation.
paultranvan
force-pushed
the
fix-store-selector
branch
from
3f703eb to
5f5b0f0
Compare
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.
Each time a useQuery is made, it is saved as a listener in redux, through the
useSelectorhook. We provide the result ofgetQueryFromStateto this hook to evaluate whether or not the state changed. By default, this evaluation is a referential equality check. See https://react-redux.js.org/api/hooks#useselector :The issue here is that the
getQueryFromStatefunction always return a new object : https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/CozyClient.js#L1428Therefore, the reference comparison will never be true.
And because the dispatch will make the evaluation of all listeners, it means any query will force the re-render to all components having a
useQuery, leading to performance issues.See this performance check, measured on Cozy Drive :
Before
After