-
Notifications
You must be signed in to change notification settings - Fork 7.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: switching language does not load the translation of the components library #4205
Conversation
…t correctly closes #4191
|
WalkthroughThe recent changes primarily involve the removal of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant LocaleManager
User->>App: Switch Language
App->>LocaleManager: Load Locale Messages
LocaleManager-->>App: Return Messages
App-->>User: Update UI
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
playground/src/locales/index.ts (1)
94-94
:loadMessages
is used externally.The
loadMessages
function is used in multiple external modules, including those in theapps/web-*
directories. This indicates that it is not solely for internal use within thepackages/locales
module.
apps/web-naive/src/locales/index.ts
apps/web-antd/src/locales/index.ts
apps/web-ele/src/locales/index.ts
Analysis chain
Confirm internal use of
loadMessages
.The
loadMessages
function is no longer exported, indicating it is intended for internal use within this module. Ensure that no external modules rely on this function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no external modules rely on the `loadMessages` function. # Test: Search for the usage of `loadMessages` in the codebase. Expect: No external usage. rg --type ts --type js 'loadMessages' --glob '!playground/src/locales/index.ts'Length of output: 816
Script:
#!/bin/bash # Check the directory structure to understand the relationship between the files. # This will help determine if the usages of `loadMessages` are internal or external. # List the directory structure for the relevant parts of the repository. tree -L 2 packages tree -L 2 appsLength of output: 1824
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- apps/web-antd/src/locales/index.ts (1 hunks)
- apps/web-ele/src/locales/index.ts (1 hunks)
- apps/web-naive/src/locales/index.ts (1 hunks)
- packages/locales/src/i18n.ts (3 hunks)
- playground/src/locales/index.ts (1 hunks)
Additional comments not posted (4)
apps/web-naive/src/locales/index.ts (1)
31-31
: Confirm internal use ofloadMessages
.The
loadMessages
function is no longer exported, indicating it is intended for internal use within this module. Ensure that no external modules rely on this function.Verification successful
No external reliance on
loadMessages
. TheloadMessages
function is not used by external modules, confirming its internal use within this module.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no external modules rely on the `loadMessages` function. # Test: Search for the usage of `loadMessages` in the codebase. Expect: No external usage. rg --type ts --type js 'loadMessages' --glob '!apps/web-naive/src/locales/index.ts'Length of output: 812
packages/locales/src/i18n.ts (1)
Line range hint
55-75
: Evaluate the impact of removingloadedLanguages
.The removal of the
loadedLanguages
set simplifies the logic but may lead to redundant loading of locale messages. Consider the performance implications and ensure this aligns with the intended behavior.Verification successful
Removal of
loadedLanguages
is justified and does not cause redundant loading. TheloadLocaleMessages
function already checks if the current locale matches the requested one and skips reloading if they are the same. This prevents unnecessary loading operations. Ensure that calls toloadLocaleMessages
with different languages are intentional and optimized for performance.
- The
loadLocaleMessages
function is used in several components and includes a check to avoid redundant loading.- The removal of
loadedLanguages
simplifies the code without introducing performance issues under normal circumstances.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Evaluate the performance impact of repeatedly loading locale messages. # Test: Check for potential performance issues related to loading the same language multiple times. # This script will simulate loading a language multiple times and measure the time taken. # Simulate loading the same language multiple times START_TIME=$(date +%s%3N) for i in {1..10}; do # Simulate loading a language (replace with actual loading logic if necessary) echo "Loading language $i" done END_TIME=$(date +%s%3N) echo "Total time taken: $((END_TIME - START_TIME)) ms"Length of output: 798
Script:
#!/bin/bash # Search for the implementation of the loadLocaleMessages function ast-grep --lang typescript --pattern 'async function loadLocaleMessages($_) { $$$ }'Length of output: 1020
Script:
#!/bin/bash # Search for all calls to the loadLocaleMessages function to understand its usage rg 'loadLocaleMessages'Length of output: 1316
apps/web-antd/src/locales/index.ts (1)
94-94
: Confirm internal use ofloadMessages
.The
loadMessages
function is no longer exported, indicating it is intended for internal use within this module. Ensure that no external modules rely on this function.Verification successful
No external reliance on
loadMessages
confirmed.The
loadMessages
function is not imported fromapps/web-antd/src/locales/index.ts
in any of the identified files, indicating that it is used internally within those modules. No external modules rely on this function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no external modules rely on the `loadMessages` function. # Test: Search for the usage of `loadMessages` in the codebase. Expect: No external usage. rg --type ts --type js 'loadMessages' --glob '!apps/web-antd/src/locales/index.ts'Length of output: 813
Script:
#!/bin/bash # Description: Check if `loadMessages` is imported from `apps/web-antd/src/locales/index.ts` in any of the identified files. # Search for import statements of `loadMessages` in the identified files. rg 'import.*loadMessages' packages/locales/src/i18n.ts playground/src/locales/index.ts apps/web-naive/src/locales/index.ts apps/web-ele/src/locales/index.tsLength of output: 158
apps/web-ele/src/locales/index.ts (1)
94-94
: Confirm internal use ofloadMessages
.The
loadMessages
function is no longer exported, indicating it is intended for internal use within this module. Ensure that no external modules rely on this function.Verification successful
No external reliance on
loadMessages
fromapps/web-ele/src/locales/index.ts
.The
loadMessages
function is not imported by other modules, indicating it is used internally within the module. The instances found in other files are likely independent implementations.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no external modules rely on the `loadMessages` function. # Test: Search for the usage of `loadMessages` in the codebase. Expect: No external usage. rg --type ts --type js 'loadMessages' --glob '!apps/web-ele/src/locales/index.ts'Length of output: 814
Script:
#!/bin/bash # Check if any of the files import `loadMessages` from `apps/web-ele/src/locales/index.ts` rg "import .*loadMessages.* from ['\"]\.\./\.\./apps/web-ele/src/locales/index['\"]" packages/locales/src/typing.ts packages/locales/src/i18n.ts playground/src/locales/index.ts apps/web-antd/src/locales/index.ts apps/web-naive/src/locales/index.tsLength of output: 253
closes #4191
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
Bug Fixes
loadMessages
function to prevent potential confusion regarding its usage in the localization process.Refactor