diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/app/components/safety-first.tsx b/frontend/packages/console-dynamic-plugin-sdk/src/app/components/safety-first.tsx deleted file mode 100644 index 4bdebd6fa3b..00000000000 --- a/frontend/packages/console-dynamic-plugin-sdk/src/app/components/safety-first.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import type { SetStateAction, Dispatch } from 'react'; -import { useRef, useEffect, useState, useCallback } from 'react'; - -// TODO(react18): Remove this hook - CONSOLE-5039 -/** - * @deprecated Directly use {@link useState} (and be happy) - * - * This hook originally was created to suppress this React warning: - * - * > Warning: Can't perform a React state update on an unmounted component. This - * > is a no-op, but it indicates a memory leak in your application. To fix, cancel - * > all subscriptions and asynchronous tasks in a useEffect cleanup function. - * - * This warning was meant to warn developers about potential memory leaks when - * subscribing to an external data source, and then forgetting to unsubscribe when - * the component unmounts. - * - * In React 18, this warning was removed as it was determined to be misleading, - * because in many cases, there is no memory leak, so the warning served to - * confuse developers. - * - * @see https://github.com/reactwg/react-18/discussions/82 - * - * --- - * - * Previously, this hook originally had this description: - * > Hook that ensures a safe asynchronous setting of the React state in case a - * > given component could be unmounted. - * - * @see https://github.com/facebook/react/issues/14113 - * - * @param initialState initial state value - * - * @returns An array with a pair of state value and its set function. - */ -export const useSafetyFirst = ( - initialState: S | (() => S), -): [S, Dispatch>] => { - const mounted = useRef(true); - useEffect(() => { - return () => { - mounted.current = false; - }; - }, []); - - const [value, setValue] = useState(initialState); - const setValueSafe = useCallback((newValue: S) => { - if (mounted.current) { - setValue(newValue); - } - }, []); - - return [value, setValueSafe]; -}; diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/app/components/utils/rbac.tsx b/frontend/packages/console-dynamic-plugin-sdk/src/app/components/utils/rbac.tsx index 35abbd20fb7..86c8aadd1b3 100644 --- a/frontend/packages/console-dynamic-plugin-sdk/src/app/components/utils/rbac.tsx +++ b/frontend/packages/console-dynamic-plugin-sdk/src/app/components/utils/rbac.tsx @@ -1,4 +1,4 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import * as _ from 'lodash'; import { K8sVerb } from '../../../api/common-types'; import { @@ -10,7 +10,6 @@ import { k8sCreate } from '../../../utils/k8s/k8s-resource'; import { getImpersonate } from '../../core/reducers/coreSelectors'; import { ImpersonateKind } from '../../redux-types'; import storeHandler from '../../storeHandler'; -import { useSafetyFirst } from '../safety-first'; /** * It provides impersonation key based on data from the redux store. @@ -109,8 +108,8 @@ export const useAccessReview = ( impersonate?: ImpersonateKind, noCheckForEmptyGroupAndResource?: boolean, ): [boolean, boolean] => { - const [loading, setLoading] = useSafetyFirst(true); - const [isAllowed, setAllowed] = useSafetyFirst(false); + const [loading, setLoading] = useState(true); + const [isAllowed, setAllowed] = useState(false); // Destructure the attributes to pass them as dependencies to `useEffect`, // which doesn't do deep comparison of object dependencies. const { diff --git a/frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx b/frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx index 065502b4ca2..d6887d2f3e8 100644 --- a/frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx +++ b/frontend/packages/console-shared/src/components/actions/menu/ActionMenu.tsx @@ -3,7 +3,6 @@ import { useState, useRef, useCallback, useEffect } from 'react'; import { Menu, MenuContent, MenuList, Popper } from '@patternfly/react-core'; import * as _ from 'lodash'; import { Action, MenuOption } from '@console/dynamic-plugin-sdk'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { checkAccess } from '@console/internal/components/utils/rbac'; import { ActionMenuVariant } from '../types'; import ActionMenuContent from './ActionMenuContent'; @@ -29,7 +28,7 @@ const ActionMenu: FC = ({ appendTo, }) => { const isKebabVariant = variant === ActionMenuVariant.KEBAB; - const [isVisible, setVisible] = useSafetyFirst(isKebabVariant); + const [isVisible, setVisible] = useState(isKebabVariant); const [isOpen, setIsOpen] = useState(false); const menuRef = useRef(null); const toggleRef = useRef(null); diff --git a/frontend/packages/console-shared/src/hooks/usePodsWatcher.ts b/frontend/packages/console-shared/src/hooks/usePodsWatcher.ts index 1178139b446..3ec293f7000 100644 --- a/frontend/packages/console-shared/src/hooks/usePodsWatcher.ts +++ b/frontend/packages/console-shared/src/hooks/usePodsWatcher.ts @@ -1,5 +1,4 @@ -import { useMemo, useCallback, useEffect } from 'react'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; +import { useMemo, useCallback, useEffect, useState } from 'react'; import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook'; import { K8sResourceKind } from '@console/internal/module/k8s'; import { PodRCData } from '../types'; @@ -17,9 +16,9 @@ export const usePodsWatcher = ( kind?: string, namespace?: string, ): { loaded: boolean; loadError: string; podData: PodRCData } => { - const [loaded, setLoaded] = useSafetyFirst(false); - const [loadError, setLoadError] = useSafetyFirst(''); - const [podData, setPodData] = useSafetyFirst(undefined); + const [loaded, setLoaded] = useState(false); + const [loadError, setLoadError] = useState(''); + const [podData, setPodData] = useState(undefined); const watchKind = kind || resource?.kind; const watchNS = namespace || resource?.metadata.namespace; const watchedResources = useMemo( diff --git a/frontend/packages/console-shared/src/utils/pod-ring-utils.ts b/frontend/packages/console-shared/src/utils/pod-ring-utils.ts index a188755db34..181e9fa1014 100644 --- a/frontend/packages/console-shared/src/utils/pod-ring-utils.ts +++ b/frontend/packages/console-shared/src/utils/pod-ring-utils.ts @@ -1,11 +1,10 @@ import type { ReactElement } from 'react'; -import { createElement, useMemo, useEffect } from 'react'; +import { createElement, useMemo, useEffect, useState } from 'react'; import { ChartLabel } from '@patternfly/react-charts/victory'; import { css } from '@patternfly/react-styles'; import i18next, { TFunction } from 'i18next'; import * as _ from 'lodash'; import type { ImpersonateKind } from '@console/dynamic-plugin-sdk'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { DaemonSetModel, PodModel, JobModel, CronJobModel } from '@console/internal/models'; import { K8sResourceKind, @@ -241,7 +240,7 @@ export const usePodScalingAccessStatus = ( const isKnativeRevision = obj.kind === 'Revision'; const isPod = obj.kind === 'Pod'; const isScalingAllowed = !isKnativeRevision && !isPod && enableScaling; - const [editable, setEditable] = useSafetyFirst(false); + const [editable, setEditable] = useState(false); useEffect(() => { if (isScalingAllowed) { diff --git a/frontend/packages/knative-plugin/src/utils/create-channel-utils.ts b/frontend/packages/knative-plugin/src/utils/create-channel-utils.ts index a02b9ecc8f6..d760f88fb26 100644 --- a/frontend/packages/knative-plugin/src/utils/create-channel-utils.ts +++ b/frontend/packages/knative-plugin/src/utils/create-channel-utils.ts @@ -1,11 +1,10 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { safeLoad } from 'js-yaml'; import * as _ from 'lodash'; import { getCommonAnnotations, getAppLabels, } from '@console/dev-console/src/utils/resource-label-utils'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { checkAccess } from '@console/internal/components/utils'; import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook'; import { ConfigMapModel } from '@console/internal/models'; @@ -40,7 +39,7 @@ export const getChannelKind = (ref: string): string => { }; export const useChannelList = (namespace: string): ChannelListProps => { - const [accessData, setAccessData] = useSafetyFirst({ loaded: false, channelList: [] }); + const [accessData, setAccessData] = useState({ loaded: false, channelList: [] }); const { channels, loaded: channelsLoaded } = useChannelResourcesList(); useEffect(() => { const accessList = []; diff --git a/frontend/packages/knative-plugin/src/utils/fetch-dynamic-eventsources-utils.ts b/frontend/packages/knative-plugin/src/utils/fetch-dynamic-eventsources-utils.ts index b2a85f4fc21..420e6245a40 100644 --- a/frontend/packages/knative-plugin/src/utils/fetch-dynamic-eventsources-utils.ts +++ b/frontend/packages/knative-plugin/src/utils/fetch-dynamic-eventsources-utils.ts @@ -1,7 +1,6 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { chart_color_red_orange_300 as knativeEventingColor } from '@patternfly/react-tokens/dist/js/chart_color_red_orange_300'; import * as _ from 'lodash'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { coFetch } from '@console/internal/co-fetch'; import { K8sKind, @@ -85,7 +84,7 @@ export const fetchEventSourcesCrd = async () => { }; export const useEventSourceModels = (): EventSourcetData => { - const [modelsData, setModelsData] = useSafetyFirst({ loaded: false, eventSourceModels: [] }); + const [modelsData, setModelsData] = useState({ loaded: false, eventSourceModels: [] }); useEffect(() => { if (eventSourceData.eventSourceModels.length === 0) { fetchEventSourcesCrd() @@ -201,7 +200,7 @@ export const fetchChannelsCrd = async () => { }; export const useChannelModels = () => { - const [modelsData, setModelsData] = useSafetyFirst({ loaded: false, eventSourceChannels: [] }); + const [modelsData, setModelsData] = useState({ loaded: false, eventSourceChannels: [] }); useEffect(() => { if (eventSourceData.eventSourceChannels.length === 0) { fetchChannelsCrd() @@ -246,7 +245,7 @@ export const getDynamicEventingChannelWatchers = (namespace: string) => { }, {}); }; export const useChannelResourcesList = (): EventChannelData => { - const [modelRefs, setModelRefs] = useSafetyFirst({ + const [modelRefs, setModelRefs] = useState({ channels: [], loaded: false, }); diff --git a/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellNamespace.ts b/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellNamespace.ts index 803eec6e35e..01743afd292 100644 --- a/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellNamespace.ts +++ b/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellNamespace.ts @@ -1,10 +1,9 @@ -import { useEffect } from 'react'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; +import { useEffect, useState } from 'react'; import { getTerminalInstalledNamespace } from './cloud-shell-utils'; const useCloudShellNamespace = (): [string, string] => { - const [terminalNamespace, setTerminalNamespace] = useSafetyFirst(undefined); - const [fetchError, setFetchError] = useSafetyFirst(undefined); + const [terminalNamespace, setTerminalNamespace] = useState(undefined); + const [fetchError, setFetchError] = useState(undefined); useEffect(() => { const fetchNamespace = async () => { try { diff --git a/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellWorkspace.ts b/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellWorkspace.ts index de701e0d76d..7727d514d09 100644 --- a/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellWorkspace.ts +++ b/frontend/packages/webterminal-plugin/src/components/cloud-shell/useCloudShellWorkspace.ts @@ -1,6 +1,5 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { WatchK8sResource, WatchK8sResult } from '@console/dynamic-plugin-sdk'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { useAccessReview2 } from '@console/internal/components/utils'; import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook'; import { ProjectModel } from '@console/internal/models'; @@ -30,9 +29,9 @@ const useCloudShellWorkspace = ( workspaceModel: K8sKind, defaultNamespace: string = null, ): WatchK8sResult => { - const [namespace, setNamespace] = useSafetyFirst(defaultNamespace); - const [searching, setSearching] = useSafetyFirst(false); - const [noNamespaceFound, setNoNamespaceFound] = useSafetyFirst(false); + const [namespace, setNamespace] = useState(defaultNamespace); + const [searching, setSearching] = useState(false); + const [noNamespaceFound, setNoNamespaceFound] = useState(false); // sync defaultNamespace to namespace useEffect(() => { diff --git a/frontend/public/components/__tests__/safety-first.spec.tsx b/frontend/public/components/__tests__/safety-first.spec.tsx deleted file mode 100644 index 9ecb8ef4d90..00000000000 --- a/frontend/public/components/__tests__/safety-first.spec.tsx +++ /dev/null @@ -1,122 +0,0 @@ -import type { FC } from 'react'; -import { screen, fireEvent, waitFor } from '@testing-library/react'; -import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils'; - -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; - -type Props = { - loader: () => Promise; -}; - -const warning = 'perform a React state update on an unmounted component.'; - -describe('useSafetyFirst hook', () => { - const Safe: FC = (props) => { - const [inFlight, setInFlight] = useSafetyFirst(true); - - const onClick = () => props.loader().then(() => setInFlight(false)); - - return ; - }; - - let consoleErrorSpy: ReturnType; - - beforeEach(() => { - consoleErrorSpy = jest.spyOn(global.console, 'error').mockImplementation(() => {}); - }); - - afterEach(() => { - jest.restoreAllMocks(); - }); - - it('does not attempt to set React state if unmounted (using hook)', async () => { - const loader = () => - new Promise((resolve) => { - // Verify loading state is displayed - expect(screen.getByText('Loading...')).toBeInTheDocument(); - resolve(); - }); - - const { rerender } = renderWithProviders(); - - fireEvent.click(screen.getByRole('button', { name: /loading/i })); - - // Unmount the component by rerendering with different content - rerender(
); - - // Wait and verify no warning was logged - await waitFor(() => { - expect( - consoleErrorSpy.mock.calls - .map((call) => call[0] as string) - .some((text) => text.includes(warning)), - ).toBe(false); - }); - }); - - it('will set React state if component remains mounted (using hook)', async () => { - const loader = () => - new Promise((resolve) => { - // Verify loading state is displayed - expect(screen.getByText('Loading...')).toBeVisible(); - resolve(); - }); - - renderWithProviders(); - - fireEvent.click(screen.getByRole('button', { name: /loading/i })); - await waitFor(() => { - expect(screen.getByText('Loaded')).toBeVisible(); - }); - - // Verify no warning was logged - expect( - consoleErrorSpy.mock.calls - .map((call) => call[0] as string) - .some((text) => text.includes(warning)), - ).toBe(false); - }); - - it('displays initial loading state correctly', () => { - const mockLoader = jest.fn(() => Promise.resolve()); - - renderWithProviders(); - - // User should see the initial loading state - expect(screen.getByText('Loading...')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /loading/i })).toBeInTheDocument(); - }); - - it('handles multiple rapid clicks gracefully', async () => { - let resolveCount = 0; - const loader = () => - new Promise((resolve) => { - resolveCount++; - setTimeout(resolve, 50); - }); - - renderWithProviders(); - - const button = screen.getByRole('button', { name: /loading/i }); - - // Click multiple times rapidly - fireEvent.click(button); - fireEvent.click(button); - fireEvent.click(button); - - // Wait for state updates - await waitFor(() => { - expect(screen.getByText('Loaded')).toBeInTheDocument(); - }); - - // Verify the loader was called for each click - expect(resolveCount).toBe(3); - - // Verify no warnings occurred - expect( - consoleErrorSpy.mock.calls - .map((call) => call[0] as string) - .some((text) => text.includes(warning)), - ).toBe(false); - }); -}); diff --git a/frontend/public/components/utils/actions-menu.tsx b/frontend/public/components/utils/actions-menu.tsx index bd023cbd663..c6674230a59 100644 --- a/frontend/public/components/utils/actions-menu.tsx +++ b/frontend/public/components/utils/actions-menu.tsx @@ -1,6 +1,5 @@ import type { FC } from 'react'; import { ImpersonateKind, impersonateStateToProps } from '@console/dynamic-plugin-sdk'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { Button, Dropdown, MenuToggle, MenuToggleElement } from '@patternfly/react-core'; import { some } from 'lodash'; import { useTranslation } from 'react-i18next'; @@ -81,7 +80,7 @@ export const ActionsMenu = connect(impersonateStateToProps)( impersonate, title = undefined, }: ActionsMenuProps & { impersonate?: ImpersonateKind }) => { - const [isVisible, setVisible] = useSafetyFirst(false); + const [isVisible, setVisible] = useState(false); // Check if any actions are visible when actions have access reviews. useEffect(() => { diff --git a/frontend/public/components/utils/managed-by.tsx b/frontend/public/components/utils/managed-by.tsx index c359e713771..a1117020184 100644 --- a/frontend/public/components/utils/managed-by.tsx +++ b/frontend/public/components/utils/managed-by.tsx @@ -1,5 +1,5 @@ import type { FC } from 'react'; -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { css } from '@patternfly/react-styles'; import { Link } from 'react-router-dom-v5-compat'; import { useTranslation } from 'react-i18next'; @@ -13,7 +13,6 @@ import { modelFor, k8sList, } from '../../module/k8s'; -import { useSafetyFirst } from '@console/dynamic-plugin-sdk/src/app/components/safety-first'; import { findOwner, matchOwnerAndCSV } from '../../module/k8s/managed-by'; import { ClusterServiceVersionModel } from '@console/operator-lifecycle-manager/src/models'; import { ClusterServiceVersionKind } from '@console/operator-lifecycle-manager'; @@ -52,7 +51,7 @@ export const ManagedByOperatorResourceLink: FC = ({ export const ManagedByOperatorLink: FC = ({ obj, className }) => { const { t } = useTranslation(); - const [data, setData] = useSafetyFirst(undefined); + const [data, setData] = useState(undefined); const namespace = obj.metadata.namespace; useEffect(() => { if (!namespace) {