Skip to content

Commit d89e137

Browse files
amcaplanamcaplan
authored
Merge pull request #6040 from Shopify/slow-autocomplete-prompt-to-avoid-rate-limiting
Slow autocomplete prompt to avoid rate limiting
2 parents 36a764d + a2623f2 commit d89e137

File tree

6 files changed

+72
-105
lines changed

6 files changed

+72
-105
lines changed

‎packages/app/src/cli/api/graphql/app-management/generated/apps.ts‎

Lines changed: 1 addition & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
/* eslint-disable @typescript-eslint/consistent-type-definitions */
22
import * as Types from './types.js'
3-
import {JsonMapType} from '@shopify/cli-kit/node/toml'
43

54
import {TypedDocumentNode as DocumentNode} from '@graphql-typed-document-node/core'
65

@@ -10,25 +9,7 @@ export type ListAppsQueryVariables = Types.Exact<{
109

1110
export type ListAppsQuery = {
1211
appsConnection?: {
13-
edges: {
14-
node: {
15-
id: string
16-
key: string
17-
activeRelease: {
18-
id: string
19-
version: {
20-
name: string
21-
appModules: {
22-
uuid: string
23-
userIdentifier: string
24-
handle: string
25-
config: JsonMapType
26-
specification: {identifier: string; externalIdentifier: string; name: string}
27-
}[]
28-
}
29-
}
30-
}
31-
}[]
12+
edges: {node: {id: string; key: string; activeRelease: {id: string; version: {name: string}}}}[]
3213
pageInfo: {hasNextPage: boolean}
3314
} | null
3415
}
@@ -92,20 +73,6 @@ export const ListApps = {
9273
kind: 'SelectionSet',
9374
selections: [
9475
{kind: 'Field', name: {kind: 'Name', value: 'name'}},
95-
{
96-
kind: 'Field',
97-
name: {kind: 'Name', value: 'appModules'},
98-
selectionSet: {
99-
kind: 'SelectionSet',
100-
selections: [
101-
{
102-
kind: 'FragmentSpread',
103-
name: {kind: 'Name', value: 'ReleasedAppModule'},
104-
},
105-
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
106-
],
107-
},
108-
},
10976
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
11077
],
11178
},
@@ -140,31 +107,5 @@ export const ListApps = {
140107
],
141108
},
142109
},
143-
{
144-
kind: 'FragmentDefinition',
145-
name: {kind: 'Name', value: 'ReleasedAppModule'},
146-
typeCondition: {kind: 'NamedType', name: {kind: 'Name', value: 'AppModule'}},
147-
selectionSet: {
148-
kind: 'SelectionSet',
149-
selections: [
150-
{kind: 'Field', name: {kind: 'Name', value: 'uuid'}},
151-
{kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}},
152-
{kind: 'Field', name: {kind: 'Name', value: 'handle'}},
153-
{kind: 'Field', name: {kind: 'Name', value: 'config'}},
154-
{
155-
kind: 'Field',
156-
name: {kind: 'Name', value: 'specification'},
157-
selectionSet: {
158-
kind: 'SelectionSet',
159-
selections: [
160-
{kind: 'Field', name: {kind: 'Name', value: 'identifier'}},
161-
{kind: 'Field', name: {kind: 'Name', value: 'externalIdentifier'}},
162-
{kind: 'Field', name: {kind: 'Name', value: 'name'}},
163-
],
164-
},
165-
},
166-
],
167-
},
168-
},
169110
],
170111
} as unknown as DocumentNode<ListAppsQuery, ListAppsQueryVariables>

‎packages/app/src/cli/api/graphql/app-management/queries/apps.graphql‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ query listApps($query: String) {
88
id
99
version {
1010
name
11-
appModules {
12-
...ReleasedAppModule
13-
}
1411
}
1512
}
1613
}

‎packages/cli-kit/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@
170170
"@types/diff": "^5.2.3",
171171
"@types/fs-extra": "9.0.13",
172172
"@types/gradient-string": "^1.1.2",
173-
"@types/lodash": "4.17.14",
173+
"@types/lodash": "4.17.19",
174174
"@types/react": "18.2.0",
175175
"@types/semver": "^7.5.2",
176176
"@types/which": "3.0.4",

‎packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.tsx‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {InfoTableProps} from './Prompts/InfoTable.js'
33
import {TextInput} from './TextInput.js'
44
import {InfoMessageProps} from './Prompts/InfoMessage.js'
55
import {Message, PromptLayout} from './Prompts/PromptLayout.js'
6-
import {debounce} from '../../../../public/common/function.js'
6+
import {throttle} from '../../../../public/common/function.js'
77
import {AbortSignal} from '../../../../public/node/abort.js'
88
import usePrompt, {PromptState} from '../hooks/use-prompt.js'
99
import React, {ReactElement, useCallback, useEffect, useRef, useState} from 'react'
@@ -84,40 +84,46 @@ function AutocompletePrompt<T>({
8484
const searchTermRef = useRef('')
8585
searchTermRef.current = searchTerm
8686

87-
// disable exhaustive-deps because we want to memoize the debounce function itself
88-
// eslint-disable-next-line react-hooks/exhaustive-deps
89-
const debounceSearch = useCallback(
90-
debounce(
91-
(term: string) => {
92-
setLoadingWhenSlow.current = setTimeout(() => {
93-
setPromptState(PromptState.Loading)
94-
}, 100)
95-
paginatedSearch(term)
96-
.then((result) => {
97-
// while we were waiting for the promise to resolve, the user
98-
// has emptied the search term, so we want to show the default
99-
// choices instead
100-
if (searchTermRef.current.length === 0) {
101-
setSearchResults(choices)
102-
setHasMorePages(initialHasMorePages)
103-
} else {
104-
setSearchResults(result.data)
105-
setHasMorePages(result.meta?.hasNextPage ?? false)
106-
}
87+
// Keep current values in refs to avoid stale closures
88+
const choicesRef = useRef(choices)
89+
choicesRef.current = choices
90+
const initialHasPagesRef = useRef(initialHasMorePages)
91+
initialHasPagesRef.current = initialHasMorePages
10792

108-
setPromptState(PromptState.Idle)
109-
})
110-
.catch(() => {
111-
setPromptState(PromptState.Error)
112-
})
113-
.finally(() => {
114-
clearTimeout(setLoadingWhenSlow.current)
115-
})
116-
},
117-
300,
118-
{leading: true},
119-
),
120-
[initialHasMorePages, choices, paginatedSearch, searchResults],
93+
// useMemo ensures debounceSearch is not recreated on every render
94+
const debounceSearch = React.useMemo(
95+
() =>
96+
throttle(
97+
(term: string) => {
98+
setLoadingWhenSlow.current = setTimeout(() => {
99+
setPromptState(PromptState.Loading)
100+
}, 100)
101+
paginatedSearch(term)
102+
.then((result) => {
103+
// while we were waiting for the promise to resolve, the user
104+
// has emptied the search term, so we want to show the default
105+
// choices instead
106+
if (searchTermRef.current.length === 0) {
107+
setSearchResults(choicesRef.current)
108+
setHasMorePages(initialHasPagesRef.current)
109+
} else {
110+
setSearchResults(result.data)
111+
setHasMorePages(result.meta?.hasNextPage ?? false)
112+
}
113+
114+
setPromptState(PromptState.Idle)
115+
})
116+
.catch(() => {
117+
setPromptState(PromptState.Error)
118+
})
119+
.finally(() => {
120+
clearTimeout(setLoadingWhenSlow.current)
121+
})
122+
},
123+
400,
124+
{leading: true, trailing: true},
125+
),
126+
[paginatedSearch, setPromptState],
121127
)
122128

123129
return (

‎packages/cli-kit/src/public/common/function.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import lodashMemoize from 'lodash/memoize.js'
22
import lodashDebounce from 'lodash/debounce.js'
3-
import type {DebouncedFunc, DebounceSettings} from 'lodash'
3+
import lodashThrottle from 'lodash/throttle.js'
4+
import type {DebouncedFunc, DebounceSettings, ThrottleSettings} from 'lodash'
45

56
/**
67
* Creates a function that memoizes the result of func. If resolver is provided it determines the cache key for
@@ -27,7 +28,7 @@ export function memoize<T extends (...args: any) => any>(func: T, resolver?: (..
2728
* Note: If leading and trailing options are true, func is invoked on the trailing edge of the timeout only
2829
* if the the debounced function is invoked more than once during the wait timeout.
2930
*
30-
* See David Corbacho’s article for details over the differences between _.debounce and _.throttle.
31+
* See David Corbacho's article for details over the differences between _.debounce and _.throttle.
3132
*
3233
* @param func - The function to debounce.
3334
* @param wait - The number of milliseconds to delay.
@@ -42,3 +43,25 @@ export function debounce<T extends (...args: any) => any>(
4243
): DebouncedFunc<T> {
4344
return lodashDebounce(func, wait, options)
4445
}
46+
47+
/**
48+
* Creates a throttled function that only invokes func at most once per every wait milliseconds.
49+
* The throttled function comes with a cancel method to cancel delayed invocations and a flush method to immediately invoke them.
50+
* Provide an options object to indicate whether func should be invoked on the leading and/or trailing edge of the wait timeout.
51+
* Subsequent calls to the throttled function return the result of the last func invocation.
52+
*
53+
* See David Corbacho's article for details over the differences between _.debounce and _.throttle.
54+
*
55+
* @param func - The function to throttle.
56+
* @param wait - The number of milliseconds to throttle invocations to.
57+
* @param options - The options object.
58+
* @returns Returns the new throttled function.
59+
*/
60+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
61+
export function throttle<T extends (...args: any) => any>(
62+
func: T,
63+
wait?: number,
64+
options?: ThrottleSettings,
65+
): DebouncedFunc<T> {
66+
return lodashThrottle(func, wait, options)
67+
}

‎pnpm-lock.yaml‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)