[DISP-72] Fix sorting in Ideas Map #1766
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
kogre
approved these changes
Apr 1, 2022
|
@lilkraftwerk could you check the linter issues? |
|
@brentguf On it |
kogre
approved these changes
Apr 1, 2022
lilkraftwerk
deleted the
DISP-72-when-changing-the-idea-sorting-on-the-map-view-the-platform-gets-stuck
branch
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.
Bug here: https://citizenlab.atlassian.net/browse/DISP-72
To reproduce on master, the window must be larger than a certain breakpoint (not exactly sure which but full width on my laptop is big enough to reproduce). Changing the sorting here causes an infinite loop of requests:
I believe the bug was due to passing arrays into the dependency array, which made React freak out when trying to reconcile the contents. This PR changes the dependency array for the
useEffectfunction to useJSON.stringifyon the arrays we were passing in, which makes them normal strings and therefore easy for React to reconcile. This fixes the bug on my local machine but please test before/afterI don't think there are many downsides using this fix, Dan Abramov of the React team recommends it here: facebook/react#14476 and it might be a little slower but likely not noticeable on this component