-
-
Notifications
You must be signed in to change notification settings - Fork 517
fix: handle falsy options.i18n types
#3900
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,11 +54,11 @@ | |
| "vue-i18n (without message compiler)": roundToKilobytes(withVueI18nDropCompiler.totalBytes - base.totalBytes), | ||
| }; | ||
|
|
||
| expect(data).toMatchInlineSnapshot(` | ||
|
Check failure on line 57 in test/bundle.test.ts
|
||
| { | ||
| "module": "69.4k", | ||
| "module (without vue-i18n)": "26.0k", | ||
| "vue-i18n": "43.3k", | ||
| "vue-i18n": "43.4k", | ||
|
||
| "vue-i18n (without message compiler)": "26.8k", | ||
| } | ||
| `); | ||
|
|
||
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.
Potential logic inconsistency in warning message.
When
nuxt.options.i18nisfalse, line 25 correctly falls back tooptions.strategy(the default), but the warning message check on line 29 evaluates(false && false.strategy) == null→false, which means "(used by default)" won't be appended.This is inconsistent: the strategy IS being used by default when
nuxt.options.i18nisfalse, so the message should indicate this.🔎 Suggested fix
Option 1 (explicit null check):
Option 2 (optional chaining is safe here since we're only checking null/undefined):
Both options correctly handle the
falsecase, but Option 1 aligns better with the PR's approach of avoiding optional chaining.📝 Committable suggestion
🤖 Prompt for AI Agents