CONSOLE-4989: bump to react-redux 8
#15854
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@logonoff: This pull request references CONSOLE-4989 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughUpgrades react-redux to v8 and introduces typed console Redux hooks (useConsoleDispatch, useConsoleSelector, useConsoleStore). Replaces react‑redux hook usage across many files, adds SDKDispatch, refactors feature-flag wiring, and converts at least one connected HOC to a hook-based functional component. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (6)
Comment |
openshift-ci
bot
added
component/core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/public/components/masthead/masthead-toolbar.tsx (1)
3-5: Consider completing the hook migration for consistency.The file has partially migrated to console-specific hooks:
useConsoleDispatchis used (line 164), butuseSelectorfrom react-redux is still used (line 172). For consistency with the migration pattern seen in other files likeuseImpersonateRefreshFeatures.tsanduseGroupActions.ts, consider also migrating touseConsoleSelector.Apply this diff:
-import { useSelector } from 'react-redux'; import { useTranslation } from 'react-i18next'; -import { useConsoleDispatch } from '@console/app/src/hooks/redux'; +import { useConsoleDispatch, useConsoleSelector } from '@console/app/src/hooks/redux';const dispatch = useConsoleDispatch(); const [activeNamespace] = useActiveNamespace(); const [activePerspective] = useActivePerspective(); const [requestTokenURL, externalLoginCommand] = useCopyLoginCommands(); const launchCopyLoginCommandModal = useCopyCodeModal( t('public~Login with this command'), externalLoginCommand, ); - const { clusterID, alertCount, canAccessNS, impersonate } = useSelector((state: RootState) => ({ + const { clusterID, alertCount, canAccessNS, impersonate } = useConsoleSelector((state) => ({ clusterID: state.UI.get('clusterID'), alertCount: state.observe.getIn(['alertCount']), canAccessNS: !!state[featureReducerName].get(FLAGS.CAN_GET_NS), impersonate: getImpersonate(state), }));Also applies to: 164-177
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
README.md(1 hunks)frontend/package.json(6 hunks)frontend/packages/console-app/src/actions/hooks/useBindingActions.ts(2 hunks)frontend/packages/console-app/src/actions/hooks/useGroupActions.ts(1 hunks)frontend/packages/console-app/src/actions/providers/user-provider.ts(1 hunks)frontend/packages/console-app/src/hooks/redux.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/README.md(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts(3 hunks)frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts(1 hunks)frontend/packages/console-shared/src/hooks/useDashboardResources.ts(3 hunks)frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx(3 hunks)frontend/public/actions/dashboards.ts(1 hunks)frontend/public/components/masthead/masthead-toolbar.tsx(2 hunks)frontend/public/components/useImpersonateRefreshFeatures.ts(2 hunks)frontend/scripts/check-patternfly-modules.sh(0 hunks)frontend/scripts/check-patternfly-modules.ts(1 hunks)test-frontend.sh(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/scripts/check-patternfly-modules.sh
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test-frontend.shfrontend/packages/console-dynamic-plugin-sdk/README.mdfrontend/scripts/check-patternfly-modules.tsfrontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.tsfrontend/packages/console-app/src/actions/hooks/useBindingActions.tsfrontend/packages/console-shared/src/hooks/useDashboardResources.tsfrontend/packages/console-app/src/hooks/redux.tsfrontend/packages/operator-lifecycle-manager/src/components/operand/index.tsxfrontend/public/components/masthead/masthead-toolbar.tsxfrontend/packages/console-app/src/actions/hooks/useGroupActions.tsfrontend/packages/console-app/src/actions/providers/user-provider.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.tsfrontend/public/actions/dashboards.tsREADME.mdfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.tsfrontend/public/components/useImpersonateRefreshFeatures.tsfrontend/package.json
🧬 Code graph analysis (11)
frontend/packages/console-app/src/actions/hooks/useBindingActions.ts (1)
frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(13-13)
frontend/packages/console-shared/src/hooks/useDashboardResources.ts (1)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(13-13)useConsoleSelector(14-14)
frontend/packages/console-app/src/hooks/redux.ts (1)
frontend/public/redux.ts (1)
RootState(19-27)
frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx (2)
frontend/public/components/namespace.jsx (3)
dispatch(348-348)dispatch(659-659)dispatch(722-722)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(13-13)
frontend/public/components/masthead/masthead-toolbar.tsx (2)
frontend/public/components/namespace.jsx (3)
dispatch(348-348)dispatch(659-659)dispatch(722-722)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(13-13)
frontend/packages/console-app/src/actions/hooks/useGroupActions.ts (3)
frontend/public/module/k8s/types.ts (1)
GroupKind(1006-1008)frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(13-13)useConsoleSelector(14-14)frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts (1)
getImpersonate(18-18)
frontend/packages/console-app/src/actions/providers/user-provider.ts (2)
frontend/public/module/k8s/types.ts (1)
UserKind(1001-1004)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(13-13)
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts (2)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleSelector(14-14)useConsoleDispatch(13-13)frontend/packages/console-dynamic-plugin-sdk/src/api/common-types.ts (1)
K8sModel(33-60)
frontend/public/actions/dashboards.ts (1)
frontend/public/components/namespace.jsx (1)
url(664-664)
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts (2)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(13-13)useConsoleSelector(14-14)frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts (1)
SDKStoreState(26-29)
frontend/public/components/useImpersonateRefreshFeatures.ts (2)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(13-13)useConsoleSelector(14-14)frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts (1)
getImpersonate(18-18)
🔇 Additional comments (20)
frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx (3)
9-9: LGTM! Import migration is correct.The import correctly switches from the generic React-Redux
useDispatchto the console-specificuseConsoleDispatchhook, which provides pre-typed dispatch withThunkDispatch<RootState, undefined, AnyAction>for improved TypeScript type safety.
362-362: LGTM! Dispatch hook migration is correct.The migration from
useDispatch()touseConsoleDispatch()in theProvidedAPIsPagecomponent is implemented correctly. The dispatch usage at line 404 remains unchanged, maintaining existing functionality while benefiting from improved TypeScript typing.
551-551: LGTM! Dispatch hook migration is correct.The migration from
useDispatch()touseConsoleDispatch()in theProvidedAPIPagecomponent is implemented correctly. The dispatch usage at line 557 remains unchanged, maintaining existing functionality while benefiting from improved TypeScript typing.frontend/public/actions/dashboards.ts (1)
136-140: LGTM! Type signatures now correctly match implementations.The updated return types using
ReturnType<typeof stopWatch>correctly capture what these action creators actually return. This improves type safety by aligning the type signatures with the actual implementations that return action objects rather than void.frontend/packages/console-dynamic-plugin-sdk/README.md (1)
219-222: LGTM!The documentation clearly communicates the react-redux v8 upgrade requirement for Console 4.22.X. This is essential information for plugin authors who need to maintain compatibility.
README.md (1)
423-423: LGTM!The command update correctly reflects the new dedupe-deps script introduced in this PR. Using
yarn run dedupe-depsinstead ofnpx yarn-deduplicateprovides better integration with the project's tooling.frontend/public/components/useImpersonateRefreshFeatures.ts (1)
3-16: LGTM!The migration from React-Redux hooks to console-specific hooks (
useConsoleDispatch,useConsoleSelector) is implemented correctly. The hook logic and functionality remain unchanged, while gaining proper type safety through the typed console hooks.frontend/packages/console-app/src/hooks/redux.ts (1)
1-15: LGTM!This is a well-structured Redux hooks utilities module. The typed hooks (
useConsoleDispatch,useConsoleSelector,useConsoleStore) provide proper type safety for the Console app's Redux usage. The TODO comment about react-redux v9'swithTypesmethod is helpful for future maintenance.frontend/packages/console-app/src/actions/providers/user-provider.ts (1)
10-16: LGTM!The migration to
useConsoleDispatchis correct and complete. The dispatch hook is properly typed for thunk actions, and the change maintains functionality while improving type safety.frontend/packages/console-app/src/actions/hooks/useBindingActions.ts (1)
18-44: LGTM!The migration to
useConsoleDispatchis correctly implemented. The dispatch hook is properly used in thestartImpersonatecallback and maintains proper typing for thunk actions.frontend/packages/console-app/src/actions/hooks/useGroupActions.ts (1)
11-28: LGTM!Complete migration to console-specific hooks (
useConsoleDispatchanduseConsoleSelector) is properly implemented. Both hooks are correctly typed and maintain the existing functionality while improving type safety.frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts (1)
3-3: LGTM! Hook migration is correct.The replacement of React-Redux hooks with console-specific hooks aligns with the PR objectives. The type assertion for
ImmutableMap<string, any>is necessary given the untyped Redux state structure.Also applies to: 14-14, 22-24
frontend/packages/console-shared/src/hooks/useDashboardResources.ts (1)
2-3: LGTM! Consistent hook migration.The changes correctly replace React-Redux hooks with console-specific equivalents, maintaining the same functionality while aligning with the new typed Redux hook pattern.
Also applies to: 22-22, 37-40
test-frontend.sh (1)
34-38: LGTM! Valuable preflight check added.The new deduplication check enhances CI reliability by catching duplicate dependency resolutions early in the pipeline, providing clear guidance to developers on remediation.
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts (1)
3-3: LGTM! Hook migration with appropriate type handling.The changes correctly migrate to console-specific hooks while handling the necessary type assertions for Redux state access. The cast to
SDKStoreStateensures proper state typing.Also applies to: 36-36, 49-51
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts (1)
42-44: Partial hook migration completed correctly.The use of
useConsoleDispatchanduseConsoleSelectorwith appropriate type assertions is correct. However, ensure that theuseSelectorimport on line 3 is removed after addressing the inconsistency issue flagged above.Also applies to: 102-102
frontend/scripts/check-patternfly-modules.ts (1)
1-123: LGTM! Comprehensive PatternFly version validation.This new script provides robust validation of PatternFly package resolutions with clear error reporting and actionable guidance. The logic correctly handles zero, single, and multiple resolutions while validating semver compliance.
frontend/package.json (3)
14-14: LGTM! Scripts updated for new tooling.The postinstall and dedupe-deps scripts correctly integrate the new TypeScript-based PatternFly validation and dependency deduplication workflow.
Also applies to: 18-18
237-237: LGTM! New tooling and type definitions added correctly.The additions of yarn lockfile tooling, deduplicate utility, and updated type definitions support the new dependency management workflow introduced in this PR.
Also applies to: 248-249, 308-309, 315-316
140-140: Dependency versions verified as stable with no security vulnerabilities.Both dependency upgrades are confirmed:
@patternfly/react-component-groups6.3.0 is a stable release (published July 23, 2025), transitioning from the 6.2.0-prerelease.10 versionreact-redux8.1.3 is a stable release (published October 1, 2023) with no known security vulnerabilitiesNo security advisories or vulnerabilities detected for either version.
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts
Outdated
Show resolved
Hide resolved
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
a933ae1 to
5953937
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/public/components/api-explorer.tsx (1)
119-121: Future consistency: Consider migrating remaining connected components.While the partial migration is intentional (as noted in the PR description),
APIResourceLink_andAPIResourcePage_still use the legacyconnectHOC pattern. For future consistency, consider adopting the typed hooks throughout this file.frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts (1)
49-51: Includenullin the type cast for accuracy.The selector can return
null(whenreduxIDis falsy), but the type cast on line 51 only includesImmutableMap<string, any>. While the runtime null check on line 57 makes the code safe, the type should accurately reflect thatresourceK8scan benullto maintain type safety guarantees.Apply this diff to fix the type cast:
- const resourceK8s = useSelector<SDKStoreState>((state) => - reduxID ? getReduxIdPayload(state, reduxID.id) : null, - ) as ImmutableMap<string, any>; + const resourceK8s = useSelector<SDKStoreState>((state) => + reduxID ? getReduxIdPayload(state, reduxID.id) : null, + ) as ImmutableMap<string, any> | null;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts(3 hunks)frontend/public/components/api-explorer.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/components/api-explorer.tsxfrontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts
🧬 Code graph analysis (2)
frontend/public/components/api-explorer.tsx (3)
frontend/packages/console-shared/src/hooks/useLocation.ts (1)
useLocation(4-5)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleSelector(14-14)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
K8sResourceKindReference(97-97)
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts (2)
SDKDispatch(33-33)SDKStoreState(28-31)
🔇 Additional comments (4)
frontend/public/components/api-explorer.tsx (2)
10-10: LGTM: Typed Redux hook imported correctly.The import follows the project's typed Redux hook pattern as defined in the redux utilities module.
153-158: Refactor to typed Redux hooks is correct—state initialization is guaranteed.The migration from HOC to hooks is well-executed and follows the typed Redux hook pattern. The
state.k8s.getIn(['RESOURCES', 'models'])path is always initialized as an ImmutableMap by the k8s reducer (even if empty on clusters without CRDs), so the!models.sizecheck on line 442 is safe and will not throw an error.Likely an incorrect or invalid review comment.
frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts (1)
2-3: LGTM!The new
SDKDispatchtype and its supporting type-only imports correctly implement typed dispatch following React-Redux v8 TypeScript patterns. The type signature properly specifiesSDKStoreState,undefinedfor the extra thunk argument, andAnyActionfor the action type.Also applies to: 33-33
frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts (1)
3-3: LGTM!The typed imports and
useDispatch<SDKDispatch>()correctly implement the React-Redux v8 TypeScript patterns, properly typing the dispatch function with the SDK-specific dispatch type.Also applies to: 6-6, 36-36
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
5953937 to
c97d953
Compare
logonoff
changed the title
react-redux 8 react-redux 8 POC
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
81597a8 to
8a1dd14
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
frontend/package.json(1 hunks)frontend/packages/console-app/src/actions/hooks/useBindingActions.ts(2 hunks)frontend/packages/console-app/src/actions/hooks/useGroupActions.ts(1 hunks)frontend/packages/console-app/src/actions/providers/user-provider.ts(1 hunks)frontend/packages/console-app/src/hooks/redux.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/README.md(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts(3 hunks)frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts(1 hunks)frontend/packages/console-shared/src/hooks/useDashboardResources.ts(3 hunks)frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx(3 hunks)frontend/public/actions/dashboards.ts(1 hunks)frontend/public/actions/features.ts(1 hunks)frontend/public/components/api-explorer.tsx(3 hunks)frontend/public/components/masthead/masthead-toolbar.tsx(2 hunks)frontend/public/components/useImpersonateRefreshFeatures.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/package.json
- frontend/public/components/useImpersonateRefreshFeatures.ts
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts
- frontend/packages/console-dynamic-plugin-sdk/README.md
- frontend/packages/console-app/src/hooks/redux.ts
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts
- frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts
- frontend/public/components/masthead/masthead-toolbar.tsx
- frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-app/src/actions/hooks/useGroupActions.tsfrontend/packages/console-app/src/actions/hooks/useBindingActions.tsfrontend/public/actions/dashboards.tsfrontend/packages/operator-lifecycle-manager/src/components/operand/index.tsxfrontend/public/components/api-explorer.tsxfrontend/packages/console-shared/src/hooks/useDashboardResources.tsfrontend/public/actions/features.tsfrontend/packages/console-app/src/actions/providers/user-provider.ts
🧬 Code graph analysis (6)
frontend/packages/console-app/src/actions/hooks/useGroupActions.ts (2)
frontend/public/module/k8s/types.ts (1)
GroupKind(1006-1008)frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(18-18)useConsoleSelector(25-25)
frontend/packages/console-app/src/actions/hooks/useBindingActions.ts (1)
frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(18-18)
frontend/public/actions/dashboards.ts (1)
frontend/public/components/namespace.jsx (1)
url(664-664)
frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx (2)
frontend/public/components/namespace.jsx (3)
dispatch(348-348)dispatch(659-659)dispatch(722-722)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(18-18)
frontend/public/actions/features.ts (1)
frontend/public/actions/flags.ts (1)
setFlag(23-24)
frontend/packages/console-app/src/actions/providers/user-provider.ts (2)
frontend/public/module/k8s/types.ts (1)
UserKind(1001-1004)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(18-18)
🔇 Additional comments (8)
frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx (1)
9-9: LGTM! Clean migration to pre-typed hooks.The migration from
useDispatchtouseConsoleDispatchis correctly implemented. The new hook provides better TypeScript support with pre-typedThunkDispatch<RootState, undefined, AnyAction>while maintaining identical functionality for dispatchinggetResources()actions.Also applies to: 362-362, 551-551
frontend/public/actions/dashboards.ts (1)
136-140: LGTM! Type signatures now correctly reflect the implementation.The change from
voidtoReturnType<typeof stopWatch>accurately represents that these action creators return action objects (viatypesafe-actions), not void. The implementations at lines 124-126 have always returned the result ofstopWatch(), so this corrects a long-standing type mismatch.This aligns well with the react-redux v8 upgrade and typed dispatch hooks (
useConsoleDispatch), where accurate return types enable proper type inference when dispatching actions.frontend/packages/console-shared/src/hooks/useDashboardResources.ts (1)
2-40: LGTM! Clean migration to typed Redux hooks.The migration from
useDispatch/useSelectortouseConsoleDispatch/useConsoleSelectoris correctly implemented. The hook replacements are mechanical, the logic remains unchanged, and type safety is improved through the pre-typed console hooks.frontend/packages/console-app/src/actions/hooks/useBindingActions.ts (1)
18-18: LGTM! Clean migration to typed Redux hooks.The replacement of
useDispatchwithuseConsoleDispatchcorrectly adopts the new pre-typed hook pattern while maintaining identical behavior. The typed dispatch ensures type safety for Redux actions.Also applies to: 40-40
frontend/packages/console-app/src/actions/providers/user-provider.ts (1)
10-10: LGTM! Consistent migration to typed Redux hooks.The migration from
useDispatchtouseConsoleDispatchfollows the same pattern established in other action files. The typed hook provides improved type safety for dispatching UI actions.Also applies to: 16-16
frontend/packages/console-app/src/actions/hooks/useGroupActions.ts (1)
11-11: LGTM! Complete migration to typed Redux hooks.The migration successfully replaces both
useDispatchanduseSelectorwith their typed equivalents (useConsoleDispatchanduseConsoleSelector). The selector usage withgetImpersonatecorrectly leverages the typed hook's automaticRootStateinference, eliminating the need for explicit type annotations.Also applies to: 18-18, 21-21
frontend/public/components/api-explorer.tsx (2)
10-10: LGTM!The import of
useConsoleSelectoraligns with the PR's objective to introduce pre-typed Console-specific Redux hooks, replacing the traditionalconnectHOC pattern.
153-158: Hook-based refactor is correct and well-typed.The migration from
connectHOC to hooks (useLocation+useConsoleSelector) is properly implemented. The selector correctly accessesstate.k8s.getIn(['RESOURCES', 'models']), anduseConsoleSelectoris properly typed asTypedUseSelectorHook<RootState>for full type inference.The type assertion to
ImmutableMap<K8sResourceKindReference, K8sKind>is necessary and safe: sinceK8sStateis defined asImmutableMap<string, any>, the assertion provides proper typing for downstream operations (reduce(),filter()) which both work correctly with ImmutableMap.
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
3 times, most recently
from
ffc69a2 to
2ae5708
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (1)
17-26: Consider consolidating duplicatefeatureFlagControllerlogic.The
featureFlagControllerimplementation here duplicates the logic infrontend/public/actions/features.ts(queueMicrotask wrappingdispatch(setFlag)). If this logic needs to change (e.g., adding a flag-value check to mitigate TOCTOU races as discussed in the previous review), both locations would need updating.Consider extracting this logic into a shared utility function or hook to ensure consistency.
Note: The same TOCTOU concern raised for
features.tsapplies here: rapid calls with different flag values could queue conflicting dispatches without re-checking the current state inside the microtask.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (18)
frontend/packages/console-app/src/actions/hooks/useBindingActions.ts(2 hunks)frontend/packages/console-app/src/actions/hooks/useGroupActions.ts(1 hunks)frontend/packages/console-app/src/actions/providers/user-provider.ts(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx(1 hunks)frontend/packages/console-app/src/components/tour/__tests__/tour-context.spec.ts(5 hunks)frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts(3 hunks)frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts(1 hunks)frontend/packages/console-shared/src/hooks/useDashboardResources.ts(3 hunks)frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx(3 hunks)frontend/public/actions/dashboards.ts(1 hunks)frontend/public/actions/features.ts(1 hunks)frontend/public/components/api-explorer.tsx(3 hunks)frontend/public/components/app.tsx(1 hunks)frontend/public/components/masthead/masthead-toolbar.tsx(2 hunks)frontend/public/components/useImpersonateRefreshFeatures.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/packages/console-app/src/actions/hooks/useBindingActions.ts
- frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts
- frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts
- frontend/public/components/masthead/masthead-toolbar.tsx
- frontend/packages/console-app/src/actions/hooks/useGroupActions.ts
- frontend/public/components/app.tsx
- frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx
- frontend/packages/console-shared/src/hooks/useDashboardResources.ts
- frontend/public/actions/dashboards.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsxfrontend/public/components/useImpersonateRefreshFeatures.tsfrontend/public/actions/features.tsfrontend/public/components/api-explorer.tsxfrontend/packages/console-app/src/components/tour/__tests__/tour-context.spec.tsfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsxfrontend/packages/console-app/src/actions/providers/user-provider.ts
🧬 Code graph analysis (6)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx (1)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (2)
FeatureFlagHandler(46-46)SetFeatureFlag(45-45)
frontend/public/components/useImpersonateRefreshFeatures.ts (2)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(18-18)useConsoleSelector(25-25)frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts (1)
getImpersonate(18-18)
frontend/public/actions/features.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
SetFeatureFlag(45-45)frontend/public/actions/flags.ts (1)
setFlag(23-24)
frontend/public/components/api-explorer.tsx (4)
frontend/public/components/RBAC/role.jsx (2)
location(347-347)location(377-377)frontend/packages/console-shared/src/hooks/useLocation.ts (1)
useLocation(4-5)frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleSelector(25-25)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
K8sResourceKindReference(97-97)
frontend/packages/console-app/src/components/tour/__tests__/tour-context.spec.ts (1)
frontend/packages/console-shared/src/hooks/useUserSettingsCompatibility.ts (1)
useUserSettingsCompatibility(5-30)
frontend/packages/console-app/src/actions/providers/user-provider.ts (1)
frontend/packages/console-app/src/hooks/redux.ts (1)
useConsoleDispatch(18-18)
🔇 Additional comments (7)
frontend/packages/console-app/src/components/tour/__tests__/tour-context.spec.ts (1)
1-177: LGTM! Test refactoring aligns with PR migration pattern.The test file has been successfully refactored to use explicit Jest module mocks and typed mock references, which improves type safety and maintainability. The changes align with the broader PR objective of adopting explicit exports and typed hooks.
Key improvements:
- Explicit module mocks with proper typing (lines 9-28)
beforeEachhook to clear mocks between tests (lines 31-33)- Preserved test behavior and assertions
frontend/public/components/api-explorer.tsx (2)
10-10: LGTM: Typed Redux hook import aligns with PR objectives.The import of
useConsoleSelectorintroduces the pre-typed hook following React-Redux TypeScript best practices, as outlined in the PR objectives.
153-158: LGTM: Clean refactoring from HOC to hooks.The migration from
compose(withRouter, connect(stateToProps))to hooks is well-executed:
useLocation()from react-router correctly provides the Location objectuseConsoleSelector()maintains type safety while accessing Redux state- The component is now self-contained without prop dependencies
This aligns with the PR's goal of modernizing to React-Redux 8 and adopting hook-based patterns.
frontend/packages/console-app/src/actions/providers/user-provider.ts (1)
10-10: LGTM: Clean migration to typed console hooks.The migration from
useDispatchtouseConsoleDispatchaligns with the PR's objective of introducing pre-typed hooks for improved type safety.Also applies to: 16-16
frontend/public/components/useImpersonateRefreshFeatures.ts (1)
3-3: LGTM: Consistent migration to typed console hooks.The migration from
useDispatch/useSelectortouseConsoleDispatch/useConsoleSelectoris consistent with the broader migration pattern and improves type safety.Also applies to: 15-16
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx (1)
1-1: LGTM: Type-only import and named export improve modularity.The shift to a type-only
FCimport and named export is appropriate and aligns with the coordinated changes inFeatureFlagExtensionLoader.tsx.Also applies to: 9-9
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (1)
1-1: LGTM: Hook integration and named export align with migration goals.The addition of
useConsoleDispatch, localfeatureFlagControllerwithuseCallback, and conversion to a named export are consistent with the PR's migration to typed console hooks.Also applies to: 9-10, 12-12
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
2 times, most recently
from
dce7878 to
0cf7e4a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (1)
17-32: Consider optimizing callback dependencies for performance.The implementation correctly uses the new typed hooks and handles the dispatch deferral with
queueMicrotask. However, includingflagsin theuseCallbackdependency array causes the callback to recreate whenever any flag changes, potentially impacting performance.Consider using a ref pattern to access the latest flags without including them in dependencies:
+const flagsRef = useRef(flags); +useEffect(() => { + flagsRef.current = flags; +}, [flags]); + const featureFlagController = useCallback<SetFeatureFlag>( (flag, enabled) => { queueMicrotask(() => { - if (flags.get(flag) === enabled) { + if (flagsRef.current.get(flag) === enabled) { return; } dispatch(setFlag(flag, enabled)); }); }, - [dispatch, flags], + [dispatch], );This would prevent callback recreation on every flag update while still reading the latest flag state.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx(1 hunks)frontend/public/actions/features.ts(1 hunks)frontend/public/components/app.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/public/actions/features.ts
- frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/components/app.tsxfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx
🔇 Additional comments (3)
frontend/public/components/app.tsx (1)
33-33: LGTM! Import updated to match new named export.The change from default to named import correctly reflects the export style change in
FeatureFlagExtensionLoader.tsx.frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (2)
1-10: LGTM! Import changes support the new typed Redux hooks.The additions include the new console-specific hooks (
useConsoleDispatch,useConsoleSelector) mentioned in the PR objectives, along with supporting types and actions. All imports are properly utilized.
12-12: LGTM! Consistent migration to named exports.The export style change aligns with the updated import in
app.tsxand maintains consistency across module boundaries.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx(1 hunks)frontend/public/actions/features.ts(1 hunks)frontend/public/components/app.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/public/components/app.tsx
- frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/actions/features.tsfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx
🧬 Code graph analysis (2)
frontend/public/actions/features.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
SetFeatureFlag(45-45)frontend/public/actions/flags.ts (1)
setFlag(23-24)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (3)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(18-18)useConsoleSelector(25-25)frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (3)
SetFeatureFlag(45-45)FeatureFlagHookProvider(25-31)isFeatureFlagHookProvider(40-41)frontend/public/actions/flags.ts (1)
setFlag(23-24)
🔇 Additional comments (2)
frontend/public/actions/features.ts (1)
140-150: TOCTOU fix looks correct.The state check is now properly performed inside the
queueMicrotask, readingstore.getState().FLAGS.get(flag)at execution time rather than at call time. This correctly addresses the race condition identified in previous reviews.frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (1)
31-57: Component structure looks good.The named export and
FCtyping align with the broader refactoring pattern. The rendering logic for flag providers is correct.
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx
Show resolved
Hide resolved
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
3 times, most recently
from
f20cf2f to
4d40fd0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/public/components/api-explorer.tsx (1)
102-121: Consider migrating remaining connected components.While the
APIResourcesListmigration is excellent,APIResourceLink_still uses the HOCconnectpattern. For consistency with the new typed Redux hooks approach, consider migrating this component to useuseConsoleSelectorin a follow-up.Example migration:
const APIResourceLink: FC<APIResourceLinkOwnProps> = ({ model }) => { const { t } = useTranslation(); const activeNamespace = useConsoleSelector((state) => state.UI.get('activeNamespace')); const to = getAPIResourceLink(activeNamespace, model); return ( <span className="co-resource-item"> <span className="co-resource-icon--fixed-width pf-v6-u-display-none pf-v6-u-display-flex-on-sm"> <ResourceIcon kind={referenceForModel(model)} /> </span> <Link to={to} className="co-resource-item__resource-name"> {model.labelKey ? t(model.labelKey) : model.kind} </Link> </span> ); };Note: This is a nice-to-have improvement for future work, as mentioned in the PR objectives about "consistent future use of the new hooks."
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx(1 hunks)frontend/public/actions/features.ts(1 hunks)frontend/public/components/api-explorer.tsx(3 hunks)frontend/public/components/app.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/public/components/app.tsx
- frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx
- frontend/public/actions/features.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/components/api-explorer.tsxfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx
🔇 Additional comments (5)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (3)
1-10: LGTM - imports align with the typed Redux hooks migration.The migration from generic
react-reduxhooks to the new typeduseConsoleDispatchanduseConsoleSelectorhooks follows the PR's objective of introducing pre-typed hooks per the React-Redux TypeScript quick start recommendation.
12-34: Stale closure fix correctly implemented using the ref pattern.The implementation properly addresses the previously identified TOCTOU issue by using
flagsRefto always access the latest flags inside the microtask. TheuseEffectkeeps the ref synchronized, and theuseCallbackdependency array correctly omits the ref (refs are stable objects).
36-62: LGTM - clean component wiring with the new controller hook.The component properly uses the new
useFeatureFlagControllerhook and passes the setter to each flag resolver. The named export change aligns with the updated import inapp.tsx.frontend/public/components/api-explorer.tsx (2)
10-10: LGTM! Correct import for typed Redux hook.The import of
useConsoleSelectoraligns with the repository-wide migration to Console-specific typed Redux hooks mentioned in the PR objectives.
153-157: Excellent migration from HOC to hooks!The conversion from
connect+withRouterHOC composition to hooks-based data access is implemented correctly:
useLocation()properly replaces the location prop fromwithRouteruseConsoleSelectorprovides typed access to the models state at the correct path- Component maintains the same functionality with improved type safety
This aligns perfectly with the PR objectives for introducing pre-typed Redux hooks.
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
4d40fd0 to
10d2443
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
frontend/package.json(1 hunks)frontend/packages/console-app/src/actions/hooks/useBindingActions.ts(2 hunks)frontend/packages/console-app/src/actions/hooks/useGroupActions.ts(1 hunks)frontend/packages/console-app/src/actions/providers/user-provider.ts(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx(1 hunks)frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx(1 hunks)frontend/packages/console-app/src/hooks/redux.ts(1 hunks)frontend/packages/console-dynamic-plugin-sdk/README.md(1 hunks)frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts(2 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts(3 hunks)frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts(3 hunks)frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts(1 hunks)frontend/packages/console-shared/src/hooks/useDashboardResources.ts(3 hunks)frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx(3 hunks)frontend/public/actions/dashboards.ts(1 hunks)frontend/public/actions/features.ts(1 hunks)frontend/public/components/api-explorer.tsx(3 hunks)frontend/public/components/app.tsx(1 hunks)frontend/public/components/masthead/masthead-toolbar.tsx(2 hunks)frontend/public/components/useImpersonateRefreshFeatures.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- frontend/packages/console-app/src/actions/hooks/useBindingActions.ts
- frontend/packages/console-app/src/hooks/redux.ts
- frontend/packages/console-app/src/actions/hooks/useGroupActions.ts
- frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts
- frontend/public/components/masthead/masthead-toolbar.tsx
- frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts
- frontend/public/components/app.tsx
- frontend/packages/console-app/src/actions/providers/user-provider.ts
- frontend/package.json
- frontend/packages/console-dynamic-plugin-sdk/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsxfrontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.tsfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsxfrontend/public/actions/dashboards.tsfrontend/public/components/api-explorer.tsxfrontend/public/components/useImpersonateRefreshFeatures.tsfrontend/public/actions/features.tsfrontend/packages/console-shared/src/hooks/useDashboardResources.ts
🧬 Code graph analysis (5)
frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts (2)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(18-18)useConsoleSelector(25-25)frontend/public/actions/dashboards.ts (2)
watchPrometheusQuery(87-113)stopWatchPrometheusQuery(124-125)
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx (1)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (2)
FeatureFlagHandler(46-46)SetFeatureFlag(45-45)
frontend/public/components/useImpersonateRefreshFeatures.ts (2)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(18-18)useConsoleSelector(25-25)frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts (1)
getImpersonate(18-18)
frontend/public/actions/features.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
SetFeatureFlag(45-45)frontend/public/actions/flags.ts (1)
setFlag(23-24)
frontend/packages/console-shared/src/hooks/useDashboardResources.ts (1)
frontend/packages/console-app/src/hooks/redux.ts (2)
useConsoleDispatch(18-18)useConsoleSelector(25-25)
🔇 Additional comments (12)
frontend/public/actions/dashboards.ts (1)
136-140: LGTM! Type definitions now correctly reflect the actual return values.The updated type definitions accurately reflect that these action creators return action objects (via
stopWatch), notvoid. This improves type safety for consumers and aligns with the Redux 8 migration goals.frontend/public/components/useImpersonateRefreshFeatures.ts (2)
3-3: LGTM!The migration to project-specific typed hooks (
useConsoleDispatch,useConsoleSelector) follows the React-Redux TypeScript quick start recommendation and aligns with the PR objectives.
14-37: LGTM!The hook implementation correctly:
- Uses the new typed console hooks for dispatch and selector
- Tracks previous impersonation state with a ref to detect actual changes
- Performs deep equality check on
groupsarray with lodash to avoid spurious refreshes- Updates the ref after dispatch to track the last reacted state
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx (2)
12-37: LGTM - TOCTOU issue properly addressed with ref pattern.The implementation correctly uses a ref to hold the latest flags, ensuring the microtask callback reads fresh state rather than a stale closure. The
useEffectkeepsflagsRef.currentsynchronized with the Redux state, and the dispatch is properly deferred to avoid render-time update errors.
39-65: Clean integration of the new controller.The component properly wires the new
useFeatureFlagControllerhook and passes it to each resolver instance via thesetFeatureFlagprop.frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx (1)
1-15: LGTM - Clean refactor to named export.Good use of type-only import and named export, which aligns with the loader's import pattern.
frontend/public/actions/features.ts (1)
140-150: LGTM - TOCTOU race condition properly addressed.The flag value check is now correctly performed inside the microtask, ensuring fresh state is read at dispatch time rather than at call time. This prevents the race condition where rapid calls could queue conflicting updates.
frontend/public/components/api-explorer.tsx (1)
153-157: LGTM - Clean migration from HOC to hooks.The component is properly refactored from
withRouter + connectto useuseLocation()anduseConsoleSelector()directly. The selector logic is preserved, and the change aligns with the PR's goal of adopting console-specific Redux hooks.frontend/packages/console-shared/src/hooks/useDashboardResources.ts (1)
22-40: LGTM: Hook migration is correct.The migration from
useDispatch/useSelectortouseConsoleDispatch/useConsoleSelectoris implemented correctly. The typed hooks provide improved type safety while maintaining the existing logic for Prometheus query and URL watching.frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts (3)
3-3: LGTM: Console-specific hooks imported correctly.The import correctly switches to the pre-typed
useConsoleDispatchanduseConsoleSelectorhooks, aligning with the PR's objective to introduce type-safe Redux hooks across the codebase.
14-14: LGTM: Dispatch hook updated correctly.The dispatch initialization properly uses
useConsoleDispatch(), maintaining the same functionality while benefiting from improved TypeScript typing.
22-24: LGTM: Selector migrated correctly with appropriate type cast.The selector properly uses
useConsoleSelectorwith state destructuring. The explicit cast toImmutableMap<string, any>is appropriate given:
- Subsequent code uses Immutable.js methods (
get,getIn)- Runtime null checks provide safety (line 26)
- Aligns with the broader migration pattern across the codebase
frontend/packages/console-shared/src/hooks/useDashboardResources.ts
Outdated
Show resolved
Hide resolved
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
10d2443 to
6813aeb
Compare
logonoff
changed the title
react-redux 8 POCreact-redux 8
|
/label px-approved |
openshift-ci
bot
added
px-approved
| "react-linkify": "^0.2.2", | ||
| "react-modal": "^3.16.3", | ||
| "react-redux": "7.2.9", | ||
| "react-redux": "8.1.3", |
There was a problem hiding this comment.
Note that react-redux is Console provided shared module, using the default config
{ singleton = true, allowFallback = false }i.e. plugins are built with the assumption that Console provides v7 impl. without plugin provided fallback.
So this is effectively a breaking change in Console provided shared modules.
frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx
Outdated
Show resolved
Hide resolved
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
6813aeb to
1882df5
Compare
React-redux 8.x has stricter TypeScript types for useDispatch that don't accept thunk actions by default. This updates all dispatch usages to use the typed useConsoleDispatch hook which properly includes ThunkDispatch. - Add ConsoleDispatch type with ThunkDispatch support to redux hooks - Update all useDispatch calls to use useConsoleDispatch - Fix StopWatchPrometheusAction and StopWatchURLAction types that incorrectly returned void instead of the action type 🤖 Generated with [Claude Code](https://claude.com/claude-code) CONSOLE-4989: fix SDK dispatch typing and api-explorer type recursion Add SDKDispatch type to redux-types.ts and use inline type parameters with useDispatch<SDKDispatch>() and useSelector<SDKStoreState>() in the SDK hooks. This avoids circular imports from @console/app since the SDK has its own separate TypeScript build. Convert api-explorer.tsx APIResourcesList from compose/connect/withRouter HOC pattern to React hooks (useLocation, useConsoleSelector) to fix "Type instantiation is excessively deep" error caused by complex HOC type inference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
1882df5 to
e5f62be
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts:
- Line 3: Replace direct react-redux usage with the console-specific selector
hook: remove useSelector import from 'react-redux' and import useConsoleSelector
instead, then update any usage of useSelector (e.g., the call at the current
useK8sWatchResources implementation around the reference on line with
useSelector) to call useConsoleSelector with the same selector function; keep
useDispatch unchanged if still required or replace with the console equivalent
if the migration pattern uses a different dispatch hook.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
frontend/package.jsonfrontend/packages/console-app/src/actions/hooks/useBindingActions.tsfrontend/packages/console-app/src/actions/hooks/useGroupActions.tsfrontend/packages/console-app/src/actions/providers/user-provider.tsfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsxfrontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsxfrontend/packages/console-dynamic-plugin-sdk/README.mdfrontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.tsfrontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.tsfrontend/packages/console-shared/src/hooks/useConsoleDispatch.tsfrontend/packages/console-shared/src/hooks/useConsoleSelector.tsfrontend/packages/console-shared/src/hooks/useConsoleStore.tsfrontend/packages/console-shared/src/hooks/useDashboardResources.tsfrontend/packages/operator-lifecycle-manager/src/components/operand/index.tsxfrontend/public/actions/dashboards.tsfrontend/public/actions/features.tsfrontend/public/components/api-explorer.tsxfrontend/public/components/app.tsxfrontend/public/components/masthead/masthead-toolbar.tsxfrontend/public/components/useImpersonateRefreshFeatures.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/public/components/masthead/masthead-toolbar.tsx
- frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx
- frontend/packages/console-app/src/actions/hooks/useGroupActions.ts
- frontend/public/components/app.tsx
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResource.ts
- frontend/packages/console-app/src/components/flags/FeatureFlagExtensionLoader.tsx
- frontend/package.json
- frontend/packages/console-app/src/components/flags/FeatureFlagExtensionHookResolver.tsx
- frontend/public/components/api-explorer.tsx
- frontend/packages/console-shared/src/components/dashboard/utilization-card/prometheus-hook.ts
- frontend/packages/console-app/src/actions/providers/user-provider.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/console-dynamic-plugin-sdk/README.mdfrontend/packages/console-shared/src/hooks/useConsoleDispatch.tsfrontend/public/actions/dashboards.tsfrontend/public/components/useImpersonateRefreshFeatures.tsfrontend/packages/console-shared/src/hooks/useConsoleSelector.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.tsfrontend/public/actions/features.tsfrontend/packages/console-shared/src/hooks/useConsoleStore.tsfrontend/packages/console-app/src/actions/hooks/useBindingActions.tsfrontend/packages/console-shared/src/hooks/useDashboardResources.ts
🧬 Code graph analysis (6)
frontend/packages/console-shared/src/hooks/useConsoleDispatch.ts (1)
frontend/public/redux.ts (1)
RootState(19-27)
frontend/public/components/useImpersonateRefreshFeatures.ts (3)
frontend/packages/console-shared/src/hooks/useConsoleDispatch.ts (1)
useConsoleDispatch(14-14)frontend/packages/console-shared/src/hooks/useConsoleSelector.ts (1)
useConsoleSelector(12-12)frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts (1)
getImpersonate(18-18)
frontend/packages/console-shared/src/hooks/useConsoleSelector.ts (1)
frontend/public/redux.ts (1)
RootState(19-27)
frontend/public/actions/features.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
SetFeatureFlag(45-45)frontend/public/actions/flags.ts (1)
setFlag(23-24)
frontend/packages/console-app/src/actions/hooks/useBindingActions.ts (1)
frontend/packages/console-shared/src/hooks/useConsoleDispatch.ts (1)
useConsoleDispatch(14-14)
frontend/packages/console-shared/src/hooks/useDashboardResources.ts (2)
frontend/packages/console-shared/src/hooks/useConsoleDispatch.ts (1)
useConsoleDispatch(14-14)frontend/packages/console-shared/src/hooks/useConsoleSelector.ts (1)
useConsoleSelector(12-12)
🔇 Additional comments (9)
frontend/public/actions/dashboards.ts (1)
136-140: LGTM! Type signatures now accurately reflect returned values.The updated return types correctly capture that these action creators return action objects rather than void, improving type safety and accuracy. This aligns well with the PR's goal of better TypeScript integration.
frontend/packages/console-dynamic-plugin-sdk/README.md (1)
219-222: LGTM!The documentation clearly communicates the react-redux v8 upgrade requirement for plugin compatibility.
frontend/packages/console-app/src/actions/hooks/useBindingActions.ts (1)
16-16: LGTM!The migration from
useDispatchtouseConsoleDispatchcorrectly follows the console-wide hook migration pattern established in this PR.Also applies to: 40-40
frontend/public/actions/features.ts (1)
140-150: LGTM!The implementation correctly addresses the TOCTOU concerns from previous reviews by checking the flag value inside the microtask before dispatching, preventing both race conditions and redundant state updates.
frontend/packages/console-shared/src/hooks/useConsoleStore.ts (1)
1-12: LGTM!The hook follows the same pattern as
useConsoleDispatchanduseConsoleSelector, providing strongly-typed console store access. The TODO note appropriately flags the future upgrade path to react-redux v9's built-in typing approach.frontend/packages/console-shared/src/hooks/useConsoleDispatch.ts (1)
1-14: LGTM!This typed wrapper correctly provides thunk-enabled dispatch for the console's Redux store. The implementation follows React-Redux's recommended pattern for TypeScript pre-v9.
frontend/packages/console-shared/src/hooks/useConsoleSelector.ts (1)
1-12: LGTM!This typed wrapper correctly provides type-safe state selection for the console's Redux store. The implementation aligns with React-Redux's recommended TypeScript patterns.
frontend/packages/console-shared/src/hooks/useDashboardResources.ts (1)
10-11: LGTM!The migration from React-Redux hooks to console-specific typed hooks (
useConsoleDispatch,useConsoleSelector) is implemented correctly. The logic and behavior remain unchanged, and the effect dependencies are properly maintained.Also applies to: 23-23, 38-41
frontend/public/components/useImpersonateRefreshFeatures.ts (1)
3-4: LGTM!The migration to console-specific typed hooks is correctly implemented. The impersonation change detection logic remains intact, and the effect dependencies are properly maintained including the
dispatchfunction.Also applies to: 16-17, 37-37
| import { useRef, useMemo, useEffect } from 'react'; | ||
| import { Map as ImmutableMap, Iterable as ImmutableIterable } from 'immutable'; | ||
| import { useSelector, useDispatch } from 'react-redux'; | ||
| import { useDispatch, useSelector } from 'react-redux'; |
There was a problem hiding this comment.
Inconsistent with migration pattern: still using react-redux's useSelector.
Despite the past review comment being marked as addressed, lines 3 and 141 still import and use useSelector from react-redux directly instead of useConsoleSelector. This is inconsistent with the migration pattern used throughout this PR (e.g., useBindingActions.ts, masthead-toolbar.tsx, prometheus-hook.ts, useDashboardResources.ts).
🔎 Apply the migration pattern
Remove the react-redux import and use console-specific hooks:
-import { useDispatch, useSelector } from 'react-redux';
+import { useDispatch } from 'react-redux';
+import { useConsoleSelector } from '@console/shared/src/hooks/useConsoleSelector';Then update line 141:
- const resourceK8s = useSelector(resourceK8sSelector);
+ const resourceK8s = useConsoleSelector(resourceK8sSelector);Also applies to: 141-141
🤖 Prompt for AI Agents
In
@frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sWatchResources.ts
at line 3, Replace direct react-redux usage with the console-specific selector
hook: remove useSelector import from 'react-redux' and import useConsoleSelector
instead, then update any usage of useSelector (e.g., the call at the current
useK8sWatchResources implementation around the reference on line with
useSelector) to call useConsoleSelector with the same selector function; keep
useDispatch unchanged if still required or replace with the console equivalent
if the migration pattern uses a different dispatch hook.
e5f62be to
f130f9b
Compare
FeatureFlagHookProvider handlers are React hooks that must run during render (e.g., usePerspectivesAvailable calls usePerspectives). In react-redux 8.x, dispatching Redux actions during render causes "Cannot update a component while rendering a different component" errors due to useSyncExternalStore. Solution: Queue flag updates during render in a ref, then dispatch them in useLayoutEffect after render completes. This allows handlers to use hooks during render while deferring the actual Redux dispatch. For non-hook handlers in features.ts (processed via subscribeToExtensions outside React), no deferral is needed since they run outside the render cycle. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
logonoff
force-pushed
the
CONSOLE-4989-redux
branch
from
f130f9b to
f5c40ad
Compare
|
@logonoff: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Depends on #15835
Introduces pre-typed hooks
useConsoleDispatch,useConsoleSelector, anduseConsoleStoreper the recommendation in https://react-redux.js.org/tutorials/typescript-quick-start.AI made the minimum changes here to decrease the diff but we should consistently use our own hooks some day