fix: correctly instantiate new API client if options change #25
Conversation
This file contains hidden or 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
before this change a new client would not be instantiated when some options change, as useMemo doesn’t deep compare the values in the dependencies array. The workaround to use state for it didn’t actually work. Also useMemo is meant to only be used as a performance improvement, which in this case is not enough because rely on the internal state of the API client so we don’t want to recreate it unless its options change.
|
Great, I can see where we'll need this soon :) I tested this out locally with the Geocode Earth website, and everything appears to work fine. Anything else to do before this is merged? |
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.
before this change a new client would not be instantiated when some options change, as useMemo doesn’t deep compare the values in the dependencies array. The workaround to use state for it didn’t actually work.
Also useMemo is meant to only be used as a performance improvement, which in this case is not enough because rely on the internal state of the API client so we don’t want to recreate it unless its options change.
Instead of useMemo we now use a deep compare effect which correctly compares the props, and then store the client as ref.