diff --git a/dashboards/src/components/Variables/Variable.test.ts b/dashboards/src/components/Variables/Variable.test.ts
index 90f13d25..f52edb40 100644
--- a/dashboards/src/components/Variables/Variable.test.ts
+++ b/dashboards/src/components/Variables/Variable.test.ts
@@ -59,6 +59,25 @@ interface TestParams {
}
describe('useListVariableState', () => {
+ it('keeps normalized All selections stable across renders', () => {
+ const spec = {
+ name: 'example',
+ plugin: { kind: 'StaticListVariable', spec: {} },
+ allowMultiple: true,
+ allowAllValue: true,
+ };
+ const state = { value: '$__all', loading: false };
+ const query = { data: [option('hello')], isFetching: false };
+ const { result, rerender } = renderHook(() => useListVariableState(spec, state, query));
+ const previous = result.current;
+
+ rerender();
+
+ expect(result.current.value).toBe(previous.value);
+ expect(result.current.selectedOptions).toBe(previous.selectedOptions);
+ expect(result.current.value).toEqual(['$__all']);
+ });
+
it.each([
{
description: '[!ALL][!MULTIPLE] is fetching',
diff --git a/dashboards/src/components/Variables/Variable.tsx b/dashboards/src/components/Variables/Variable.tsx
index 269ebc0d..c921273f 100644
--- a/dashboards/src/components/Variables/Variable.tsx
+++ b/dashboards/src/components/Variables/Variable.tsx
@@ -81,12 +81,13 @@ export function useListVariableState(
const loading = useMemo(() => variablesOptionsQuery.isFetching ?? false, [variablesOptionsQuery.isFetching]);
const options = useMemo(() => variablesOptionsQuery.data ?? [], [variablesOptionsQuery.data]);
- let initialValue = state?.value;
-
- // Make sure initialValue is an array if allowMultiple is true
- if (allowMultiple && !Array.isArray(initialValue)) {
- initialValue = typeof initialValue === 'string' ? [initialValue] : [];
- }
+ const initialValue = useMemo(() => {
+ const currentValue = state?.value;
+ if (allowMultiple && !Array.isArray(currentValue)) {
+ return typeof currentValue === 'string' ? [currentValue] : [];
+ }
+ return currentValue;
+ }, [allowMultiple, state?.value]);
// Sort the provided list of options according to the method defined
const sortedOptions = useMemo((): VariableOption[] => {
@@ -138,9 +139,8 @@ export function useListVariableState(
// In the case Autocomplete.multiple equals false, Autocomplete.value expects a single object, not
// an array, hence this conditional
if (Array.isArray(value)) {
- return viewOptions.filter((o) => {
- return value?.includes(o.value);
- });
+ const selectedValues = new Set(value);
+ return viewOptions.filter((option) => selectedValues.has(option.value));
} else {
return (
viewOptions.find((o) => {
@@ -157,6 +157,8 @@ const StyledPopper = (props: PopperProps): ReactElement => (
);
+const filterOptions = createFilterOptions();
+
const LETTER_HSIZE = 8; // approximation
const ARROW_OFFSET = 40; // right offset for list variables (= take into account the dropdown toggle size)
const getWidthPx = (inputValue: string, kind: 'list' | 'text'): number => {
@@ -188,11 +190,9 @@ function ListVariable({ name, source }: VariableProps): ReactElement {
const allowMultiple = definition?.spec.allowMultiple === true;
const allowAllValue = definition?.spec.allowAllValue === true;
- const filterOptions = createFilterOptions({});
-
const filteredOptions = useMemo(
() => filterOptions(viewOptions, { inputValue, getOptionLabel: (o) => o.label }),
- [inputValue, viewOptions, filterOptions],
+ [inputValue, viewOptions],
);
// Update value when changed
@@ -320,7 +320,6 @@ function ListVariable({ name, source }: VariableProps): ReactElement {
}, [
allowAllValue,
allowMultiple,
- filterOptions,
inputValue,
inputWidth,
loading,
diff --git a/dashboards/src/context/VariableProvider/VariableProvider.performance.test.tsx b/dashboards/src/context/VariableProvider/VariableProvider.performance.test.tsx
new file mode 100644
index 00000000..ec1dd3cb
--- /dev/null
+++ b/dashboards/src/context/VariableProvider/VariableProvider.performance.test.tsx
@@ -0,0 +1,197 @@
+// Copyright The Perses Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import type * as PluginSystem from '@perses-dev/plugin-system';
+import { TimeRangeProviderBasic, useVariableValues } from '@perses-dev/plugin-system';
+import type { VariableDefinition } from '@perses-dev/spec';
+import { DEFAULT_ALL_VALUE } from '@perses-dev/spec';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { act, fireEvent, render, renderHook, screen } from '@testing-library/react';
+import type { ReactElement, ReactNode } from 'react';
+import { Suspense, useState } from 'react';
+
+import { Variable } from '../../components/Variables/Variable';
+import type * as QueryParams from './query-params';
+import {
+ useVariableDefinitionStoreCtx,
+ useVariableDefinitionActions,
+ useVariableDefinitionAndState,
+ useVariableDefinitionStates,
+ VariableProvider,
+ VariableProviderWithQueryParams,
+} from './VariableProvider';
+
+const options = [
+ { value: 'dev', label: 'Development' },
+ { value: 'prod', label: 'Production' },
+];
+const variableQuery = { data: options, isFetching: false };
+vi.mock('@perses-dev/plugin-system', async (importOriginal) => ({
+ ...(await importOriginal()),
+ useListVariablePluginValues: (): typeof variableQuery => variableQuery,
+}));
+
+const setQueryParams = vi.fn();
+const emptyQueryValues = {};
+vi.mock('./query-params', async (importOriginal) => ({
+ ...(await importOriginal()),
+ useVariableQueryParams: (): ReturnType => [
+ emptyQueryValues,
+ setQueryParams,
+ ],
+}));
+const chartFallback = Loading chart
;
+
+const definitions: VariableDefinition[] = [
+ { kind: 'TextVariable', spec: { name: 'region', value: 'west' } },
+ {
+ kind: 'ListVariable',
+ spec: {
+ name: 'environment',
+ defaultValue: 'dev',
+ allowAllValue: true,
+ allowMultiple: false,
+ plugin: { kind: 'StaticListVariable', spec: {} },
+ },
+ },
+];
+const initialTimeRange = { pastDuration: '30m' } as const;
+
+function Wrapper({ children, queryParams = false }: { children: ReactNode; queryParams?: boolean }): ReactElement {
+ const Provider = queryParams ? VariableProviderWithQueryParams : VariableProvider;
+ const [queryClient] = useState(() => new QueryClient());
+ return (
+
+
+ {children}
+
+
+ );
+}
+
+function QueryParamsWrapper({ children }: { children: ReactNode }): ReactElement {
+ return {children};
+}
+
+function SlowVariableConsumer({ read }: { read: (value: string) => void }): ReactElement {
+ const values = useVariableValues();
+ const value = values.environment?.value as string;
+ read(value);
+ return Chart: {value}
;
+}
+
+it('does not notify subscribers or rewrite the URL for identical normalized values', () => {
+ setQueryParams.mockClear();
+ const { result } = renderHook(() => useVariableDefinitionStoreCtx(), { wrapper: QueryParamsWrapper });
+ const store = result.current;
+ const onChange = vi.fn();
+ store.subscribe(onChange);
+ const actions = store.getState();
+
+ act(() => actions.setVariableValue('environment', 'dev'));
+ expect(onChange).not.toHaveBeenCalled();
+ expect(setQueryParams).not.toHaveBeenCalled();
+
+ act(() => actions.setVariableValue('environment', ['prod']));
+ act(() => actions.setVariableValue('environment', ['prod']));
+ expect(onChange).toHaveBeenCalledTimes(1);
+ expect(setQueryParams).toHaveBeenCalledTimes(1);
+
+ act(() => actions.setVariableValue('environment', ['prod', DEFAULT_ALL_VALUE]));
+ act(() => actions.setVariableValue('environment', [DEFAULT_ALL_VALUE]));
+ expect(store.getState().variableState.get({ name: 'environment' })?.value).toBe(DEFAULT_ALL_VALUE);
+ expect(onChange).toHaveBeenCalledTimes(2);
+ expect(setQueryParams).toHaveBeenCalledTimes(2);
+
+ act(() => actions.setVariableValue('environment', [DEFAULT_ALL_VALUE, 'prod']));
+ expect(store.getState().variableState.get({ name: 'environment' })?.value).toEqual(['prod']);
+ expect(setQueryParams).toHaveBeenLastCalledWith({ 'var-environment': ['prod'] });
+});
+
+it('does not publish identical options or loading state again', () => {
+ const { result } = renderHook(() => useVariableDefinitionStoreCtx(), { wrapper: Wrapper });
+ const store = result.current;
+ const actions = store.getState();
+ act(() => actions.setVariableOptions('environment', options));
+ const previous = store.getState();
+ const onChange = vi.fn();
+ store.subscribe(onChange);
+
+ act(() =>
+ actions.setVariableOptions(
+ 'environment',
+ options.map((option) => Object.assign({}, option)),
+ ),
+ );
+ act(() => actions.setVariableLoading('environment', false));
+ expect(store.getState()).toBe(previous);
+ expect(onChange).not.toHaveBeenCalled();
+
+ act(() => actions.setVariableOptions('environment', [{ value: 'dev', label: 'Renamed development' }]));
+ expect(onChange).toHaveBeenCalledTimes(1);
+});
+
+it('keeps unrelated input and variable-state subscriptions unchanged', () => {
+ const { result } = renderHook(
+ () => ({
+ variable: useVariableDefinitionAndState('environment'),
+ states: useVariableDefinitionStates(['environment']),
+ actions: useVariableDefinitionActions(),
+ }),
+ { wrapper: Wrapper },
+ );
+ const previous = result.current;
+
+ act(() => result.current.actions.setVariableValue('region', 'east'));
+ expect(result.current).toBe(previous);
+
+ act(() => result.current.actions.setVariableValue('environment', 'prod'));
+ expect(result.current.variable.state?.value).toBe('prod');
+ expect(result.current.states.environment?.value).toBe('prod');
+});
+
+it('updates the selected input while a chart is still rendering the new variable value', async () => {
+ let ready = false;
+ let finish!: () => void;
+ const pending = new Promise((resolve) => {
+ finish = resolve;
+ });
+ const read = vi.fn((value: string): void => {
+ if (value === 'prod' && !ready) throw pending;
+ });
+ render(
+
+
+
+
+
+ ,
+ );
+ const input = screen.getByRole('combobox', { name: 'environment' });
+ expect(input).toHaveValue('Development');
+ const initialReads = read.mock.calls.filter(([value]) => value === 'dev').length;
+ fireEvent.mouseDown(input);
+ fireEvent.click(screen.getByRole('option', { name: 'Production' }));
+
+ expect(input).toHaveValue('Production');
+ expect(read.mock.calls.filter(([value]) => value === 'dev')).toHaveLength(initialReads);
+ expect(screen.getByText('Chart: dev')).toBeInTheDocument();
+ expect(screen.queryByText('Loading chart')).not.toBeInTheDocument();
+
+ await act(async () => {
+ ready = true;
+ finish();
+ await pending;
+ });
+ expect(screen.getByText('Chart: prod')).toBeInTheDocument();
+});
diff --git a/dashboards/src/context/VariableProvider/VariableProvider.tsx b/dashboards/src/context/VariableProvider/VariableProvider.tsx
index fa983f4b..239b8403 100644
--- a/dashboards/src/context/VariableProvider/VariableProvider.tsx
+++ b/dashboards/src/context/VariableProvider/VariableProvider.tsx
@@ -30,7 +30,7 @@ import type {
import { DEFAULT_ALL_VALUE as ALL_VALUE, formatDuration, intervalToDuration } from '@perses-dev/spec';
import { produce } from 'immer';
import type { ReactElement, ReactNode } from 'react';
-import { createContext, useContext, useEffect, useMemo, useState } from 'react';
+import { createContext, useContext, useDeferredValue, useEffect, useMemo, useState } from 'react';
import type { StoreApi } from 'zustand';
import { createStore, useStore } from 'zustand';
import { devtools } from 'zustand/middleware';
@@ -150,9 +150,7 @@ export function useVariableDefinitionStates(variableNames?: string[]): VariableS
return varStates;
},
- (left, right) => {
- return JSON.stringify(left) === JSON.stringify(right);
- },
+ shallow,
);
}
@@ -201,15 +199,19 @@ export function useVariableDefinitionAndState(
state: VariableState | undefined;
} {
const store = useVariableDefinitionStoreCtx();
- return useStore(store, (s) => {
- const state = s.variableState.get({ name, source });
- const definitions = source
- ? s.externalVariableDefinitions.find((v) => v.source === source)?.definitions
- : s.variableDefinitions;
- const definition = (definitions || []).find((v) => v.spec.name === name);
-
- return { state, definition };
- });
+ return useStoreWithEqualityFn(
+ store,
+ (s) => {
+ const state = s.variableState.get({ name, source });
+ const definitions = source
+ ? s.externalVariableDefinitions.find((v) => v.source === source)?.definitions
+ : s.variableDefinitions;
+ const definition = (definitions || []).find((v) => v.spec.name === name);
+
+ return { state, definition };
+ },
+ shallow,
+ );
}
export function useVariableDefinitionActions(): {
@@ -253,7 +255,8 @@ interface PluginProviderProps {
}
function PluginProvider({ children, builtinVariables }: PluginProviderProps): ReactElement {
- const originalValues = useVariableDefinitionStates();
+ // Inputs read the store immediately; chart and query consumers can render in the background.
+ const originalValues = useDeferredValue(useVariableDefinitionStates());
const definitions = useVariableDefinitions();
const externalDefinitions = useExternalVariableDefinitions();
const { absoluteTimeRange } = useTimeRange();
@@ -356,9 +359,12 @@ function PluginProvider({ children, builtinVariables }: PluginProviderProps): Re
return result;
}, [absoluteTimeRange, builtinVariables]);
+ const builtinContext = useMemo(() => ({ variables: allBuiltinVariables }), [allBuiltinVariables]);
+ const variableContext = useMemo(() => ({ state: values }), [values]);
+
return (
-
- {children}
+
+ {children}
);
}
@@ -423,6 +429,16 @@ function createVariableDefinitionStore({
);
},
setVariableOptions(name, options, source?: string): void {
+ const currentOptions = get().variableState.get({ name, source })?.options;
+ if (currentOptions === options) return;
+ if (
+ currentOptions?.length === options.length &&
+ currentOptions.every(
+ (option, index) => option.value === options[index]?.value && option.label === options[index]?.label,
+ )
+ ) {
+ return;
+ }
set(
(state) => {
const varState = state.variableState.get({ name, source });
@@ -436,6 +452,7 @@ function createVariableDefinitionStore({
);
},
setVariableLoading(name, loading, source?: string): void {
+ if (get().variableState.get({ name, source })?.loading === loading) return;
set(
(state) => {
const varState = state.variableState.get({ name, source });
@@ -448,32 +465,31 @@ function createVariableDefinitionStore({
'[Variables] setVariableLoading',
);
},
- setVariableValue: (name, value, source?: string): void =>
+ setVariableValue: (name, value, source?: string): void => {
+ const currentState = get().variableState.get({ name, source });
+ if (!currentState) return;
+
+ let nextValue = value;
+ // Normalize the All option before comparing, including repeated multi-select updates.
+ if (Array.isArray(nextValue) && nextValue.includes(ALL_VALUE)) {
+ nextValue = nextValue.at(-1) === ALL_VALUE ? ALL_VALUE : nextValue.filter((v) => v !== ALL_VALUE);
+ }
+ if (areVariableValuesEqual(currentState.value, nextValue)) return;
+
set(
(state) => {
- let val = value;
const varState = state.variableState.get({ name, source });
if (!varState) {
return;
}
- // Make sure there is only one all value
- if (Array.isArray(val) && val.includes(ALL_VALUE)) {
- if (val.at(-1) === ALL_VALUE) {
- val = ALL_VALUE;
- } else {
- val = val.filter((v) => v !== ALL_VALUE);
- }
- }
- if (queryParams) {
- const setQueryParams = queryParams[1];
- setQueryParams({ [getURLQueryParamName(name)]: val });
- }
- varState.value = val;
+ varState.value = nextValue;
},
false,
'[Variables] setVariableValue',
- ),
+ );
+ queryParams?.[1]({ [getURLQueryParamName(name)]: nextValue });
+ },
setVariableDefaultValues: (): VariableDefinition[] => {
const variableDefinitions = get().variableDefinitions;
const variableState = get().variableState;
@@ -543,12 +559,14 @@ export interface VariableProviderProps {
initialVariableValues?: Record;
}
+const EMPTY_BUILTIN_VARIABLE_DEFINITIONS: BuiltinVariableDefinition[] = [];
+
// TODO: merge the different providers related to Variables under a single one (and keep "VariableProvider" as a name)
export function VariableProvider({
children,
initialVariableDefinitions = [],
externalVariableDefinitions = [],
- builtinVariableDefinitions = [],
+ builtinVariableDefinitions = EMPTY_BUILTIN_VARIABLE_DEFINITIONS,
initialVariableValues,
}: VariableProviderProps): ReactElement {
const [store] = useState(() =>
@@ -566,7 +584,7 @@ export function VariableProviderWithQueryParams({
children,
initialVariableDefinitions = [],
externalVariableDefinitions = [],
- builtinVariableDefinitions: builtinVariables = [],
+ builtinVariableDefinitions: builtinVariables = EMPTY_BUILTIN_VARIABLE_DEFINITIONS,
}: VariableProviderProps): ReactElement {
const allVariableDefs = mergeVariableDefinitions(initialVariableDefinitions, externalVariableDefinitions);
const queryParams = useVariableQueryParams(allVariableDefs);