diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dd59a71bdb..8cfc03121e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/client/web/src/Layout.tsx b/client/web/src/Layout.tsx index bc6ab29ba74..6de099b3e89 100644 --- a/client/web/src/Layout.tsx +++ b/client/web/src/Layout.tsx @@ -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 // Search - navbarSearchQueryState: QueryState - onNavbarQueryChange: (queryState: QueryState) => void fetchHighlightedFileLineRanges: (parameters: FetchFileParameters, force?: boolean) => Observable globbing: boolean diff --git a/client/web/src/SourcegraphWebApp.tsx b/client/web/src/SourcegraphWebApp.tsx index c2baae01d7b..755320c0cf7 100644 --- a/client/web/src/SourcegraphWebApp.tsx +++ b/client/web/src/SourcegraphWebApp.tsx @@ -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 { - this.setState({ navbarSearchQueryState }) - } - private setParsedSearchQuery = (query: string): void => { this.setState({ parsedSearchQuery: query }) } diff --git a/client/web/src/nav/GlobalNavbar.story.tsx b/client/web/src/nav/GlobalNavbar.story.tsx index 6f07fd3c3b8..6b3e2240382 100644 --- a/client/web/src/nav/GlobalNavbar.story.tsx +++ b/client/web/src/nav/GlobalNavbar.story.tsx @@ -61,8 +61,6 @@ const defaultProps = ( defaultSearchContextSpec: '', showOnboardingTour: false, isLightTheme: props.isLightTheme, - navbarSearchQueryState: { query: '' }, - onNavbarQueryChange: () => {}, isExtensionAlertAnimating: false, batchChangesEnabled: true, batchChangesExecutionEnabled: true, diff --git a/client/web/src/nav/GlobalNavbar.test.tsx b/client/web/src/nav/GlobalNavbar.test.tsx index 72604b889e4..2dca854d325 100644 --- a/client/web/src/nav/GlobalNavbar.test.tsx +++ b/client/web/src/nav/GlobalNavbar.test.tsx @@ -27,8 +27,6 @@ const PROPS: React.ComponentProps = { history: createMemoryHistory(), keyboardShortcuts: [], isSourcegraphDotCom: false, - navbarSearchQueryState: { query: 'q' }, - onNavbarQueryChange: () => undefined, onThemePreferenceChange: () => undefined, isLightTheme: true, themePreference: ThemePreference.Light, diff --git a/client/web/src/nav/GlobalNavbar.tsx b/client/web/src/nav/GlobalNavbar.tsx index 1e2a50a2b40..08f074e2ab5 100644 --- a/client/web/src/nav/GlobalNavbar.tsx +++ b/client/web/src/nav/GlobalNavbar.tsx @@ -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 = ({ authRequired, showSearchBox, - navbarSearchQueryState, caseSensitive, patternType, - onNavbarQueryChange, hideNavLinks, variant, isLightTheme, @@ -154,6 +150,8 @@ export const GlobalNavbar: React.FunctionComponent = ({ ) ) + 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 = ({ const searchNavBar = ( void history: H.History globbing: boolean showSearchNotebook: boolean @@ -314,7 +313,8 @@ export const RepoContainer: React.FunctionComponent = 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) { diff --git a/client/web/src/search/input/SearchNavbarItem.tsx b/client/web/src/search/input/SearchNavbarItem.tsx index b5ad83ddc4e..2b9b225880b 100644 --- a/client/web/src/search/input/SearchNavbarItem.tsx +++ b/client/web/src/search/input/SearchNavbarItem.tsx @@ -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 @@ -52,12 +51,14 @@ export const SearchNavbarItem: React.FunctionComponent = (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) = void +} +export const useNavbarQueryState = create(set => ({ + queryState: { query: '' }, + setQueryState: queryState => set({ queryState }), +})) diff --git a/client/web/src/search/results/StreamingSearchResults.story.tsx b/client/web/src/search/results/StreamingSearchResults.story.tsx index ee7f3cb20bf..1a1a1cee3fb 100644 --- a/client/web/src/search/results/StreamingSearchResults.story.tsx +++ b/client/web/src/search/results/StreamingSearchResults.story.tsx @@ -55,8 +55,6 @@ const defaultProps: StreamingSearchResultsProps = { } as AuthenticatedUser, isLightTheme: true, - navbarSearchQueryState: { query: '' }, - onNavbarQueryChange: () => {}, isSourcegraphDotCom: false, settingsCascade: { diff --git a/client/web/src/search/results/StreamingSearchResults.test.tsx b/client/web/src/search/results/StreamingSearchResults.test.tsx index 1ba95d9487d..33ca00605f9 100644 --- a/client/web/src/search/results/StreamingSearchResults.test.tsx +++ b/client/web/src/search/results/StreamingSearchResults.test.tsx @@ -52,8 +52,6 @@ describe('StreamingSearchResults', () => { location: history.location, authenticatedUser: null, - navbarSearchQueryState: { query: '' }, - onNavbarQueryChange: () => {}, isSourcegraphDotCom: false, settingsCascade: { diff --git a/client/web/src/search/results/StreamingSearchResults.tsx b/client/web/src/search/results/StreamingSearchResults.tsx index 17406110ed4..3432f4eb883 100644 --- a/client/web/src/search/results/StreamingSearchResults.tsx +++ b/client/web/src/search/results/StreamingSearchResults.tsx @@ -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 @@ -244,7 +242,6 @@ export const StreamingSearchResults: React.FunctionComponent diff --git a/client/web/src/search/results/sidebar/SearchReference.tsx b/client/web/src/search/results/sidebar/SearchReference.tsx index 69a4cadf86d..4aa9ec5ba5f 100644 --- a/client/web/src/search/results/sidebar/SearchReference.tsx +++ b/client/web/src/search/results/sidebar/SearchReference.tsx @@ -483,7 +483,6 @@ export interface SearchReferenceProps Pick { 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 = ( diff --git a/client/web/src/search/results/sidebar/SearchSidebar.story.tsx b/client/web/src/search/results/sidebar/SearchSidebar.story.tsx index 495db3388c5..df71bc56469 100644 --- a/client/web/src/search/results/sidebar/SearchSidebar.story.tsx +++ b/client/web/src/search/results/sidebar/SearchSidebar.story.tsx @@ -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, } diff --git a/client/web/src/search/results/sidebar/SearchSidebar.tsx b/client/web/src/search/results/sidebar/SearchSidebar.tsx index 82a04d086c3..cd3e3f07dae 100644 --- a/client/web/src/search/results/sidebar/SearchSidebar.tsx +++ b/client/web/src/search/results/sidebar/SearchSidebar.tsx @@ -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 = 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 = 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 = props startCollapsed={collapsedSections?.[SectionID.SEARCH_TYPES]} onToggle={open => persistToggleState(SectionID.SEARCH_TYPES, open)} > - {getSearchTypeLinks(props)} + {getSearchTypeLinks(sectionProps)} = props // (false is just an arbitrary but static value) clearSearchOnChange={false} > - {getSearchReferenceFactory(props)} + {getSearchReferenceFactory(sectionProps)} { query: string - navbarSearchQueryState: QueryState onNavbarQueryChange: (queryState: QueryState) => void } @@ -86,10 +85,7 @@ const SearchTypeButton: React.FunctionComponent = ({ chil */ const SearchSymbol: React.FunctionComponent> = 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> = }) }, [query, onNavbarQueryChange]) - if (containsLiteralOrPattern(props.navbarSearchQueryState.query)) { + if (containsLiteralOrPattern(query)) { return ( {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, diff --git a/package.json b/package.json index 8fb58560f93..e2932afbcfb 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/yarn.lock b/yarn.lock index 69f4d5ae487..171a7804d77 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"