Chore: Upgrade react-redux (#77296)

* 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
2024-11-25 10:20:29 -06:00 · 2023-10-31 16:24:34 +00:00 · 2023-10-31 16:24:34 +00:00 · 13c0268d6b
commit 13c0268d6b
parent da1a53ebb1
10 changed files with 117 additions and 67 deletions
--- a/.github/renovate.json5
+++ b/.github/renovate.json5
@ -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/**"],
--- a/package.json
+++ b/package.json
@ -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",
@ -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",
--- a/public/app/core/reducers/appNotification.ts
+++ b/public/app/core/reducers/appNotification.ts
@ -1,4 +1,4 @@
-import { createSlice, PayloadAction } from '@reduxjs/toolkit';
+import { createSelector, createSlice, PayloadAction } from '@reduxjs/toolkit';

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

@ -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

--- a/public/app/core/selectors/navModel.ts
+++ b/public/app/core/selectors/navModel.ts
@ -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';
@ -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;
+    }
+
+    return getNotFoundModel();
  }
-
-  if (fallback) {
-    return fallback;
-  }
-
-  return getNotFoundModel();
-};
+);

 export function getRootSectionForNode(node: NavModelItem): NavModelItem {
  // Don't recurse fully up the folder tree when nested folders is enabled
--- a/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.test.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.test.tsx
@ -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';
@ -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();
--- a/public/app/features/alerting/unified/hooks/useExternalAmSelector.ts
+++ b/public/app/features/alerting/unified/hooks/useExternalAmSelector.ts
@ -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';
@ -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)
    )
  );

--- a/public/app/features/alerting/unified/hooks/useUnifiedAlertingSelector.ts
+++ b/public/app/features/alerting/unified/hooks/useUnifiedAlertingSelector.ts
@ -1,4 +1,6 @@
-import { useSelector } from 'app/types';
+import { createSelector } from 'reselect';
+
+import { StoreState, useSelector } from 'app/types';

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

@ -6,5 +8,11 @@ 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
+  );
 }
--- a/public/app/features/commandPalette/actions/useActions.ts
+++ b/public/app/features/commandPalette/actions/useActions.ts
@ -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);
--- a/public/app/features/datasources/state/selectors.ts
+++ b/public/app/features/datasources/state/selectors.ts
@ -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) => {
@ -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) {
--- a/yarn.lock
+++ b/yarn.lock
@ -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

@ -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"
@ -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:
@ -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"
@ -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"
@ -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"
@ -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

@ -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

@ -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