fix(query-builder): Modify callbacks to include more information and only fire when necessary (#75082)

The onChange was firing far too frequently, because the `onChange`
passed in was not memoized. We can prevent this by tracking
`previousQuery`.

Also added some extra info to the callback, like whether the query is
valid and the parsed contents since that should be useful for some
implementations.

Author: malwilley

static/app/components/searchQueryBuilder/hooks/useHandleSearch.tsx

@@ -3,7 +3,9 @@ import * as Sentry from '@sentry/react';
 
 import {saveRecentSearch} from 'sentry/actionCreators/savedSearches';
 import type {Client} from 'sentry/api';
+import type {CallbackSearchState} from 'sentry/components/searchQueryBuilder/types';
 import {
+  queryIsValid,
   recentSearchTypeToLabel,
   tokenIsInvalid,
 } from 'sentry/components/searchQueryBuilder/utils';
@@ -18,7 +20,7 @@ type UseHandleSearchProps = {
   parsedQuery: ParseResult | null;
   recentSearches: SavedSearchType | undefined;
   searchSource: string;
-  onSearch?: (query: string) => void;
+  onSearch?: (query: string, state: CallbackSearchState) => void;
 };
 
 async function saveAsRecentSearch({
@@ -97,7 +99,7 @@ export function useHandleSearch({
 
   return useCallback(
     (query: string) => {
-      onSearch?.(query);
+      onSearch?.(query, {parsedQuery, queryIsValid: queryIsValid(parsedQuery)});
 
       const searchType = recentSearchTypeToLabel(recentSearches);
 

static/app/components/searchQueryBuilder/index.spec.tsx

@@ -132,27 +132,35 @@ describe('SearchQueryBuilder', function () {
       render(
         <SearchQueryBuilder
           {...defaultProps}
-          initialQuery=""
+          initialQuery="a"
           onChange={mockOnChange}
           onBlur={mockOnBlur}
           onSearch={mockOnSearch}
         />
       );
 
       await userEvent.click(getLastInput());
-      await userEvent.keyboard('foo{enter}');
+      await userEvent.keyboard('b{enter}');
+
+      const expectedQueryState = expect.objectContaining({
+        parsedQuery: expect.arrayContaining([expect.any(Object)]),
+        queryIsValid: true,
+      });
 
       // Should call onChange and onSearch after enter
       await waitFor(() => {
-        expect(mockOnChange).toHaveBeenCalledWith('foo');
-        expect(mockOnSearch).toHaveBeenCalledWith('foo');
+        expect(mockOnChange).toHaveBeenCalledTimes(1);
+        expect(mockOnChange).toHaveBeenCalledWith('ab', expectedQueryState);
+        expect(mockOnSearch).toHaveBeenCalledTimes(1);
+        expect(mockOnSearch).toHaveBeenCalledWith('ab', expectedQueryState);
       });
 
       await userEvent.click(document.body);
 
       // Clicking outside activates onBlur
       await waitFor(() => {
-        expect(mockOnBlur).toHaveBeenCalledWith('foo');
+        expect(mockOnBlur).toHaveBeenCalledTimes(1);
+        expect(mockOnBlur).toHaveBeenCalledWith('ab', expectedQueryState);
       });
     });
   });
@@ -172,8 +180,8 @@ describe('SearchQueryBuilder', function () {
       userEvent.click(screen.getByRole('button', {name: 'Clear search query'}));
 
       await waitFor(() => {
-        expect(mockOnChange).toHaveBeenCalledWith('');
-        expect(mockOnSearch).toHaveBeenCalledWith('');
+        expect(mockOnChange).toHaveBeenCalledWith('', expect.anything());
+        expect(mockOnSearch).toHaveBeenCalledWith('', expect.anything());
       });
 
       expect(
@@ -253,7 +261,10 @@ describe('SearchQueryBuilder', function () {
       expect(screen.getByRole('textbox')).toHaveValue('browser.name:firefox assigned:me');
 
       await waitFor(() => {
-        expect(mockOnChange).toHaveBeenLastCalledWith('browser.name:firefox assigned:me');
+        expect(mockOnChange).toHaveBeenLastCalledWith(
+          'browser.name:firefox assigned:me',
+          expect.anything()
+        );
       });
     });
   });
@@ -534,7 +545,7 @@ describe('SearchQueryBuilder', function () {
       await userEvent.click(getLastInput());
       await userEvent.type(screen.getByRole('combobox'), 'some free text{enter}');
       await waitFor(() => {
-        expect(mockOnSearch).toHaveBeenCalledWith('some free text');
+        expect(mockOnSearch).toHaveBeenCalledWith('some free text', expect.anything());
       });
       // Should still have text in the input
       expect(screen.getByRole('combobox')).toHaveValue('some free text');
@@ -831,7 +842,7 @@ describe('SearchQueryBuilder', function () {
 
       // Pressing delete should remove all selected tokens
       await userEvent.keyboard('{Backspace}');
-      expect(mockOnChange).toHaveBeenCalledWith('');
+      expect(mockOnChange).toHaveBeenCalledWith('', expect.anything());
     });
 
     it('focus goes to first input after ctrl+a and arrow left', async function () {
@@ -933,7 +944,7 @@ describe('SearchQueryBuilder', function () {
       await userEvent.keyboard('{Control>}x{/Control}');
 
       expect(navigator.clipboard.writeText).toHaveBeenCalledWith('browser.name:firefox');
-      expect(mockOnChange).toHaveBeenCalledWith('');
+      expect(mockOnChange).toHaveBeenCalledWith('', expect.anything());
     });
 
     it('can undo last action with ctrl-z', async function () {
@@ -1118,7 +1129,7 @@ describe('SearchQueryBuilder', function () {
         );
         await userEvent.click(await screen.findByRole('option', {name: 'does not have'}));
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith('!has:key');
+          expect(mockOnChange).toHaveBeenCalledWith('!has:key', expect.anything());
         });
         expect(
           within(
@@ -1311,7 +1322,10 @@ describe('SearchQueryBuilder', function () {
 
         // Value should be surrounded by quotes and escaped
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith(`browser.name:${expected}`);
+          expect(mockOnChange).toHaveBeenCalledWith(
+            `browser.name:${expected}`,
+            expect.anything()
+          );
         });
       });
 
@@ -1826,7 +1840,7 @@ describe('SearchQueryBuilder', function () {
         ).toBeInTheDocument();
 
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith('foo age:-1h');
+          expect(mockOnChange).toHaveBeenCalledWith('foo age:-1h', expect.anything());
         });
       });
 
@@ -1854,7 +1868,7 @@ describe('SearchQueryBuilder', function () {
         ).toBeInTheDocument();
 
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith('foo age:+1h');
+          expect(mockOnChange).toHaveBeenCalledWith('foo age:+1h', expect.anything());
         });
       });
 
@@ -1876,7 +1890,7 @@ describe('SearchQueryBuilder', function () {
         await userEvent.click(screen.getByRole('button', {name: 'Save'}));
 
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith('age:>2017-10-17');
+          expect(mockOnChange).toHaveBeenCalledWith('age:>2017-10-17', expect.anything());
         });
       });
 
@@ -1899,7 +1913,10 @@ describe('SearchQueryBuilder', function () {
         await userEvent.click(await screen.findByRole('button', {name: 'Save'}));
 
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith('age:>2017-10-17T00:00:00Z');
+          expect(mockOnChange).toHaveBeenCalledWith(
+            'age:>2017-10-17T00:00:00Z',
+            expect.anything()
+          );
         });
       });
 
@@ -1923,7 +1940,10 @@ describe('SearchQueryBuilder', function () {
         await userEvent.click(await screen.findByRole('button', {name: 'Save'}));
 
         await waitFor(() => {
-          expect(mockOnChange).toHaveBeenCalledWith('age:>2017-10-17T00:00:00+00:00');
+          expect(mockOnChange).toHaveBeenCalledWith(
+            'age:>2017-10-17T00:00:00+00:00',
+            expect.anything()
+          );
         });
       });
 

static/app/components/searchQueryBuilder/index.tsx

@@ -12,11 +12,15 @@ import {useQueryBuilderState} from 'sentry/components/searchQueryBuilder/hooks/u
 import {PlainTextQueryInput} from 'sentry/components/searchQueryBuilder/plainTextQueryInput';
 import {TokenizedQueryGrid} from 'sentry/components/searchQueryBuilder/tokenizedQueryGrid';
 import {
+  type CallbackSearchState,
   type FieldDefinitionGetter,
   type FilterKeySection,
   QueryInterfaceType,
 } from 'sentry/components/searchQueryBuilder/types';
-import {parseQueryBuilderValue} from 'sentry/components/searchQueryBuilder/utils';
+import {
+  parseQueryBuilderValue,
+  queryIsValid,
+} from 'sentry/components/searchQueryBuilder/utils';
 import type {SearchConfig} from 'sentry/components/searchSyntax/parser';
 import {IconClose, IconSearch} from 'sentry/icons';
 import {t} from 'sentry/locale';
@@ -26,6 +30,7 @@ import {getFieldDefinition} from 'sentry/utils/fields';
 import PanelProvider from 'sentry/utils/panelProvider';
 import {useDimensions} from 'sentry/utils/useDimensions';
 import {useEffectAfterFirstRender} from 'sentry/utils/useEffectAfterFirstRender';
+import usePrevious from 'sentry/utils/usePrevious';
 
 export interface SearchQueryBuilderProps {
   /**
@@ -74,15 +79,15 @@ export interface SearchQueryBuilderProps {
    */
   invalidMessages?: SearchConfig['invalidMessages'];
   label?: string;
-  onBlur?: (query: string) => void;
+  onBlur?: (query: string, state: CallbackSearchState) => void;
   /**
    * Called when the query value changes
    */
-  onChange?: (query: string) => void;
+  onChange?: (query: string, state: CallbackSearchState) => void;
   /**
    * Called when the user presses enter
    */
-  onSearch?: (query: string) => void;
+  onSearch?: (query: string, state: CallbackSearchState) => void;
   placeholder?: string;
   queryInterface?: QueryInterfaceType;
   /**
@@ -169,9 +174,12 @@ export function SearchQueryBuilder({
     dispatch({type: 'UPDATE_QUERY', query: initialQuery});
   }, [dispatch, initialQuery]);
 
+  const previousQuery = usePrevious(state.query);
   useEffectAfterFirstRender(() => {
-    onChange?.(state.query);
-  }, [onChange, state.query]);
+    if (previousQuery !== state.query) {
+      onChange?.(state.query, {parsedQuery, queryIsValid: queryIsValid(parsedQuery)});
+    }
+  }, [onChange, state.query, previousQuery, parsedQuery]);
 
   const handleSearch = useHandleSearch({
     parsedQuery,
@@ -222,7 +230,9 @@ export function SearchQueryBuilder({
       <PanelProvider>
         <Wrapper
           className={className}
-          onBlur={() => onBlur?.(state.query)}
+          onBlur={() =>
+            onBlur?.(state.query, {parsedQuery, queryIsValid: queryIsValid(parsedQuery)})
+          }
           ref={wrapperRef}
           aria-disabled={disabled}
         >

static/app/components/searchQueryBuilder/types.tsx

@@ -1,5 +1,6 @@
 import type {ReactNode} from 'react';
 
+import type {ParseResult} from 'sentry/components/searchSyntax/parser';
 import type {FieldDefinition} from 'sentry/utils/fields';
 
 export type FilterKeySection = {
@@ -19,3 +20,8 @@ export type FocusOverride = {
 };
 
 export type FieldDefinitionGetter = (key: string) => FieldDefinition | null;
+
+export type CallbackSearchState = {
+  parsedQuery: ParseResult | null;
+  queryIsValid: boolean;
+};

static/app/components/searchQueryBuilder/utils.tsx

@@ -165,6 +165,14 @@ export function tokenIsInvalid(token: TokenResult<Token>) {
   return Boolean(token.invalid);
 }
 
+export function queryIsValid(parsedQuery: ParseResult | null) {
+  if (!parsedQuery) {
+    return false;
+  }
+
+  return !parsedQuery.some(tokenIsInvalid);
+}
+
 export function isDateToken(token: TokenResult<Token.FILTER>) {
   return [FilterType.DATE, FilterType.RELATIVE_DATE, FilterType.SPECIFIC_DATE].includes(
     token.filter