artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 17:31:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

search: Move query state out of root component state (#24844)

Browse Source 
This commit moves the "global" query state out of the state of the root component. Because the query state is updated on every keypress it caused excessive re-rendering of the whole application, which, depending on the page, causes massive performance issues.

The query state isn't really "global", rather it's information that is needed by a handful of components rendered in different parts of the page. This commit adds https://zustand.surge.sh/ to make this data available to those components without impacting the overall application (also as a potential solution for other similar use cases).
This commit is contained in:
Felix Kling 2021-09-16 09:50:34 +02:00 committed by GitHub
parent 020af73dac
commit a2e870793a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 74 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -32,6 +32,7 @@ All notable changes to Sourcegraph are documented in this file.
### Fixed
- Fixed a number of issues where repository permissions sync may fail for instances with very large numbers of repositories. [#24852](https://github.com/sourcegraph/sourcegraph/pull/24852), [#24972](https://github.com/sourcegraph/sourcegraph/pull/24972)
- Fixed excessive re-rendering of the whole web application on every keypress in the search query input. [#24844](https://github.com/sourcegraph/sourcegraph/pull/24844)
### Removed

3
client/web/src/Layout.tsx
View File

@ -59,7 +59,6 @@ import {
SearchContextProps,
getGlobalSearchContextFilter,
} from './search'
import { QueryState } from './search/helpers'
import { SiteAdminAreaRoute } from './site-admin/SiteAdminArea'
import { SiteAdminSideBarGroups } from './site-admin/SiteAdminSidebar'
import { useTheme } from './theme'
@ -123,8 +122,6 @@ export interface LayoutProps
viewerSubject: Pick<GQL.ISettingsSubject, 'id' | 'viewerCanAdminister'>
// Search
navbarSearchQueryState: QueryState
onNavbarQueryChange: (queryState: QueryState) => void
fetchHighlightedFileLineRanges: (parameters: FetchFileParameters, force?: boolean) => Observable<string[][]>
globbing: boolean

13
client/web/src/SourcegraphWebApp.tsx
View File

@ -80,7 +80,6 @@ import {
getUserSearchContextNamespaces,
fetchSearchContextBySpec,
} from './search/backend'
import { QueryState } from './search/helpers'
import { SearchResultsCacheProvider } from './search/results/SearchResultsCacheProvider'
import { TemporarySettingsProvider } from './settings/temporary/TemporarySettingsProvider'
import { listUserRepositories } from './site-admin/backend'
@ -140,11 +139,6 @@ interface SourcegraphWebAppState extends SettingsCascadeProps {
viewerSubject: LayoutProps['viewerSubject']
/**
* The current search query in the navbar.
*/
navbarSearchQueryState: QueryState
/**
* The current parsed search query, with all UI-configurable parameters
* (eg. pattern type, case sensitivity, version context) removed
@ -287,7 +281,6 @@ export class SourcegraphWebApp extends React.Component<SourcegraphWebAppProps, S
: undefined
this.state = {
navbarSearchQueryState: { query: '' },
settingsCascade: EMPTY_SETTINGS_CASCADE,
viewerSubject: SITE_SUBJECT_NO_ADMIN,
parsedSearchQuery: parsedSearchURL.query || '',
@ -496,8 +489,6 @@ export class SourcegraphWebApp extends React.Component<SourcegraphWebAppProps, S
this.state.settingsCascade
)}
// Search query
navbarSearchQueryState={this.state.navbarSearchQueryState}
onNavbarQueryChange={this.onNavbarQueryChange}
fetchHighlightedFileLineRanges={fetchHighlightedFileLineRanges}
parsedSearchQuery={this.state.parsedSearchQuery}
setParsedSearchQuery={this.setParsedSearchQuery}
@ -573,10 +564,6 @@ export class SourcegraphWebApp extends React.Component<SourcegraphWebAppProps, S
)
}
private onNavbarQueryChange = (navbarSearchQueryState: QueryState): void => {
this.setState({ navbarSearchQueryState })
}
private setParsedSearchQuery = (query: string): void => {
this.setState({ parsedSearchQuery: query })
}

2
client/web/src/nav/GlobalNavbar.story.tsx
View File

@ -61,8 +61,6 @@ const defaultProps = (
defaultSearchContextSpec: '',
showOnboardingTour: false,
isLightTheme: props.isLightTheme,
navbarSearchQueryState: { query: '' },
onNavbarQueryChange: () => {},
isExtensionAlertAnimating: false,
batchChangesEnabled: true,
batchChangesExecutionEnabled: true,

2
client/web/src/nav/GlobalNavbar.test.tsx
View File

@ -27,8 +27,6 @@ const PROPS: React.ComponentProps<typeof GlobalNavbar> = {
history: createMemoryHistory(),
keyboardShortcuts: [],
isSourcegraphDotCom: false,
navbarSearchQueryState: { query: 'q' },
onNavbarQueryChange: () => undefined,
onThemePreferenceChange: () => undefined,
isLightTheme: true,
themePreference: ThemePreference.Light,

10
client/web/src/nav/GlobalNavbar.tsx
View File

@ -48,8 +48,8 @@ import {
getGlobalSearchContextFilter,
SearchContextInputProps,
} from '../search'
import { QueryState } from '../search/helpers'
import { SearchNavbarItem } from '../search/input/SearchNavbarItem'
import { useNavbarQueryState } from '../search/navbarSearchQueryState'
import { ThemePreferenceProps } from '../theme'
import { userExternalServicesEnabledFromTags } from '../user/settings/cloud-ga'
import { showDotComMarketing } from '../util/features'
@ -79,8 +79,6 @@ interface Props
location: H.Location<{ query: string }>
authenticatedUser: AuthenticatedUser | null
authRequired: boolean
navbarSearchQueryState: QueryState
onNavbarQueryChange: (queryState: QueryState) => void
isSourcegraphDotCom: boolean
showSearchBox: boolean
routes: readonly LayoutRouteProps<{}>[]
@ -112,10 +110,8 @@ interface Props
export const GlobalNavbar: React.FunctionComponent<Props> = ({
authRequired,
showSearchBox,
navbarSearchQueryState,
caseSensitive,
patternType,
onNavbarQueryChange,
hideNavLinks,
variant,
isLightTheme,
@ -154,6 +150,8 @@ export const GlobalNavbar: React.FunctionComponent<Props> = ({
)
)
const onNavbarQueryChange = useNavbarQueryState(state => state.setQueryState)
useEffect(() => {
// On a non-search related page or non-repo page, we clear the query in
// the main query input to avoid misleading users
@ -193,8 +191,6 @@ export const GlobalNavbar: React.FunctionComponent<Props> = ({
const searchNavBar = (
<SearchNavbarItem
{...props}
navbarSearchState={navbarSearchQueryState}
onChange={onNavbarQueryChange}
location={location}
history={history}
isLightTheme={isLightTheme}

2
client/web/src/nav/__snapshots__/GlobalNavbar.test.tsx.snap
View File

@ -168,7 +168,6 @@ exports[`GlobalNavbar default 1`] = `
history="[object Object]"
keyboardshortcuts=""
location="[object Object]"
navbarsearchstate="[object Object]"
parsedsearchquery="r:golang/oauth2 test f:travis"
patterntype="literal"
platformcontext="[object Object]"
@ -351,7 +350,6 @@ exports[`GlobalNavbar low-profile 1`] = `
history="[object Object]"
keyboardshortcuts=""
location="[object Object]"
navbarsearchstate="[object Object]"
parsedsearchquery="r:golang/oauth2 test f:travis"
patterntype="literal"
platformcontext="[object Object]"

6
client/web/src/repo/RepoContainer.tsx
View File

@ -52,7 +52,7 @@ import {
searchQueryForRepoRevision,
SearchStreamingProps,
} from '../search'
import { QueryState } from '../search/helpers'
import { useNavbarQueryState } from '../search/navbarSearchQueryState'
import { StreamingSearchResultsListProps } from '../search/results/StreamingSearchResultsList'
import { browserExtensionInstalled } from '../tracking/analyticsUtils'
import { RouteDescriptor } from '../util/contributions'
@ -144,7 +144,6 @@ interface RepoContainerProps
repoSettingsAreaRoutes: readonly RepoSettingsAreaRoute[]
repoSettingsSidebarGroups: readonly RepoSettingsSideBarGroup[]
authenticatedUser: AuthenticatedUser | null
onNavbarQueryChange: (state: QueryState) => void
history: H.History
globbing: boolean
showSearchNotebook: boolean
@ -314,7 +313,8 @@ export const RepoContainer: React.FunctionComponent<RepoContainerProps> = props
}, [props.extensionsController, repoName, resolvedRevisionOrError, revision])
// Update the navbar query to reflect the current repo / revision
const { globbing, onNavbarQueryChange } = props
const { globbing } = props
const onNavbarQueryChange = useNavbarQueryState(state => state.setQueryState)
useEffect(() => {
let query = searchQueryForRepoRevision(repoName, globbing, revision)
if (filePath) {

14
client/web/src/search/input/SearchNavbarItem.tsx
View File

@ -17,7 +17,8 @@ import {
} from '..'
import { AuthenticatedUser } from '../../auth'
import { VersionContext } from '../../schema/site.schema'
import { submitSearch, QueryState } from '../helpers'
import { submitSearch } from '../helpers'
import { useNavbarQueryState } from '../navbarSearchQueryState'
import { SearchBox } from './SearchBox'
@ -34,9 +35,7 @@ interface Props
authenticatedUser: AuthenticatedUser | null
location: H.Location
history: H.History
navbarSearchState: QueryState
isSourcegraphDotCom: boolean
onChange: (newValue: QueryState) => void
globbing: boolean
isSearchAutoFocusRequired?: boolean
setVersionContext: (versionContext: string | undefined) => Promise<void>
@ -52,12 +51,14 @@ export const SearchNavbarItem: React.FunctionComponent<Props> = (props: Props) =
// or remove the search help button
const isSearchPage = props.location.pathname === '/search' && Boolean(parseSearchURLQuery(props.location.search))
const { queryState, setQueryState } = useNavbarQueryState()
const onSubmit = useCallback(
(event?: React.FormEvent): void => {
event?.preventDefault()
submitSearch({ ...props, query: props.navbarSearchState.query, source: 'nav' })
submitSearch({ ...props, query: queryState.query, source: 'nav' })
},
[props]
[props, queryState]
)
return (
@ -68,7 +69,8 @@ export const SearchNavbarItem: React.FunctionComponent<Props> = (props: Props) =
<SearchBox
{...props}
hasGlobalQueryBehavior={true}
queryState={props.navbarSearchState}
queryState={queryState}
onChange={setQueryState}
onSubmit={onSubmit}
autoFocus={autoFocus}
showSearchContextFeatureTour={true}

17
client/web/src/search/navbarSearchQueryState.ts Normal file
View File

@ -0,0 +1,17 @@
// NOTE (@fkling): The use of 'zustand' in this codebase should be considered as
// experimental until we had more time to evaluate this library. General
// application of this library is not recommended at this point.
// It is used here because it solves a very real performance issue
// (see https://github.com/sourcegraph/sourcegraph/issues/21200).
import create from 'zustand'
import { QueryState } from './helpers'
interface NavbarQueryState {
queryState: QueryState
setQueryState: (queryState: QueryState) => void
}
export const useNavbarQueryState = create<NavbarQueryState>(set => ({
queryState: { query: '' },
setQueryState: queryState => set({ queryState }),
}))

2
client/web/src/search/results/StreamingSearchResults.story.tsx
View File

@ -55,8 +55,6 @@ const defaultProps: StreamingSearchResultsProps = {
} as AuthenticatedUser,
isLightTheme: true,
navbarSearchQueryState: { query: '' },
onNavbarQueryChange: () => {},
isSourcegraphDotCom: false,
settingsCascade: {

2
client/web/src/search/results/StreamingSearchResults.test.tsx
View File

@ -52,8 +52,6 @@ describe('StreamingSearchResults', () => {
location: history.location,
authenticatedUser: null,
navbarSearchQueryState: { query: '' },
onNavbarQueryChange: () => {},
isSourcegraphDotCom: false,
settingsCascade: {

5
client/web/src/search/results/StreamingSearchResults.tsx
View File

@ -32,7 +32,7 @@ import { CodeInsightsProps } from '../../insights/types'
import { isCodeInsightsEnabled } from '../../insights/utils/is-code-insights-enabled'
import { SavedSearchModal } from '../../savedSearches/SavedSearchModal'
import { SearchBetaIcon } from '../CtaIcons'
import { getSubmittedSearchesCount, QueryState, submitSearch } from '../helpers'
import { getSubmittedSearchesCount, submitSearch } from '../helpers'
import { StreamingProgress } from './progress/StreamingProgress'
import { SearchAlert } from './SearchAlert'
@ -60,8 +60,6 @@ export interface StreamingSearchResultsProps
authenticatedUser: AuthenticatedUser | null
location: H.Location
history: H.History
navbarSearchQueryState: QueryState
onNavbarQueryChange: (queryState: QueryState) => void
isSourcegraphDotCom: boolean
fetchHighlightedFileLineRanges: (parameters: FetchFileParameters, force?: boolean) => Observable<string[][]>
@ -244,7 +242,6 @@ export const StreamingSearchResults: React.FunctionComponent<StreamingSearchResu
styles.streamingSearchResultsSidebar,
showSidebar && styles.streamingSearchResultsSidebarShow
)}
query={props.navbarSearchQueryState.query}
filters={results?.filters}
/>

28
client/web/src/search/results/sidebar/SearchReference.tsx
View File

@ -483,7 +483,6 @@ export interface SearchReferenceProps
Pick<SearchContextProps, 'selectedSearchContextSpec'> {
query: string
filter: string
navbarSearchQueryState: QueryState
onNavbarQueryChange: (queryState: QueryState) => void
isSourcegraphDotCom: boolean
}
@ -491,7 +490,7 @@ export interface SearchReferenceProps
const SearchReference = (props: SearchReferenceProps): ReactElement => {
const [selectedTab, setSelectedTab] = useLocalStorage(SEARCH_REFERENCE_TAB_KEY, 0)
const { onNavbarQueryChange, navbarSearchQueryState, telemetryService } = props
const { onNavbarQueryChange, query, telemetryService } = props
const filter = props.filter.trim()
const hasFilter = filter.length > 0
@ -505,16 +504,11 @@ const SearchReference = (props: SearchReferenceProps): ReactElement => {
const updateQuery = useCallback(
(searchReference: FilterInfo, negate: boolean) => {
const updatedQuery = updateQueryWithFilterAndExample(
navbarSearchQueryState.query,
searchReference.field,
searchReference,
{
singular: Boolean(FILTERS[searchReference.field].singular),
negate: negate && isNegatableFilter(searchReference.field),
emptyValue: shouldShowSuggestions(searchReference),
}
)
const updatedQuery = updateQueryWithFilterAndExample(query, searchReference.field, searchReference, {
singular: Boolean(FILTERS[searchReference.field].singular),
negate: negate && isNegatableFilter(searchReference.field),
emptyValue: shouldShowSuggestions(searchReference),
})
onNavbarQueryChange({
changeSource: QueryChangeSource.searchReference,
query: updatedQuery.query,
@ -523,22 +517,22 @@ const SearchReference = (props: SearchReferenceProps): ReactElement => {
showSuggestions: shouldShowSuggestions(searchReference),
})
},
[onNavbarQueryChange, navbarSearchQueryState]
[onNavbarQueryChange, query]
)
const updateQueryWithOperator = useCallback(
(info: OperatorInfo) => {
onNavbarQueryChange({
query: navbarSearchQueryState.query + ` ${info.operator} `,
query: query + ` ${info.operator} `,
})
},
[onNavbarQueryChange, navbarSearchQueryState]
[onNavbarQueryChange, query]
)
const updateQueryWithExample = useCallback(
(example: string) => {
telemetryService.log(hasFilter ? 'SearchReferenceSearchedAndClicked' : 'SearchReferenceFilterClicked')
onNavbarQueryChange({ query: navbarSearchQueryState.query.trimEnd() + ' ' + example })
onNavbarQueryChange({ query: query.trimEnd() + ' ' + example })
},
[onNavbarQueryChange, navbarSearchQueryState, hasFilter, telemetryService]
[onNavbarQueryChange, query, hasFilter, telemetryService]
)
const filterList = (

3
client/web/src/search/results/sidebar/SearchSidebar.story.tsx
View File

@ -46,12 +46,9 @@ const defaultProps: SearchSidebarProps = {
patternType: SearchPatternType.literal,
versionContext: undefined,
selectedSearchContextSpec: 'global',
query: '',
settingsCascade: EMPTY_SETTINGS_CASCADE,
telemetryService: NOOP_TELEMETRY_SERVICE,
featureFlags: EMPTY_FEATURE_FLAGS,
navbarSearchQueryState: { query: '' },
onNavbarQueryChange: () => {},
isSourcegraphDotCom: false,
}

27
client/web/src/search/results/sidebar/SearchSidebar.tsx
View File

@ -15,7 +15,8 @@ import { AuthenticatedUser } from '../../../auth'
import { FeatureFlagProps } from '../../../featureFlags/featureFlags'
import { TemporarySettings } from '../../../settings/temporary/TemporarySettings'
import { useTemporarySetting } from '../../../settings/temporary/useTemporarySetting'
import { QueryState, submitSearch, toggleSearchFilter } from '../../helpers'
import { submitSearch, toggleSearchFilter } from '../../helpers'
import { useNavbarQueryState } from '../../navbarSearchQueryState'
import { getDynamicFilterLinks, getRepoFilterLinks, getSearchSnippetLinks } from './FilterLink'
import { getFiltersOfKind, useLastRepoName } from './helpers'
@ -35,11 +36,8 @@ export interface SearchSidebarProps
TelemetryProps,
FeatureFlagProps {
authenticatedUser: AuthenticatedUser | null
query: string
filters?: Filter[]
className?: string
navbarSearchQueryState: QueryState
onNavbarQueryChange: (queryState: QueryState) => void
isSourcegraphDotCom: boolean
}
@ -56,22 +54,24 @@ export enum SectionID {
export const SearchSidebar: React.FunctionComponent<SearchSidebarProps> = props => {
const history = useHistory()
const [collapsedSections, setCollapsedSections] = useTemporarySetting('search.collapsedSidebarSections', {})
const query = useNavbarQueryState(state => state.queryState.query)
const setQueryState = useNavbarQueryState(state => state.setQueryState)
const toggleFilter = useCallback(
(value: string) => {
const newQuery = toggleSearchFilter(props.query, value)
const newQuery = toggleSearchFilter(query, value)
submitSearch({ ...props, query: newQuery, source: 'filter', history })
},
[history, props]
[history, props, query]
)
// Unlike onFilterClicked, this function will always append or update a filter
const updateOrAppendFilter = useCallback(
(filter: string, value: string) => {
const newQuery = updateFilter(props.query, filter, value)
const newQuery = updateFilter(query, filter, value)
submitSearch({ ...props, query: newQuery, source: 'filter', history })
},
[history, props]
[history, props, query]
)
const onDynamicFilterClicked = useCallback(
@ -114,12 +114,17 @@ export const SearchSidebar: React.FunctionComponent<SearchSidebarProps> = props
)
const repoFilters = useMemo(() => getFiltersOfKind(props.filters, FilterType.repo), [props.filters])
const repoName = useLastRepoName(props.query, repoFilters)
const repoName = useLastRepoName(query, repoFilters)
const repoFilterLinks = useMemo(() => getRepoFilterLinks(repoFilters, onDynamicFilterClicked), [
repoFilters,
onDynamicFilterClicked,
])
const showReposSection = repoFilterLinks.length > 1
const sectionProps = useMemo(() => ({ ...props, query, onNavbarQueryChange: setQueryState }), [
props,
query,
setQueryState,
])
let body
@ -135,7 +140,7 @@ export const SearchSidebar: React.FunctionComponent<SearchSidebarProps> = props
startCollapsed={collapsedSections?.[SectionID.SEARCH_TYPES]}
onToggle={open => persistToggleState(SectionID.SEARCH_TYPES, open)}
>
{getSearchTypeLinks(props)}
{getSearchTypeLinks(sectionProps)}
</SearchSidebarSection>
<SearchSidebarSection
className={styles.searchSidebarItem}
@ -184,7 +189,7 @@ export const SearchSidebar: React.FunctionComponent<SearchSidebarProps> = props
// (false is just an arbitrary but static value)
clearSearchOnChange={false}
>
{getSearchReferenceFactory(props)}
{getSearchReferenceFactory(sectionProps)}
</SearchSidebarSection>
<SearchSidebarSection
className={styles.searchSidebarItem}

23
client/web/src/search/results/sidebar/SearchTypeLink.tsx
View File

@ -21,7 +21,6 @@ export interface SearchTypeLinksProps
VersionContextProps,
Pick<SearchContextProps, 'selectedSearchContextSpec'> {
query: string
navbarSearchQueryState: QueryState
onNavbarQueryChange: (queryState: QueryState) => void
}
@ -86,10 +85,7 @@ const SearchTypeButton: React.FunctionComponent<SearchTypeButtonProps> = ({ chil
*/
const SearchSymbol: React.FunctionComponent<Omit<SearchTypeLinkProps, 'type'>> = props => {
const type = 'symbol'
const {
navbarSearchQueryState: { query },
onNavbarQueryChange,
} = props
const { query, onNavbarQueryChange } = props
const setSymbolSearch = useCallback(() => {
onNavbarQueryChange({
@ -97,7 +93,7 @@ const SearchSymbol: React.FunctionComponent<Omit<SearchTypeLinkProps, 'type'>> =
})
}, [query, onNavbarQueryChange])
if (containsLiteralOrPattern(props.navbarSearchQueryState.query)) {
if (containsLiteralOrPattern(query)) {
return (
<SearchTypeLink {...props} type={type}>
{props.children}
@ -111,16 +107,11 @@ const repoExample = createQueryExampleFromString('{regexp-pattern}')
export const getSearchTypeLinks = (props: SearchTypeLinksProps): ReactElement[] => {
function updateQueryWithRepoExample(): void {
const updatedQuery = updateQueryWithFilterAndExample(
props.navbarSearchQueryState.query,
FilterType.repo,
repoExample,
{
singular: false,
negate: false,
emptyValue: true,
}
)
const updatedQuery = updateQueryWithFilterAndExample(props.query, FilterType.repo, repoExample, {
singular: false,
negate: false,
emptyValue: true,
})
props.onNavbarQueryChange({
changeSource: QueryChangeSource.searchTypes,
query: updatedQuery.query,

3
package.json
View File

@ -404,7 +404,8 @@
"uuid": "^8.3.0",
"webext-domain-permission-toggle": "^1.0.1",
"webext-patterns": "^0.9.0",
"webextension-polyfill": "^0.6.0"
"webextension-polyfill": "^0.6.0",
"zustand": "^3.5.10"
},
"resolutions": {
"history": "4.5.1",

5
yarn.lock
View File

@ -24406,6 +24406,11 @@ zip-stream@^1.2.0:
lodash "^4.8.0"
readable-stream "^2.0.0"
zustand@^3.5.10:
version "3.5.10"
resolved "https://registry.npmjs.org/zustand/-/zustand-3.5.10.tgz#d2622efd64530ffda285ee5b13ff645b68ab0faf"
integrity sha512-upluvSRWrlCiExu2UbkuMIPJ9AigyjRFoO7O9eUossIj7rPPq7pcJ0NKk6t2P7KF80tg/UdPX6/pNKOSbs9DEg==
zwitch@^1.0.0:
version "1.0.5"
resolved "https://registry.npmjs.org/zwitch/-/zwitch-1.0.5.tgz#d11d7381ffed16b742f6af7b3f223d5cd9fe9920"