feat: compatible Vue3 vapor mode #2299
Conversation
Deploying vue-i18n-next with
|
| Latest commit: |
2e2f7b2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a9c0d00a.vue-i18n-next.pages.dev |
| Branch Preview URL: | https://feat-vue-vapor3.vue-i18n-next.pages.dev |
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: |
WalkthroughUpdates pnpm Vue override to 3.6.0-alpha.2 and adapts code for Vue 3.6 compatibility by introducing Changes
Sequence Diagram(s)sequenceDiagram
participant Comp as Component
participant Setup as setupLifeCycle
participant I18n as I18nInternal
participant Dev as DevTools
Note over Comp,Dev: Mount (new flow)
Comp->>Setup: mount target (ComponentInternalInstance | GenericComponentInstance)
Setup->>I18n: __setInstance(target, composer)
I18n->>I18n: store in __instances (key = target)
Setup->>Comp: set target.__VUE_I18N__ = composer
Setup->>Dev: notify using instance.__VUE_I18N__
Note over Comp,Dev: Unmount (new flow)
Comp->>Setup: unmount target
Setup->>I18n: __deleteInstance(target)
I18n->>I18n: remove from __instances
Setup->>Comp: delete target.__VUE_I18N__
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/vue-i18n-core/src/i18n.ts (1)
⏰ 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). (14)
🔇 Additional comments (3)
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. Comment |
kazupon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/vue-i18n-core/src/i18n.ts (2)
492-503: Harden instance shape assumptions for Vapor (avoid NPE, robust inject key).Accessing
instance.appContext.appcan throw if Vapor’s instance shape differs. Use optional chaining and derive the injection key safely.@@ - if ( - !instance.isCE && - instance.appContext.app != null && - !instance.appContext.app.__VUE_I18N_SYMBOL__ - ) { + if ( + !instance.isCE && + (instance as any).appContext?.app && + !(instance as any).appContext.app.__VUE_I18N_SYMBOL__ + ) { throw createI18nError(I18nErrorCodes.NOT_INSTALLED) } @@ -function getI18nInstance(instance: ComponentInternalInstance | GenericComponentInstance): I18n { - const i18n = inject( - !instance.isCE ? instance.appContext.app.__VUE_I18N_SYMBOL__! : I18nInjectionKey - ) +function getI18nInstance(instance: ComponentInternalInstance | GenericComponentInstance): I18n { + const appSymbol = + !instance.isCE ? (instance as any).appContext?.app?.__VUE_I18N_SYMBOL__ : undefined + const key = (appSymbol as InjectionKey<I18n> | undefined) ?? I18nInjectionKey + const i18n = inject(key)Please run your Vapor e2e/tests that reproduce #2234 with
<script setup vapor>to confirm no NPEs occur before inject and thatNOT_INSTALLEDstill triggers as expected when the plugin isn’t provided.Also applies to: 565-575
597-606: Parent traversal may differ in Vapor. Add safe accessors.
root,parent, andvnode.ctxmight differ or be absent in Vapor. Guard with optional chaining to avoid crashes while walking up.function getComposer( i18n: I18n, - target: ComponentInternalInstance | GenericComponentInstance, + target: ComponentInternalInstance | GenericComponentInstance, useComponent = false ): Composer | null { let composer: Composer | null = null - const root = target.root + const root = (target as any).root let current: ComponentInternalInstance | GenericComponentInstance | null = - getParentComponentInstance(target, useComponent) + getParentComponentInstance(target, useComponent) while (current != null) { const i18nInternal = i18n as unknown as I18nInternal composer = i18nInternal.__getInstance(current) if (composer != null) { break } - if (root === current) { + if (root === current) { break } - current = current.parent + current = ((current as any).parent ?? null) as any } return composer } function getParentComponentInstance( - target: ComponentInternalInstance | GenericComponentInstance | null, + target: ComponentInternalInstance | GenericComponentInstance | null, useComponent = false ) { if (target == null) { return null } // if `useComponent: true` will be specified, we get lexical scope owner instance for use-case slots - return !useComponent ? target.parent : (target.vnode as any).ctx || target.parent // eslint-disable-line @typescript-eslint/no-explicit-any + return !useComponent + ? ((target as any).parent ?? null) + : ((target as any).vnode?.ctx) || (target as any).parent || null // eslint-disable-line @typescript-eslint/no-explicit-any }Add/confirm a unit test under Vapor that exercises slot/lexical owner traversal (
useComponent: true) to ensure parent resolution still finds the nearest composer.Also applies to: 620-628
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.json(1 hunks)packages/vue-i18n-core/src/composer.ts(4 hunks)packages/vue-i18n-core/src/devtools.ts(5 hunks)packages/vue-i18n-core/src/i18n.ts(7 hunks)packages/vue-i18n-core/src/utils.ts(3 hunks)packages/vue-i18n-core/src/vue.d.ts(2 hunks)packages/vue-i18n-core/test/i18n.test.ts(1 hunks)packages/vue-i18n-core/test/issues.test.ts(1 hunks)packages/vue-i18n-core/test/wc.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/vue-i18n-core/src/composer.ts (1)
packages/vue-i18n-core/src/vue.d.ts (2)
ComponentInternalInstance(15-26)GenericComponentInstance(28-39)
packages/vue-i18n-core/src/utils.ts (1)
packages/vue-i18n-core/src/vue.d.ts (2)
ComponentInternalInstance(15-26)GenericComponentInstance(28-39)
packages/vue-i18n-core/src/i18n.ts (1)
packages/vue-i18n-core/src/vue.d.ts (2)
ComponentInternalInstance(15-26)GenericComponentInstance(28-39)
packages/vue-i18n-core/src/devtools.ts (2)
packages/vue-i18n-core/src/vue.d.ts (3)
App(7-10)ComponentInternalInstance(15-26)GenericComponentInstance(28-39)packages/vue-i18n-core/src/i18n.ts (1)
global(389-391)
⏰ 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). (6)
- GitHub Check: Build (ubuntu-latest, 24)
- GitHub Check: Build (macos-latest, 24)
- GitHub Check: Build (windows-latest, 24)
- GitHub Check: release
- GitHub Check: release
- GitHub Check: Cloudflare Pages
🔇 Additional comments (11)
package.json (1)
165-165: LGTM! Vue 3.6 alpha is required for Vapor mode support.The upgrade to Vue 3.6.0-alpha.2 aligns with the PR objective to support Vue 3 Vapor mode. The alpha version is expected since Vapor is still experimental.
packages/vue-i18n-core/test/i18n.test.ts (1)
26-29: LGTM! Import relocation supports Vapor compatibility.The
getCurrentInstanceimport has been correctly relocated from Vue to the local utils module. This change aligns with the broader refactor to support Vue 3.6 Vapor mode via a unified instance accessor.packages/vue-i18n-core/test/issues.test.ts (1)
24-26: LGTM! Consistent with test refactoring.The
getCurrentInstanceimport relocation matches the pattern in other test files, maintaining consistency across the test suite.packages/vue-i18n-core/src/devtools.ts (1)
19-19: LGTM! Instance-based tracking supports Vapor mode.The changes correctly refactor devtools integration for Vapor compatibility:
- Type widening: Added
GenericComponentInstanceto support Vapor's component model- Tracking shift: Replaced
vnode.el.__VUE_I18N__withcomponentInstance.__VUE_I18N__for direct instance-based composer access- Return types: Widened
getComponentInstanceto return both traditional and generic component instancesThis approach is more robust for Vapor mode where vnode structures may differ from traditional Vue.
Also applies to: 64-65, 82-82, 137-139, 262-265
packages/vue-i18n-core/test/wc.test.ts (1)
5-18: LGTM! Import reorganization for Vapor compatibility.The updated imports correctly:
- Add required
fallbackWithLocaleChainandregisterLocaleFallbackerfrom@intlify/core-base- Relocate
getCurrentInstanceto the utils module- Maintain proper type imports for web component testing
These changes align with the broader Vapor mode support refactor.
packages/vue-i18n-core/src/vue.d.ts (1)
2-2: LGTM! New type enables Vapor mode support.The introduction of
GenericComponentInstanceis the core type enabling Vapor compatibility:
- Parallel structure: Mirrors
ComponentInternalInstancewithisCEand__VUE_I18N__properties- Composer storage: Both interfaces store the i18n
Composerinstance via__VUE_I18N__- Vapor compatibility: Allows the i18n system to work with both traditional Vue and Vapor component instances
This type augmentation properly extends Vue's module declarations for seamless integration.
Also applies to: 18-39
packages/vue-i18n-core/src/utils.ts (1)
13-13: LGTM! Unified instance accessor for Vapor compatibility.The changes create a compatibility layer for Vue 3.6 Vapor mode:
- Namespace import:
import * as Vueenables accessing bothcurrentInstance(Vapor) andgetCurrentInstance()(traditional)- Type widening:
getComponentOptionsnow accepts bothComponentInternalInstanceandGenericComponentInstance- Unified accessor: The new
getCurrentInstance()function checksVue.currentInstance || Vue.getCurrentInstance()to work in both modesThe comment correctly notes this is for Vue 3.6 compatibility. This approach provides a single source of truth for obtaining the current component instance across Vapor and non-Vapor contexts.
Also applies to: 18-23, 163-165, 217-220
packages/vue-i18n-core/src/composer.ts (1)
40-40: LGTM! Public API widened for Vapor compatibility.The changes correctly extend the Composer API for Vapor mode:
- Import relocation:
getCurrentInstancenow sourced from utils for unified access across Vapor/non-Vapor- Type extension: Added
GenericComponentInstanceto support Vapor's component model- API signature:
MissingHandlernow acceptsComponentInternalInstance | GenericComponentInstance, allowing it to work with both traditional and Vapor componentsThis is a backward-compatible widening of the API that maintains existing functionality while adding Vapor support.
Also applies to: 52-58, 113-113, 228-233
packages/vue-i18n-core/src/i18n.ts (3)
21-27: No changes needed—module augmentation is correctly implemented.The verification confirms that
packages/vue-i18n-core/src/vue.d.tsproperly augments the 'vue' module with theGenericComponentInstanceinterface (line 28) within adeclare module 'vue'block and exports it correctly. The import ini18n.tswill resolve without errors.
641-642: DevTools storage migration totarget.__VUE_I18N__verified: no DOM-based storage detected.Verification confirms:
- No remnants of
vnode.el.__VUE_I18N__storage patterns- All usages are properly instance-based:
target.__VUE_I18N__,componentInstance.__VUE_I18N__,instance.__VUE_I18N__- DevTools hooks correctly read from instance properties (devtools.ts lines 64-65, 137-139)
- Proper cleanup in place (i18n.ts line 659:
delete target.__VUE_I18N__)
10-10: Verification complete—no changes required.The implementation already follows the correct Vapor compatibility pattern. The fallback
Vue.currentInstance || Vue.getCurrentInstance()correctly attempts Vapor mode first, then falls back to classic Vue 3. Both modes provide ComponentInternalInstance with the required properties (appContext, root, parent), and the code includes defensive null checks (e.g., line 498 checksinstance.appContext.app != null). No Vapor-specific issues were found in the codebase.
1 participant
related #2234
Summary by CodeRabbit
New Features
Chores
Tests