Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't fetch for candidateapps at start #1037

Merged

Conversation

acezard
Copy link
Contributor

@acezard acezard commented Nov 27, 2023

This is triggering unecessary rerenders elsewhere in the app
when the user doesn't has files to upload.
Instead we fetch registry if we detect presence of files.
It might be slower in file upload scenario, but it cleans up a lot
app start rerenders (file upload state is linked to localMethods)

This is triggering unecessary rerenders elsewhere in the app
when the user doesn't has files to upload.
Instead we fetch registry if we detect presence of files.
It might be slower in file upload scenario, but it cleans up a lot
app start rerenders (file upload state is linked to localMethods)
@@ -62,7 +62,7 @@ export const OsReceiveProvider = ({
}, [client?.isLogged])

useEffect(() => {
if (!client || didCall.current) return
if (!client || didCall.current || state.filesToUpload.length === 0) return

const fetchRegistry = async (): Promise<void> => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We do not call the registry. We only call the cozy to fetch the apps. The method name should reflect that and should not be misleading

export const fetchOsReceiveCozyApps = {

Copy link
Contributor Author

@acezard acezard Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, updating

We're not calling the registry but the sack.
The distinction is meaningful and should be reflected in the
function name
@acezard acezard requested a review from Crash-- November 27, 2023 13:09
@acezard acezard merged commit 27f50a4 into master Nov 27, 2023
1 check passed
@acezard acezard deleted the fix--don't-call-registry-for-candidate-apps-at-app-start branch November 27, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants