CONSOLE-4778: Bump i18next and react-i18next to the reasonably recent version #15875
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@krishagarwal278: This pull request references CONSOLE-4778 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. |
openshift-merge-robot
added
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: krishagarwal278 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/label docs-approved |
WalkthroughBumps React/i18n dependency versions, widens translation-function parameter types and introduces a new TranslationFunction alias, removes some top-level React imports, makes translateExtension return its input, wraps many translated strings in JSX fragments or coerces to string, simplifies test mocks, and moves/removes several locale keys. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes ✨ Finishing touches🧪 Generate unit tests (beta)
📜 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 ignored due to path filters (1)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
openshift-ci
bot
added
component/metal3
… version modified: dynamic-demo-plugin/package.json modified: dynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsx modified: dynamic-demo-plugin/yarn.lock modified: frontend/package.json modified: frontend/packages/console-app/src/components/nodes/NodesPage.tsx modified: frontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.ts modified: frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts modified: frontend/packages/console-plugin-sdk/src/utils/useTranslationExt.ts modified: frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts modified: frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx modified: frontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.ts modified: frontend/packages/dev-console/src/components/import/import-submit-utils.ts modified: frontend/packages/metal3-plugin/src/validations/validations.tsx modified: frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx modified: frontend/packages/operator-lifecycle-manager/src/status/csv-status.ts modified: frontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsx modified: frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx modified: frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx modified: frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts modified: frontend/packages/vsphere-plugin/src/components/persist.ts modified: frontend/packages/vsphere-plugin/src/components/utils.ts modified: frontend/public/components/about-modal.tsx modified: frontend/public/components/cron-job.tsx modified: frontend/public/components/custom-resource-definition.tsx modified: frontend/public/components/error.tsx modified: frontend/public/components/modals/column-management-modal.tsx modified: frontend/public/components/modals/configure-count-modal.tsx modified: frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx modified: frontend/public/components/pod.tsx modified: frontend/public/module/k8s/get-resources.ts modified: frontend/yarn.lock
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
73-77: Missing translation function call on line 76.Line 76 returns a raw translation key string without calling the
t()function, unlike all other branches that properly callt().🔎 Proposed fix
const syncErr = getPrometheusMetricValue(prometheusResult, 'SyncError'); if (toInteger(syncErr?.[1])) { // TODO: Add timestamp to the message - return { state: HealthState.WARNING, message: 'vsphere-plugin~Synchronization failed' }; + return { state: HealthState.WARNING, message: t('vsphere-plugin~Synchronization failed') }; }
🧹 Nitpick comments (1)
frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx (1)
21-21: Consider the implications of using a custom TranslationFunction type.The PR introduces a custom
TranslationFunctiontype ((key: string, options?: Record<string, unknown>) => string) instead of using react-i18next'sTFunction. While this simplifies the type signature, it may lose type safety features thatTFunctionprovides, such as namespace validation or translation key checking. This is acceptable if it's necessary for compatibility with the new library versions, but ensure this trade-off is intentional.Based on the library upgrade context in this PR.
Also applies to: 50-50
📜 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 (2)
dynamic-demo-plugin/yarn.lockis excluded by!**/yarn.lock,!**/*.lockfrontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
dynamic-demo-plugin/package.jsondynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsxfrontend/package.jsonfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.tsfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/packages/operator-lifecycle-manager/src/status/csv-status.tsfrontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsxfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/vsphere-plugin/src/components/utils.tsfrontend/public/components/about-modal.tsxfrontend/public/components/cron-job.tsxfrontend/public/components/custom-resource-definition.tsxfrontend/public/components/error.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/public/components/modals/configure-count-modal.tsxfrontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsxfrontend/public/components/pod.tsxfrontend/public/module/k8s/get-resources.ts
💤 Files with no reviewable changes (1)
- dynamic-demo-plugin/src/components/ProjectOverview/Inventory.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/package.jsonfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/public/components/pod.tsxfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/public/components/modals/configure-count-modal.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/vsphere-plugin/src/components/utils.tsfrontend/public/components/error.tsxfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxdynamic-demo-plugin/package.jsonfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/operator-lifecycle-manager/src/status/csv-status.tsfrontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.tsfrontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsxfrontend/public/module/k8s/get-resources.tsfrontend/public/components/cron-job.tsxfrontend/public/components/about-modal.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsxfrontend/public/components/custom-resource-definition.tsx
🧬 Code graph analysis (16)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (2)
Extension(12-12)Extension(29-33)frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/modals/configure-count-modal.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/modals/column-management-modal.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/console-app/src/components/nodes/NodesPage.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/utils.ts (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/public/components/error.tsx (3)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/public/components/namespace.jsx (1)
error(857-857)frontend/public/components/events.jsx (1)
error(386-386)
frontend/packages/vsphere-plugin/src/components/persist.ts (2)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/cron-job.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/about-modal.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
🔇 Additional comments (24)
frontend/packages/console-app/src/components/nodes/NodesPage.tsx (1)
272-277: LGTM! Translation wrapped in fragment for type consistency.The change correctly wraps the translation result in a React fragment, ensuring consistent React node output. This aligns with the i18next/react-i18next version update and maintains the same functional behavior while providing better type safety.
frontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsx (1)
2-2: LGTM! Clean migration to import TFunction directly from i18next.Importing the TFunction type directly from i18next rather than the react-i18next re-export is correct and aligns with the PR's objective to update these dependencies.
frontend/public/components/modals/configure-count-modal.tsx (1)
73-76: LGTM! Type-safe string coercion for updated i18next.The explicit
String()coercion ensuresresourceKindsremains a string type, aligning with theRecord<string, string>type expectation formessageVariables. This is a sound defensive practice given the updated react-i18next version (15.1.4), which may return broader types from the translation function.frontend/public/module/k8s/get-resources.ts (2)
3-3: LGTM: Import simplified to align with updated i18next usage.The removal of the
TFunctiontype import is consistent with the PR's objective to update i18next and simplify type dependencies.
106-107: No type compatibility issues after removing the TFunction annotation.The
translateExtensionfunction explicitly acceptsTFunction | ((key: string) => string)as its second parameter (line 32 of extension-i18n.ts). The inferred type oftcbas(value: string) => stringmatches this union type and is fully compatible. The removal of the explicit annotation is type-safe.Likely an incorrect or invalid review comment.
frontend/packages/metal3-plugin/src/validations/validations.tsx (1)
1-1: This import change is consistent with the established codebase pattern and poses no type compatibility issues.The import from
i18nextis used across 20+ files throughout the codebase, andTFunction<'metal3-plugin'>is working correctly in this file. Type compatibility with i18next v23.16.8 is confirmed.frontend/packages/operator-lifecycle-manager/src/status/csv-status.ts (1)
44-59: No actionable issues found.The code correctly uses
ClusterServiceVersionStatusenum values, and the single consumer (getOperatorStatus) is properly designed to handle these enum-based status values with appropriate switch logic. There is no breaking change—the consumer is not expecting translated strings in the status field and is already correctly mapping enum values toHealthStatefor downstream processing.Likely an incorrect or invalid review comment.
frontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.ts (1)
35-36: LGTM! Test mock updated to align with production code type change.The indirection through
mockFnproperly accommodates the widenedtparameter type inimport-submit-utils.ts(line 773), where it changed fromTFunctiontoTFunction | ((key: string) => string).frontend/public/components/custom-resource-definition.tsx (1)
281-284: LGTM! Fragment wrapper aligns with PR-wide pattern.Wrapping the translated conditional in a React fragment changes the return type from
stringtoReactNodewhile preserving the rendered output. This aligns with the broader pattern adopted throughout the codebase for translation outputs.frontend/public/components/modals/column-management-modal.tsx (1)
67-70: LGTM! Fragment wrapper consistent with PR pattern.The fragment wrapper changes the return type from
stringtoReactNodewhile preserving the displayed text. This is consistent with the translation output pattern adopted throughout the codebase.frontend/public/components/about-modal.tsx (1)
56-62: LGTM! Fragment wrapper maintains consistency.The fragment wrapper around the fallback text aligns with the PR-wide pattern for translation outputs, changing the return type from
stringtoReactNodewhile preserving functionality.frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx (1)
161-191: LGTM! All return paths consistently use fragment wrappers.All switch cases and the default fallback now return React fragments wrapping the translated text. This maintains consistency with the PR-wide pattern while preserving all functionality and display text.
frontend/public/components/cron-job.tsx (1)
88-91: LGTM! Fragment wrapper consistent with PR pattern.The fragment wrapper around the conditional translation maintains the pattern adopted throughout the codebase, changing the return type from
stringtoReactNodewhile preserving the boolean display logic.frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
772-780: No issues identified. The type widening of thetparameter fromTFunctiontoTFunction | ((key: string) => string)is backward compatible. All callers passtfrom theuseTranslation()hook (which returnsTFunction), and tests mock it asjest.fn(), both of which are compatible with the wider union type. The single usage oft()within the function body works correctly with both types in the union.frontend/package.json (1)
197-197: No action required. react-i18next@15.1.4 is compatible with react ^17.0.1 and i18next ^23.16.8, has no security vulnerabilities, and the codebase usage relies on stable APIs that are compatible with this version.frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
6-7: LGTM! Test mock aligns with broadened type signature.The simplified mock function type correctly reflects the production code's acceptance of both
TFunctionand plain string-mapping functions.frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx (1)
15-18: LGTM! Type broadening supports flexible translation function usage.The updated signature accepts both
TFunctionfrom i18next and plain string-mapping functions, enabling easier testing while maintaining production compatibility.frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
11-12: LGTM! Custom translation type simplifies interface.The
TranslationFunctiontype provides a cleaner interface while supporting both key-only and key-with-options translation patterns.frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
30-39: LGTM! API enhancement enables fluent usage and type flexibility.The updated signature accepts both
TFunctionand plain string-mapping functions, and returning the extension enables method chaining. This is a backward-compatible improvement.frontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.ts (1)
46-80: LGTM! Tests correctly updated for new API.The test properly validates:
- The broadened translation function type via
mockFnalias- The fluent API by checking return values
- Translation behavior remains unchanged
frontend/public/components/error.tsx (1)
95-122: Fragment wrapping is required for type compatibility with React.FC.The
LoginErrorMessagecomponent is typed asReact.FC, which requires aJSX.Elementreturn type. Sincet()from react-i18next v15.1.4 returns a string, the fragments (<>{t(...)}</>) are necessary to convert the string result to JSX. Removing them would cause TypeScript type errors. This is not unnecessary overhead but a required type safety measure.Likely an incorrect or invalid review comment.
dynamic-demo-plugin/package.json (1)
37-37: Expand verification scope for react-i18next: upgrade path is v11→v23, not v11→v15.The react-i18next upgrade to v15.1.4 is accompanied by i18next v23.16.8, spanning 12 major versions rather than 4. Breaking changes across v11→v23 are more extensive than v11→v15 alone.
Current state:
- Promise-based API (v13+ breaking change) is correctly implemented:
i18n.init({...}).then(...).catch(...)- Modern APIs only: useTranslation hooks and Trans component throughout; no deprecated withTranslation HOC
- useSuspense and fragment-wrapping are configured
Verify:
- TypeScript type augmentation moved to i18next package (v14→v15 change) — confirm in tsconfig/type declarations
- returnNull default behavior (changed to false in later versions) — confirm expected null-return behavior not relied upon
- The "fragment-wrapping pattern" mentioned in the original review—this term does not appear in official migration docs; clarify if this refers to React 18 Suspense boundaries or a different pattern
frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (1)
57-58: LGTM! Improved fallback translation function.The fallback translation function now returns the key itself instead of an empty string, which is better for debugging and provides a meaningful fallback when the translation function is unavailable.
frontend/packages/console-plugin-sdk/src/utils/useTranslationExt.ts (1)
11-18: This is a new file, not a modification of existing code, so the concern about "removal of explicit TFunction typing" is invalid.The file
useTranslationExt.tswas created as part of this commit (not modified from a prior version). The type design usingUseTranslationOptions<undefined>is intentional and aligns with react-i18next v15.1.4 behavior—when no namespace is explicitly supplied, the library uses the configured default namespace and falls back to undefined for generic inference. All existing usages inuseTranslatedExtensions.tsand tests inuseTranslationExt.spec.tspass without type errors, confirming that the callback type inference works correctly. No compatibility issues exist.Likely an incorrect or invalid review comment.
frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx
Show resolved
Hide resolved
krishagarwal278
force-pushed
the
CONSOLE-4778-new
branch
from
50ae957 to
0b2b36a
Compare
openshift-merge-robot
removed
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
781-781: Consider aligning withTranslationFunctiontype for consistency.The union type
TFunction | ((key: string) => string)differs from theTranslationFunctiontype introduced in other parts of this PR (e.g.,getVSphereHealth.ts). TheTranslationFunctiontype includes an optionaloptionsparameter:(key: string, options?: Record<string, unknown>) => string, which provides better alignment with i18next's actual usage patterns.While the current usage in this file (line 825) doesn't require the options parameter, standardizing on
TranslationFunctionwould improve consistency and future-proof the signature.🔎 Suggested alignment
- t: TFunction | ((key: string) => string), + t: TFunction | TranslationFunction,And add the import at the top:
import { TranslationFunction } from '@console/vsphere-plugin/src/components/getVSphereHealth';Or define it locally if cross-package imports aren't desired.
📜 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 (2)
dynamic-demo-plugin/yarn.lockis excluded by!**/yarn.lock,!**/*.lockfrontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
dynamic-demo-plugin/package.jsondynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsxfrontend/package.jsonfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.tsfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/packages/operator-lifecycle-manager/src/status/csv-status.tsfrontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsxfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/vsphere-plugin/src/components/utils.tsfrontend/public/components/about-modal.tsxfrontend/public/components/cron-job.tsxfrontend/public/components/custom-resource-definition.tsxfrontend/public/components/error.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/public/components/modals/configure-count-modal.tsxfrontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsxfrontend/public/components/pod.tsxfrontend/public/module/k8s/get-resources.ts
💤 Files with no reviewable changes (1)
- dynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsx
🚧 Files skipped from review as they are similar to previous changes (22)
- frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx
- frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx
- frontend/packages/console-plugin-sdk/src/utils/useTranslationExt.ts
- dynamic-demo-plugin/package.json
- frontend/public/components/modals/configure-count-modal.tsx
- frontend/public/components/custom-resource-definition.tsx
- frontend/packages/operator-lifecycle-manager/src/status/csv-status.ts
- frontend/public/components/about-modal.tsx
- frontend/packages/vsphere-plugin/src/components/utils.ts
- frontend/public/components/cron-job.tsx
- frontend/packages/metal3-plugin/src/validations/validations.tsx
- frontend/packages/console-plugin-sdk/src/utils/tests/extension-i18n.spec.ts
- frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx
- frontend/public/module/k8s/get-resources.ts
- frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts
- frontend/public/components/modals/column-management-modal.tsx
- frontend/packages/dev-console/src/components/import/tests/import-submit-utils.spec.ts
- frontend/public/components/pod.tsx
- frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx
- frontend/packages/topology/src/tests/TopologyShortcuts.spec.tsx
- frontend/packages/dev-console/src/components/catalog/providers/tests/useConsoleSamples.spec.ts
- frontend/packages/console-app/src/components/nodes/NodesPage.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/packages/vsphere-plugin/src/components/persist.tsfrontend/public/components/error.tsxfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/package.jsonfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx
🧬 Code graph analysis (4)
frontend/packages/vsphere-plugin/src/components/persist.ts (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/public/components/error.tsx (3)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/public/components/events.jsx (1)
error(386-386)frontend/public/components/namespace.jsx (1)
error(857-857)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
🔇 Additional comments (4)
frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (1)
57-58: LGTM! Good defensive fallback pattern.The fallback translation function ensures
getVSphereHealthalways receives a valid translation function, even whentis undefined. The identity function(key: string) => keyis a sensible fallback that returns the untranslated key.frontend/packages/vsphere-plugin/src/components/persist.ts (1)
17-17: LGTM! Consistent adoption ofTranslationFunctiontype.The migration from
TFunction<'vsphere-plugin'>toTranslationFunctionis applied consistently across all function signatures in this file. The usage patterns throughout the file (including interpolation with options) are fully compatible with theTranslationFunctionsignature.Also applies to: 95-95, 132-132, 252-252, 363-363, 554-554, 602-602
frontend/package.json (2)
180-180: i18next upgrade looks properly addressed. The breaking changes from v21 → v23 were comprehensively handled across the codebase (per TranslationFunction type updates and test refactoring), and version 23.16.8 exists with no reported security vulnerabilities. TypeScript 5.7.2 satisfies the v23 requirement for TS v5.
205-205: This upgrade is compatible with React 17.0.1 — no action required.react-i18next 15.1.4 has peer dependency
react >= 16.8.0, and your current React 17.0.1 satisfies this. The i18next dependency (>= 23.2.3) is also met by your pinned version 23.16.8. There are no known security vulnerabilities for v15.1.4, and no breaking changes affecting JavaScript usage with React 17.
| export type TranslationFunction = (key: string, options?: Record<string, unknown>) => string; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsRepository: openshift/console
Length of output: 3712
🏁 Script executed:
rg -nP --type=ts --type=tsx -C3 'getVSphereHealth\s*\(' -g '!*.spec.ts' -g '!*.test.ts'Repository: openshift/console
Length of output: 88
🏁 Script executed:
rg -nP --type=ts -C3 'getVSphereHealth\s*\(' -g '!*.spec.ts' -g '!*.test.ts'Repository: openshift/console
Length of output: 1848
🏁 Script executed:
cat -n frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxRepository: openshift/console
Length of output: 2823
Type abstraction is well-designed and consumers are compatible. The TranslationFunction type successfully abstracts the translation interface, improving testability as shown by the fallback implementation in healthHandler. All consumers pass compatible signatures.
However, fix missing translation wrapper at line 76: the message 'vsphere-plugin~Synchronization failed' should be wrapped with t() to match the pattern used elsewhere in the function and ensure proper translation:
message: t('vsphere-plugin~Synchronization failed')
🤖 Prompt for AI Agents
In frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts around
lines 11 and 76, the TranslationFunction type is fine but at line 76 the error
message string 'vsphere-plugin~Synchronization failed' is not wrapped with the
translation function; update that message to call
t('vsphere-plugin~Synchronization failed') so it follows the established pattern
and returns a translated string consistent with the rest of the function.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/public/locales/en/public.json (1)
1779-1787: Improve consistency and clarity of new error messages.The new localization entries could benefit from the following improvements:
Resource name formatting (Lines 1779, 1783): "kubecontrollermanager" should follow Kubernetes naming conventions. Consider using "KubeControllerManager" or "kube-controller-manager" to match the API resource format and improve readability.
Placeholder spacing inconsistency (Line 1781): The placeholder
{{ expectedValues }}has spaces around the variable, while line 1780 uses{{cm}}without spaces. Standardize spacing for consistency—existing patterns in the file predominantly use no spaces (e.g., line 1780).Generic error message (Line 1787): "Unexpected error" provides minimal context. Consider making it more descriptive or specific to the operation that failed to aid user troubleshooting.
Capitalization alignment: Line 1781 uses "configMap" while existing patterns capitalize resource types (e.g., line 523: "ConfigMaps"). Consider "ConfigMap" for consistency.
🔎 Suggested improvements
- "Failed to load kubecontrollermanager": "Failed to load kubecontrollermanager", + "Failed to load kubecontrollermanager": "Failed to load KubeControllerManager", "Failed to parse cloud provider config {{cm}}": "Failed to parse cloud provider config {{cm}}", - "The following content was expected to be defined in the configMap: {{ expectedValues }}": "The following content was expected to be defined in the configMap: {{ expectedValues }}", + "The following content was expected to be defined in the configMap: {{expectedValues}}": "The following content was expected to be defined in the ConfigMap: {{expectedValues}}", "Failed to persist {{secret}}": "Failed to persist {{secret}}", - "Failed to patch kubecontrollermanager": "Failed to patch kubecontrollermanager", + "Failed to patch kubecontrollermanager": "Failed to patch KubeControllerManager", "Failed to patch cloud provider config": "Failed to patch cloud provider config", "Failed to add taint to nodes": "Failed to add taint to nodes", "Failed to patch infrastructure spec": "Failed to patch infrastructure spec", - "Unexpected error": "Unexpected error", + "Unexpected error": "An unexpected error occurred",
📜 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 (3)
frontend/packages/operator-lifecycle-manager/locales/en/olm.jsonfrontend/packages/vsphere-plugin/locales/en/vsphere-plugin.jsonfrontend/public/locales/en/public.json
💤 Files with no reviewable changes (2)
- frontend/packages/operator-lifecycle-manager/locales/en/olm.json
- frontend/packages/vsphere-plugin/locales/en/vsphere-plugin.json
🧰 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/locales/en/public.json
krishagarwal278
force-pushed
the
CONSOLE-4778-new
branch
from
489994c to
db37e00
Compare
|
/retest |
|
@krishagarwal278: 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. |
4 participants
No description provided.