CONSOLE-5073: Bump i18next to latest#16150
Conversation
|
@logonoff: This pull request references CONSOLE-5073 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. |
|
/label px-approved |
📝 WalkthroughWalkthroughThis pull request upgrades the i18next internationalization library ecosystem across the frontend, including major version updates to i18next, react-i18next, and related packages. Configuration changes suppress a library support notice, adjust initialization options, and register custom formatters post-initialization. Dependency updates include adding a webpack polyfill plugin and removing d3, while the release notes are updated to reflect the new shared module requirement for react-i18next v16. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/public/i18n.js`:
- Around line 99-107: The i18n formatter registrations are calling the wrong
signatures: change the 'fromNow' formatter to pass the locale as the fourth
argument (use undefined for the second param) and change the 'dateTime'
formatter to invoke the factory before formatting; specifically, update the
i18n.services.formatter.add callbacks so fromNow is called as fromNow(val,
undefined, options, lng) and dateTimeFormatter is invoked as
dateTimeFormatter(lng).format(val) (adjusting the callbacks for the 'fromNow'
and 'dateTime' registrations that reference fromNow and dateTimeFormatter).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b9999da1-5b33-4e72-a114-877439cd9f05
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
frontend/i18next-parser.config.jsfrontend/package.jsonfrontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.mdfrontend/public/i18n.js
📜 Review details
🧰 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/i18next-parser.config.jsfrontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.mdfrontend/public/i18n.jsfrontend/package.json
🧬 Code graph analysis (1)
frontend/public/i18n.js (1)
frontend/packages/console-shared/src/utils/datetime.ts (2)
fromNow(84-136)dateTimeFormatter(41-48)
🔀 Multi-repo context openshift/console-operator
Linked repositories findings
openshift/console-operator
-
Plugin CRD / types referencing i18n plugin config:
- vendor/github.com/openshift/api/console/v1/types_console_plugin.go — ConsolePluginI18n field and LoadType enum references to i18n configuration. [::openshift/console-operator::vendor/github.com/openshift/api/console/v1/types_console_plugin.go]
- vendor/github.com/openshift/api/console/v1/zz_generated.crd-manifests/90_consoleplugins.crd.yaml — CRD schema describing
i18nconfig. [::openshift/console-operator::vendor/github.com/openshift/api/console/v1/zz_generated.crd-manifests/90_consoleplugins.crd.yaml] - vendor/github.com/openshift/client-go/console/applyconfigurations/console/v1/consolepluginspec.go — apply-configuration struct includes
I18nfield. [::openshift/console-operator::vendor/github.com/openshift/client-go/console/applyconfigurations/console/v1/consolepluginspec.go]
-
Console server / configmap code that builds i18n namespaces passed to the console frontend:
- pkg/console/subresource/consoleserver/config_builder.go — I18nNamespaces builder, I18nNamespaces field in generated config. (functions: ConsoleServerCLIConfigBuilder.I18nNamespaces, i18nNamespaces()) [::openshift/console-operator::pkg/console/subresource/consoleserver/config_builder.go]
- pkg/console/subresource/consoleserver/types.go — ConsoleServer config contains I18nNamespaces []string. [::openshift/console-operator::pkg/console/subresource/consoleserver/types.go]
- pkg/console/subresource/configmap/configmap.go — computes i18nNamespaces from plugins and adds to configmap. [::openshift/console-operator::pkg/console/subresource/configmap/configmap.go]
- pkg/api/api.go — V1Alpha1PluginI18nAnnotation constant ("console.openshift.io/use-i18n") used to indicate plugin i18n usage. [::openshift/console-operator::pkg/api/api.go]
-
Generated client / swagger docs referencing i18n:
- vendor/github.com/openshift/client-go/console/applyconfigurations/internal/internal.go — contains
i18nname entry. [::openshift/console-operator::vendor/github.com/openshift/client-go/console/applyconfigurations/internal/internal.go] - vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.go — swagger doc includes
"i18n": "i18n is the configuration of plugin's localization resources."[::openshift/console-operator::vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.go]
- vendor/github.com/openshift/client-go/console/applyconfigurations/internal/internal.go — contains
Relevance: the operator produces console server config and plugin-related CRD fields that include i18n namespaces and plugin i18n configuration. Upgrades in frontend i18next/react-i18next and changes to namespaces, formatter behavior, or config shape (as in the PR) could affect how the operator populates or expects i18n-related config, and how plugins declare/are served i18n resources.
🔇 Additional comments (4)
frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.md (1)
13-13: Release-note compatibility guidance is correctly updated.The
react-i18nextshared-module requirement now matches the v16 upgrade and communicates plugin compatibility clearly.frontend/i18next-parser.config.js (1)
3-9: Suppression shim is safely scoped for parser startup.This runs early, is guarded by
typeof globalThis !== 'undefined', and does not alter exported parser options.frontend/package.json (1)
188-192: Dependency upgrades are consistent with the i18n migration objective.The manifest changes align cleanly with the
react-i18nextv16/i18next ecosystem bump and related build-tool updates.Also applies to: 212-212, 239-239, 281-281, 299-299, 323-323, 327-327
frontend/public/i18n.js (1)
52-52:showSupportNoticetoggle placement looks good.Setting it inside init options keeps behavior explicit for runtime i18n initialization.
| i18n.services.formatter.add('number', (val, lng) => { | ||
| return new Intl.NumberFormat(lng).format(val); | ||
| }); | ||
| i18n.services.formatter.add('fromNow', (val, lng, options) => { | ||
| return fromNow(val, null, options); | ||
| }); | ||
| i18n.services.formatter.add('dateTime', (val) => { | ||
| return dateTimeFormatter.format(val); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Verify the signature/style of dateTimeFormatter and fromNow in datetime utils
fd -t f 'datetime\.(ts|tsx|js|jsx)$' | xargs -r rg -n -C2 'dateTimeFormatter|fromNow'
# 2) Confirm current formatter registrations in i18n bootstrap
rg -n -C3 "formatter\\.add\\('(number|fromNow|dateTime)'" frontend/public/i18n.jsRepository: openshift/console
Length of output: 3698
Fix formatter registration: dateTimeFormatter factory invocation and missing locale parameter.
At lines 105–106, dateTimeFormatter.format(val) is incorrect—dateTimeFormatter is a factory function, not an instance (defined in frontend/packages/console-shared/src/utils/datetime.ts:41). This will throw at runtime. The correct pattern is dateTimeFormatter(lng).format(val).
Similarly, at line 103, fromNow(val, null, options) omits the langArg parameter. The function signature at frontend/packages/console-shared/src/utils/datetime.ts:84 accepts a fourth parameter to ensure locale-aware relative time formatting. Pass lng as the fourth argument and use undefined instead of null for the second parameter to match existing codebase patterns.
Proposed patch
i18n.services.formatter.add('fromNow', (val, lng, options) => {
- return fromNow(val, null, options);
+ return fromNow(val, undefined, options, lng);
});
i18n.services.formatter.add('dateTime', (val) => {
- return dateTimeFormatter.format(val);
+ return dateTimeFormatter(lng).format(val);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/i18n.js` around lines 99 - 107, The i18n formatter
registrations are calling the wrong signatures: change the 'fromNow' formatter
to pass the locale as the fourth argument (use undefined for the second param)
and change the 'dateTime' formatter to invoke the factory before formatting;
specifically, update the i18n.services.formatter.add callbacks so fromNow is
called as fromNow(val, undefined, options, lng) and dateTimeFormatter is invoked
as dateTimeFormatter(lng).format(val) (adjusting the callbacks for the 'fromNow'
and 'dateTime' registrations that reference fromNow and dateTimeFormatter).
|
[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 |
| "{{count}} Worker Node": "{{count}} Worker Node", | ||
| "{{count}} Worker Node_plural": "{{count}} Worker Nodes", | ||
| "{{count}} Worker Node_one": "{{count}} Worker Node", | ||
| "{{count}} Worker Node_other": "{{count}} Worker Node", |
There was a problem hiding this comment.
nit: This change makes it look as if we are forcing dynamic plugins to bump their versions, which I believe is not the intention here.
There was a problem hiding this comment.
I can revert the changes in dynamic-demo-plugin
| return fromNow(val, null, options); | ||
| }); | ||
| i18n.services.formatter.add('dateTime', (val) => { | ||
| return dateTimeFormatter.format(val); |
There was a problem hiding this comment.
Although this doesn't appear to be entirely new, it must still be verified since we are bumping versions. The formatter registrations might have incorrect signatures: shouldn't the fromNow formatter pass lng as the 4th argument, and the dateTime formatter invoke dateTimeFormatter(lng) as a factory?
Did you verify these against the actual function signatures in datetime.ts?
|
@logonoff: The following test 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. |
The goal of this PR is to update the react-i18next shared module to a newer available version. Note we are still using the deprecated
i18next-parserinstead ofi18next-cli, that is out of scope for a simple shared module upgrade. There are some differences in configuration which may require additional work.Summary by CodeRabbit
Release Notes