-
-
Notifications
You must be signed in to change notification settings - Fork 377
fix: change warning from error for mutiple useI18n local scope calling #2235
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
Conversation
WalkthroughThis change updates Vue I18n to replace the error thrown for duplicate local Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useI18n
participant Warnings
Component->>useI18n: call useI18n (local scope)
useI18n->>useI18n: check for existing Composer instance
alt No existing instance
useI18n-->>Component: return Composer instance
else Existing instance found
useI18n->>Warnings: issue DUPLICATE_USE_I18N_CALLING warning
useI18n-->>Component: return Composer instance
end
Estimated code review effort2 (~15 minutes) Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying vue-i18n-next with
|
Latest commit: |
bd074c9
|
Status: | ✅ Deploy successful! |
Preview URL: | https://a5f536ea.vue-i18n-next.pages.dev |
Branch Preview URL: | https://fix-multi-call-local-use-i18.vue-i18n-next.pages.dev |
@coderabbitai review |
✅ Actions performedReview triggered.
|
@intlify/core
@intlify/core-base
@intlify/devtools-types
@intlify/message-compiler
petite-vue-i18n
@intlify/shared
vue-i18n
@intlify/vue-i18n-core
commit: |
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
🧹 Nitpick comments (1)
docs/guide/advanced/composition.md (1)
403-469
: LGTM! Comprehensive documentation with minor fix needed.Excellent documentation that clearly explains the issue with multiple
useI18n
calls and provides actionable guidance with good/bad examples. The warning message documentation helps developers understand what they'll encounter.However, there's a minor formatting issue to address:
-``` -[Vue I18n warn]: Duplicate `useI18n` calling by local scope. Please don't call it on local scope, due to it does not work properly in component. -``` +```text +[Vue I18n warn]: Duplicate `useI18n` calling by local scope. Please don't call it on local scope, due to it does not work properly in component. +```This addresses the markdownlint warning about missing language specification for fenced code blocks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/guide/advanced/composition.md
(1 hunks)packages/vue-i18n-core/src/errors.ts
(2 hunks)packages/vue-i18n-core/src/i18n.ts
(1 hunks)packages/vue-i18n-core/src/warnings.ts
(2 hunks)packages/vue-i18n-core/test/i18n.test.ts
(4 hunks)
🧬 Code Graph Analysis (2)
packages/vue-i18n-core/src/i18n.ts (2)
packages/shared/src/warn.ts (1)
warn
(1-9)packages/vue-i18n-core/src/warnings.ts (2)
getWarnMessage
(38-43)I18nWarnCodes
(4-18)
packages/vue-i18n-core/test/i18n.test.ts (2)
packages/vue-i18n-core/src/warnings.ts (2)
warnMessages
(22-36)I18nWarnCodes
(4-18)packages/vue-i18n-core/src/i18n.ts (1)
useI18n
(697-787)
🪛 markdownlint-cli2 (0.17.2)
docs/guide/advanced/composition.md
465-465: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/vue-i18n-core/src/i18n.ts (2)
packages/shared/src/warn.ts (1)
warn
(1-9)packages/vue-i18n-core/src/warnings.ts (2)
getWarnMessage
(38-43)I18nWarnCodes
(4-18)
packages/vue-i18n-core/test/i18n.test.ts (2)
packages/vue-i18n-core/src/warnings.ts (2)
warnMessages
(22-36)I18nWarnCodes
(4-18)packages/vue-i18n-core/src/i18n.ts (1)
useI18n
(697-787)
🪛 markdownlint-cli2 (0.17.2)
docs/guide/advanced/composition.md
465-465: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build (ubuntu-latest, 18)
- GitHub Check: Build (macos-latest, 18)
- GitHub Check: Build (windows-latest, 18)
- GitHub Check: release
🔇 Additional comments (7)
packages/vue-i18n-core/src/errors.ts (1)
29-29
: LGTM! Correctly removes the error code for duplicate useI18n calls.The removal of
DUPLICATE_USE_I18N_CALLING
error code and its message is consistent with the PR objective to change duplicateuseI18n
calls from throwing errors to emitting warnings. The corresponding warning code has been properly added to the warnings module.Also applies to: 60-60
packages/vue-i18n-core/src/i18n.ts (1)
776-778
: LGTM! Correctly implements the error-to-warning behavior change.The implementation properly replaces the error throw with a warning emit for duplicate
useI18n
calls in local scope. The warning is appropriately guarded by development mode and local scope conditions, and uses the new warning infrastructure. This maintains backward compatibility while providing better developer guidance.packages/vue-i18n-core/src/warnings.ts (2)
15-15
: Good formatting improvement.Adding the trailing comma maintains consistency and makes future additions cleaner.
16-17
: LGTM! Well-implemented warning code and message.The new
DUPLICATE_USE_I18N_CALLING
warning code (13) follows the existing numbering scheme, and the warning message is clear and actionable. It effectively communicates the issue and provides guidance to developers about avoiding multipleuseI18n
calls in local scope.Also applies to: 34-35
packages/vue-i18n-core/test/i18n.test.ts (2)
38-38
: LGTM! Necessary import for warning testing.The import of
I18nWarnCodes
andwarnMessages
is required for the updated test that verifies warning behavior instead of error throwing.
627-686
: LGTM! Comprehensive test update for warning behavior.The test properly validates the new warning behavior for duplicate
useI18n
calls:
- Proper mocking: Correctly mocks the
warn
function and suppresses console output- Realistic scenario: Creates a practical test case with multiple
useI18n
calls through composables- Accurate verification: Checks that the exact warning message is emitted
- Clean structure: Well-organized test that follows best practices
The test effectively ensures the behavior change from error to warning works as intended.
docs/guide/advanced/composition.md (1)
378-401
: LGTM! Clear documentation of implicit useScope behavior.The new section effectively explains the implicit
useScope
resolution behavior, helping developers understand whenuseI18n
defaults to local vs global scope. The examples clearly illustrate both scenarios and promote explicit specification for better maintainability.
resolve #2223
resolve #2207
Summary by CodeRabbit
useScope
resolution and best practices to avoid multipleuseI18n
calls in a single component.useI18n
in local scope now trigger a warning instead of an error, improving developer experience.useI18n
calls occur.