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

Instance list request modifications to fix settings persistence #1556

Conversation

clpetersonucf
Copy link
Member

@clpetersonucf clpetersonucf commented Jan 9, 2024

Resolves #1553

Even though settings are correctly saved to the server and subsequent pulls from the API include the correct data, the local query cache containing the instance list wasn't being updated because the function that converts the raw query data into the actual instance list is memoized for performance. The useMemo hook only does a shallow comparison of objects and would not register the changes applied to an individual instance.

Instead, this PR adds a modified value to the local query cache when it is altered locally via setQueryData - for example, updating, copying, or deleting an instance. This is recognized by the useMemo hook which refreshes the instance list appropriately.

Additionally, the useUpdateWidget hook was stringifying certain values that should be integers - as far as I can tell there's no need for them to be strings, when the values from the server are always ints. They've been updated accordingly.

Copy link
Contributor

@cayb0rg cayb0rg left a comment

Choose a reason for hiding this comment

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

Nice work! Not seeing anything breaking here 👍🏻

@clpetersonucf clpetersonucf merged commit 699e343 into ucfopen:dev/10.1.0 Jan 10, 2024
2 checks passed
@clpetersonucf clpetersonucf deleted the issue/1553-fix-widget-settings-local-instance-cache branch January 10, 2024 21:54
@clpetersonucf clpetersonucf mentioned this pull request Jan 30, 2024
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