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 open quotas bugging UI like hell when there are many events #126

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

kahlstrm
Copy link

@kahlstrm kahlstrm commented Jan 21, 2024

If there are lots of events (created 2000 for performance optimization testing), the site event list becomes quite wonky when switching between languages. This should fix that.

@kahlstrm kahlstrm self-assigned this Feb 20, 2024
@@ -26,7 +26,7 @@ export function useEventListState({ category }: EventListProps = {}) {
}, [category]);

return useShallowMemo<State>({
events: fetchEvents.result,
events: fetchEvents.result ?? [],
Copy link
Member

@PurkkaKoodari PurkkaKoodari Feb 20, 2024

Choose a reason for hiding this comment

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

This will neutralize the entire point of the useShallowMemo as it will invalidate every time. Move the [] to a global constant to keep the reference stable.

(Or just return undefined here as before, and handle this within the useMemo later)

Copy link
Author

@kahlstrm kahlstrm Feb 21, 2024

Choose a reason for hiding this comment

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

What does this useShallowMemo even try to achieve here if we are using useMemo anyway?

Copy link
Member

@PurkkaKoodari PurkkaKoodari Feb 21, 2024

Choose a reason for hiding this comment

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

It ensures the caller gets a stable reference (as ilmomasiina-modules is public interface). Probably not necessary, true. (Still, this particular combination is worse than either option)

Copy link
Member

Choose a reason for hiding this comment

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

Actually, why that exists: so that the context isn't unnecessarily updated with the same values if the provider re-renders, forcing all context consumers to re-render (since there's no useContextWithSelector)

@PurkkaKoodari PurkkaKoodari merged commit df1a4b2 into dev Feb 22, 2024
1 check passed
@PurkkaKoodari PurkkaKoodari deleted the frontend-quota-fix branch February 22, 2024 19:33
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