Skip to content

Commit

Permalink
fix: correctly instantiate new API client if options change
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mxlje committed May 9, 2022
1 parent efc670b commit 9f7d82b
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 13 deletions.
41 changes: 40 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
"lodash.throttle": "^4.1.1",
"lodash.unescape": "^4.0.1",
"react": "^17.0.2",
"react-dom": "^17.0.2"
"react-dom": "^17.0.2",
"use-deep-compare-effect": "^1.8.1"
},
"devDependencies": {
"esbuild": "^0.14.38",
Expand Down
20 changes: 9 additions & 11 deletions src/autocomplete/autocomplete.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useState, useMemo, useCallback, useRef, useEffect } from 'react'
import React, { useState, useCallback, useRef, useEffect } from 'react'
import useDeepCompareEffect from 'use-deep-compare-effect'
import { useCombobox } from 'downshift'
import { createAutocomplete } from '@geocodeearth/core-js'
import throttle from 'lodash.throttle'
Expand Down Expand Up @@ -31,6 +32,7 @@ export default ({
const [results, setResults] = useState(emptyResults)
const [isLoading, setIsLoading] = useState(false)
const inputRef = useRef()
const autocomplete = useRef()

// call user-supplied onFeatures callback
useEffect(() => {
Expand All @@ -39,22 +41,18 @@ export default ({
}
}, [results])

// setting params & options as state so they can be passed to useMemo as dependencies,
// which doesn’t work if they’re just objects as the internal comparison fails
const [apiParams, setApiParams] = useState(params)
const [apiOptions, setApiOptions] = useState(options)

// Geocode Earth Autocomplete Client
const autocomplete = useMemo(() => {
return createAutocomplete(apiKey, params, {
// deep compare is used to to only instantiate a new autocomplete API client if
// required properties for it change
useDeepCompareEffect(() => {
autocomplete.current = createAutocomplete(apiKey, params, {
...options,
client: `ge-autocomplete${typeof VERSION !== 'undefined' ? `-${VERSION}` : ''}`
})
}, [apiKey, apiParams, apiOptions])
}, [apiKey, params, options])

// search queries the autocomplete API
const search = useCallback(text => {
autocomplete(text).then(({ features, discard }) => {
autocomplete.current(text).then(({ features, discard }) => {
setIsLoading(false)

if (discard || inputRef.current.value !== text) {
Expand Down

0 comments on commit 9f7d82b

Please sign in to comment.