-
-
Notifications
You must be signed in to change notification settings - Fork 356
drop Legacy API #2110
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
drop Legacy API #2110
Conversation
Size ReportBundles
Usages
|
@intlify/core
@intlify/core-base
@intlify/devtools-types
@intlify/message-compiler
petite-vue-i18n
@intlify/shared
vue-i18n
@intlify/vue-i18n-core
commit: |
: GeneratedInstanceType extends true | ||
? VueI18n | ||
: ExportedGlobalComposer | ||
export type VueI18nInstance = ExportedGlobalComposer |
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.
@BobbieGoede
I think this change will be affected nuxt-i18n.
Can you check whether this change is okay with nuxt-i18n? 🙏
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.
Since these changes are for the next major of vue-i18n removing it should be fine!
We're actually still not yet (functionally) making use of the conditional type in nuxt-i18n v9 (uses vue-i18n v10), but we will in nuxt-i18n v10 (with vue-i18n v11).
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.
Thank you for quickly comment! 💚
Okay, We'll go on as we are!
// @ts-ignore | ||
expect(messages.en.mainMenu.buttonStart).toEqual('Start!') | ||
}) | ||
|
||
test('composition mode', () => { |
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.
By the legacy API dropping, users no longer need to be aware of the composition API. Let's roll up this test case into createI18n with flat json messages
and simplify it!
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.
Thank you for your review!
Is there any problem with this kind of understanding?
cd0c762?w=1
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.
Yeah! That's right! 💯
Thanks!
await nextTick() | ||
expect(html()).toMatchSnapshot('en') | ||
}) | ||
|
||
test('composable', async () => { |
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.
same as https://github.com/intlify/vue-i18n/pull/2110/files#r1956294943
Let's try to roll up to test case, if it's possible to do.
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.
Thank you so much!
Your contribution is so outstanding!
I've just reviewed your PR!
I've put some comments.
Please check it out!
FYI: |
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.
Well done!
Thank you ❤️ !
closes #2100
Is there any problem with this understanding of drop legacyAPI?
May still need to revise about JSDOC and Type