Skip to content

Commit

Permalink
Chore: Upgrade react-redux (grafana#77296)
Browse files Browse the repository at this point in the history
* try upgrading react-redux

* memoize getNavModel selector, don't return a new object in command palette selector

* use createSelector to memoize some alert manager selectors correctly

* memoize selectors correctly in appNotifications

* memoize some datasource selectors

* use fake timers to avoid the debounce causing flakiness

* remove duplicate import

* just use memoize-one
  • Loading branch information
ashharrison90 authored Oct 31, 2023
1 parent da1a53e commit 13c0268
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 66 deletions.
1 change: 0 additions & 1 deletion .github/renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"react-router-dom", // we should bump this together with history (see https://github.com/grafana/grafana/issues/76744)
"monaco-editor", // due to us exposing this via @grafana/ui/CodeEditor's props bumping can break plugins
"react-hook-form", // due to us exposing these hooks via @grafana/ui form components bumping can break plugins
"react-redux", // react-beautiful-dnd depends on react-redux 7.x, we need to update that one first
],
"includePaths": ["package.json", "packages/**", "public/app/plugins/**"],
"ignorePaths": ["emails/**", "plugins-bundled/**", "**/mocks/**", "packages/grafana-e2e/**"],
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@
"@types/react-dom": "18.2.7",
"@types/react-grid-layout": "1.3.2",
"@types/react-highlight-words": "0.16.4",
"@types/react-redux": "7.1.25",
"@types/react-router-dom": "5.3.3",
"@types/react-table": "7.7.14",
"@types/react-test-renderer": "18.0.0",
Expand Down Expand Up @@ -382,7 +381,7 @@
"react-moveable": "0.46.1",
"react-popper": "2.3.0",
"react-popper-tooltip": "4.4.2",
"react-redux": "7.2.6",
"react-redux": "8.1.3",
"react-resizable": "3.0.5",
"react-responsive-carousel": "^3.2.23",
"react-router-dom": "5.3.3",
Expand Down
12 changes: 7 additions & 5 deletions public/app/core/reducers/appNotification.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createSlice, PayloadAction } from '@reduxjs/toolkit';
import { createSelector, createSlice, PayloadAction } from '@reduxjs/toolkit';

import { AppNotification, AppNotificationSeverity, AppNotificationsState } from 'app/types/';

Expand Down Expand Up @@ -60,10 +60,12 @@ export const appNotificationsReducer = appNotificationsSlice.reducer;
// Selectors

export const selectLastReadTimestamp = (state: AppNotificationsState) => state.lastRead;
export const selectAll = (state: AppNotificationsState) =>
Object.values(state.byId).sort((a, b) => b.timestamp - a.timestamp);
export const selectWarningsAndErrors = (state: AppNotificationsState) => selectAll(state).filter(isAtLeastWarning);
export const selectVisible = (state: AppNotificationsState) => Object.values(state.byId).filter((n) => n.showing);
export const selectById = (state: AppNotificationsState) => state.byId;
export const selectAll = createSelector(selectById, (byId) =>
Object.values(byId).sort((a, b) => b.timestamp - a.timestamp)
);
export const selectWarningsAndErrors = createSelector(selectAll, (all) => all.filter(isAtLeastWarning));
export const selectVisible = createSelector(selectById, (byId) => Object.values(byId).filter((n) => n.showing));

// Helper functions

Expand Down
34 changes: 19 additions & 15 deletions public/app/core/selectors/navModel.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import memoizeOne from 'memoize-one';

import { NavModel, NavModelItem, NavIndex } from '@grafana/data';
import { newBrowseDashboardsEnabled } from 'app/features/browse-dashboards/featureFlag';
import { FOLDER_ID } from 'app/features/folders/state/navModel';
Expand All @@ -19,24 +21,26 @@ const getNotFoundModel = (): NavModel => {
};
};

export const getNavModel = (navIndex: NavIndex, id: string, fallback?: NavModel, onlyChild = false): NavModel => {
if (navIndex[id]) {
const node = navIndex[id];
const main = onlyChild ? node : getRootSectionForNode(node);
const mainWithActive = enrichNodeWithActiveState(main, id);
export const getNavModel = memoizeOne(
(navIndex: NavIndex, id: string, fallback?: NavModel, onlyChild = false): NavModel => {
if (navIndex[id]) {
const node = navIndex[id];
const main = onlyChild ? node : getRootSectionForNode(node);
const mainWithActive = enrichNodeWithActiveState(main, id);

return {
node: node,
main: mainWithActive,
};
}
return {
node: node,
main: mainWithActive,
};
}

if (fallback) {
return fallback;
}
if (fallback) {
return fallback;
}

return getNotFoundModel();
};
return getNotFoundModel();
}
);

export function getRootSectionForNode(node: NavModelItem): NavModelItem {
// Don't recurse fully up the folder tree when nested folders is enabled
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render } from '@testing-library/react';
import { act, render } from '@testing-library/react';
import { noop } from 'lodash';
import React from 'react';
import { AutoSizerProps } from 'react-virtualized-auto-sizer';
Expand Down Expand Up @@ -38,10 +38,21 @@ const ui = {
};

describe('DashboardPicker', () => {
beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.useRealTimers();
});

it('Renders panels without ids', async () => {
render(<DashboardPicker isOpen={true} onChange={noop} onDismiss={noop} dashboardUid="dash-2" panelId={2} />, {
wrapper: TestProvider,
});
act(() => {
jest.advanceTimersByTime(500);
});

expect(await ui.dashboardButton(/Dashboard 1/).find()).toBeInTheDocument();
expect(await ui.dashboardButton(/Dashboard 2/).find()).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { countBy, keyBy } from 'lodash';
import { createSelector } from 'reselect';

import { DataSourceInstanceSettings, DataSourceJsonData, DataSourceSettings } from '@grafana/data';
import { AlertManagerDataSourceJsonData } from 'app/plugins/datasource/alertmanager/types';
import { useSelector } from 'app/types';
import { StoreState, useSelector } from 'app/types';

import { alertmanagerApi } from '../api/alertmanagerApi';
import { getAlertManagerDataSources } from '../utils/datasource';
Expand All @@ -20,10 +21,10 @@ export function useExternalDataSourceAlertmanagers(): ExternalDataSourceAM[] {

const externalDsAlertManagers = getAlertManagerDataSources().filter((ds) => ds.jsonData.handleGrafanaManagedAlerts);

const alertmanagerDatasources = useSelector((state) =>
keyBy(
state.dataSources.dataSources.filter((ds) => ds.type === 'alertmanager'),
(ds) => ds.uid
const alertmanagerDatasources = useSelector(
createSelector(
(state: StoreState) => state.dataSources.dataSources.filter((ds) => ds.type === 'alertmanager'),
(datasources) => keyBy(datasources, (ds) => ds.uid)
)
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import { useSelector } from 'app/types';
import { createSelector } from 'reselect';

import { StoreState, useSelector } from 'app/types';

import { UnifiedAlertingState } from '../state/reducers';

export function useUnifiedAlertingSelector<TSelected = unknown>(
selector: (state: UnifiedAlertingState) => TSelected,
equalityFn?: (left: TSelected, right: TSelected) => boolean
): TSelected {
return useSelector((state) => selector(state.unifiedAlerting), equalityFn);
return useSelector(
createSelector(
(state: StoreState) => state.unifiedAlerting,
(unifiedAlerting) => selector(unifiedAlerting)
),
equalityFn
);
}
7 changes: 2 additions & 5 deletions public/app/features/commandPalette/actions/useActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,8 @@ export default function useActions(searchQuery: string) {
const [navTreeActions, setNavTreeActions] = useState<CommandPaletteAction[]>([]);
const [recentDashboardActions, setRecentDashboardActions] = useState<CommandPaletteAction[]>([]);

const { navBarTree } = useSelector((state) => {
return {
navBarTree: state.navBarTree,
};
});
const navBarTree = useSelector((state) => state.navBarTree);

// Load standard static actions
useEffect(() => {
const staticActionsResp = getStaticActions(navBarTree);
Expand Down
10 changes: 6 additions & 4 deletions public/app/features/datasources/state/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import memoizeOne from 'memoize-one';

import { DataSourcePluginMeta, DataSourceSettings, UrlQueryValue } from '@grafana/data';
import { DataSourcesState } from 'app/types/datasources';

export const getDataSources = (state: DataSourcesState) => {
export const getDataSources = memoizeOne((state: DataSourcesState) => {
const regex = new RegExp(state.searchQuery, 'i');

const filteredDataSources = state.dataSources.filter((dataSource: DataSourceSettings) => {
Expand All @@ -11,15 +13,15 @@ export const getDataSources = (state: DataSourcesState) => {
return filteredDataSources.sort((a, b) =>
state.isSortAscending ? a.name.localeCompare(b.name) : b.name.localeCompare(a.name)
);
};
});

export const getFilteredDataSourcePlugins = (state: DataSourcesState) => {
export const getFilteredDataSourcePlugins = memoizeOne((state: DataSourcesState) => {
const regex = new RegExp(state.dataSourceTypeSearchQuery, 'i');

return state.plugins.filter((type: DataSourcePluginMeta) => {
return regex.test(type.name);
});
};
});

export const getDataSource = (state: DataSourcesState, dataSourceId: UrlQueryValue): DataSourceSettings => {
if (state.dataSource.uid === dataSourceId) {
Expand Down
79 changes: 53 additions & 26 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6118,10 +6118,10 @@ __metadata:
languageName: node
linkType: hard

"@remix-run/router@npm:1.10.0":
version: 1.10.0
resolution: "@remix-run/router@npm:1.10.0"
checksum: 61f5e6374f2d0edd177a61c8a44715290d9197a61362505fb0784f948a39e8353d9604ce3747151f837fc7b950e1fbd71bc797b161643325c545037ffbedd134
"@remix-run/router@npm:1.5.0":
version: 1.5.0
resolution: "@remix-run/router@npm:1.5.0"
checksum: 3da27d64519df94919020f4b42aaa016f26891331be98468cbc807bc4d2cfb401d7e47d4f88a4a3d777fc3af23d162c668357a8e5d2c5947acdbca7b691bc325
languageName: node
linkType: hard

Expand Down Expand Up @@ -8501,6 +8501,16 @@ __metadata:
languageName: node
linkType: hard

"@types/hoist-non-react-statics@npm:^3.3.1":
version: 3.3.4
resolution: "@types/hoist-non-react-statics@npm:3.3.4"
dependencies:
"@types/react": "npm:*"
hoist-non-react-statics: "npm:^3.3.0"
checksum: dee430941a9ea16b7f665ecafa9b134066a49d13ae497fc051cf5d41b3aead394ab1a8179c3c98c9a3584f80aed16fab82dd7979c7dcddfbb5f74a132575d362
languageName: node
linkType: hard

"@types/html-minifier-terser@npm:^6.0.0":
version: 6.0.0
resolution: "@types/html-minifier-terser@npm:6.0.0"
Expand Down Expand Up @@ -8916,7 +8926,7 @@ __metadata:
languageName: node
linkType: hard

"@types/react-redux@npm:7.1.25, @types/react-redux@npm:^7.1.20":
"@types/react-redux@npm:^7.1.20":
version: 7.1.25
resolution: "@types/react-redux@npm:7.1.25"
dependencies:
Expand Down Expand Up @@ -9236,6 +9246,13 @@ __metadata:
languageName: node
linkType: hard

"@types/use-sync-external-store@npm:^0.0.3":
version: 0.0.3
resolution: "@types/use-sync-external-store@npm:0.0.3"
checksum: 161ddb8eec5dbe7279ac971531217e9af6b99f7783213566d2b502e2e2378ea19cf5e5ea4595039d730aa79d3d35c6567d48599f69773a02ffcff1776ec2a44e
languageName: node
linkType: hard

"@types/uuid@npm:9.0.2":
version: 9.0.2
resolution: "@types/uuid@npm:9.0.2"
Expand Down Expand Up @@ -17189,7 +17206,6 @@ __metadata:
"@types/react-dom": "npm:18.2.7"
"@types/react-grid-layout": "npm:1.3.2"
"@types/react-highlight-words": "npm:0.16.4"
"@types/react-redux": "npm:7.1.25"
"@types/react-resizable": "npm:3.0.4"
"@types/react-router-dom": "npm:5.3.3"
"@types/react-table": "npm:7.7.14"
Expand Down Expand Up @@ -17358,7 +17374,7 @@ __metadata:
react-moveable: "npm:0.46.1"
react-popper: "npm:2.3.0"
react-popper-tooltip: "npm:4.4.2"
react-redux: "npm:7.2.6"
react-redux: "npm:8.1.3"
react-refresh: "npm:0.14.0"
react-resizable: "npm:3.0.5"
react-responsive-carousel: "npm:^3.2.23"
Expand Down Expand Up @@ -24984,24 +25000,35 @@ __metadata:
languageName: node
linkType: hard

"react-redux@npm:7.2.6":
version: 7.2.6
resolution: "react-redux@npm:7.2.6"
"react-redux@npm:8.1.3":
version: 8.1.3
resolution: "react-redux@npm:8.1.3"
dependencies:
"@babel/runtime": "npm:^7.15.4"
"@types/react-redux": "npm:^7.1.20"
"@babel/runtime": "npm:^7.12.1"
"@types/hoist-non-react-statics": "npm:^3.3.1"
"@types/use-sync-external-store": "npm:^0.0.3"
hoist-non-react-statics: "npm:^3.3.2"
loose-envify: "npm:^1.4.0"
prop-types: "npm:^15.7.2"
react-is: "npm:^17.0.2"
react-is: "npm:^18.0.0"
use-sync-external-store: "npm:^1.0.0"
peerDependencies:
react: ^16.8.3 || ^17
"@types/react": ^16.8 || ^17.0 || ^18.0
"@types/react-dom": ^16.8 || ^17.0 || ^18.0
react: ^16.8 || ^17.0 || ^18.0
react-dom: ^16.8 || ^17.0 || ^18.0
react-native: ">=0.59"
redux: ^4 || ^5.0.0-beta.0
peerDependenciesMeta:
"@types/react":
optional: true
"@types/react-dom":
optional: true
react-dom:
optional: true
react-native:
optional: true
checksum: 62fed236e8ce022c7fd45c9fd5e26893d9154b5a53bb8aea2989fb5e82e938fe23414329510a3efe6a75c755da446f35bad78db73c5e839f87839a79b091e6c1
redux:
optional: true
checksum: c4c7586cff3abeb784e73598d330f5301116a4e9942fd36895f2bccd8990001709c6c3ea1817edb75ee477470d6c67c9113e05a7f86b2b68a3950c9c29fe20cb
languageName: node
linkType: hard

Expand Down Expand Up @@ -25111,16 +25138,16 @@ __metadata:
linkType: hard

"react-router-dom-v5-compat@npm:^6.10.0":
version: 6.17.0
resolution: "react-router-dom-v5-compat@npm:6.17.0"
version: 6.10.0
resolution: "react-router-dom-v5-compat@npm:6.10.0"
dependencies:
history: "npm:^5.3.0"
react-router: "npm:6.17.0"
react-router: "npm:6.10.0"
peerDependencies:
react: ">=16.8"
react-dom: ">=16.8"
react-router-dom: 4 || 5
checksum: 77062bcdf2f1a5789d146e42d29e24f45074a49e6a1467ea7e5a57d2e50a961ebe1d9a219138bf3feef1e577d9635dd8d15d0e0f5851e574efe44a479441e261
checksum: b86edb22640e25687a843fd46c22b58e147316ee957653e684d9f46a7e71da113a761a2f74c60d42f5d4222e497f825e541a1f4d8872b26edb4b623fb7c87928
languageName: node
linkType: hard

Expand Down Expand Up @@ -25161,14 +25188,14 @@ __metadata:
languageName: node
linkType: hard

"react-router@npm:6.17.0":
version: 6.17.0
resolution: "react-router@npm:6.17.0"
"react-router@npm:6.10.0":
version: 6.10.0
resolution: "react-router@npm:6.10.0"
dependencies:
"@remix-run/router": "npm:1.10.0"
"@remix-run/router": "npm:1.5.0"
peerDependencies:
react: ">=16.8"
checksum: 5c589c67b53cc1a210bd08285392e951a2c3d51a2502806f68d9ce604307944239b0a3b766d8390b484d707ace3068af858e999a1c242662b917ddcd4ab3c453
checksum: 3c78db213d2c67c7ae06125b296889ebf3407963268ef23319312b4d7bf455ecfaa59164be73d6b4e19fb2ef6c2771d7dfe764d5a91cbdbb7c8e84c95aca99cc
languageName: node
linkType: hard

Expand Down

0 comments on commit 13c0268

Please sign in to comment.