-
-
Notifications
You must be signed in to change notification settings - Fork 34
feat: Support better tags interpolation in Vue.js #3477
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
base: main
Are you sure you want to change the base?
Conversation
- Add tags interpolation similar how to do it in React - So the user can provide their <a> or <strong> component without the need to provide inner text as separate translation key
WalkthroughIntroduces a new Vue 3 component T2 that enables templated translations with slot injection. The component parses XML-like tags in translated strings and replaces them with corresponding slot content, supporting both wrapped and unwrapped rendering modes. Changes
Sequence DiagramsequenceDiagram
participant Parent as Parent Component
participant T2 as T2 Component
participant Hook as useTranslateInternal
participant Regex as Slot Parser
Parent->>T2: Render with modelValue + params
T2->>Hook: useTranslateInternal(props)
Hook-->>T2: t function
T2->>T2: Call t(params)
T2->>T2: Check if result is string or VNode
alt Result is VNode
T2-->>Parent: Return VNode directly
else Result is string
T2->>Regex: Parse for <slot-name>...</slot-name> tags
Regex-->>T2: Array of text & slot references
alt noWrap is true
T2-->>Parent: Return parsed array
else noWrap is false
T2->>T2: Wrap array in span element
T2-->>Parent: Return wrapped span
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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: 2
🧹 Nitpick comments (3)
packages/vue/src/T2.ts (3)
7-8: ImportVNodeas a type-only to avoid bundling itPrevents unnecessary runtime import.
-import { defineComponent, h, VNode } from 'vue'; -import type { PropType } from 'vue'; +import { defineComponent, h } from 'vue'; +import type { PropType, VNode } from 'vue';
52-56: Regex won’t match multiline tag contents and can mis-handle nested tagsCurrent
.*?doesn’t span newlines. Use a dotall-safe class to cover multiline; nested same‑name tags remain unsupported (acceptable for now, but document it).- const tagRegex = /<([a-zA-Z-]+)>(.*?)<\/\1>/g; + // Allow multiline inner text + const tagRegex = /<([a-zA-Z-]+)>([\s\S]*?)<\/\1>/g;Consider adding tests with newlines and repeated tags of the same name.
15-16: Normalize prop typing forparamsUse the object form for consistency with other props and better TS inference.
- params: Object as PropType<TranslateParams>, + params: { type: Object as PropType<TranslateParams> },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/vue/src/T2.ts(1 hunks)packages/vue/src/index.ts(1 hunks)testapps/vue/src/TranslationMethods.vue(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/vue/src/T2.ts (1)
packages/core/src/types/general.ts (4)
TranslationKey(7-7)TranslateParams(17-19)NsType(3-3)TranslateProps(28-35)
🔇 Additional comments (2)
packages/vue/src/index.ts (1)
3-3: Public surface OKRe-export of T2 looks correct and consistent with the package’s entrypoint.
testapps/vue/src/TranslationMethods.vue (1)
145-145: Import is fine once props alignImporting
{ T, T2 }is correct; after fixing the prop mismatch above, the sample should render as intended.
| modelValue: { type: String as PropType<TranslationKey>, required: true }, | ||
| params: Object as PropType<TranslateParams>, | ||
| defaultValue: String as PropType<string>, | ||
| noWrap: { | ||
| type: Boolean, | ||
| default: false, | ||
| }, | ||
| ns: { type: String as PropType<NsType> }, | ||
| language: { type: String as PropType<string> }, | ||
| }, |
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.
🛠️ Refactor suggestion | 🟠 Major
Prop/API mismatch: use keyName (and keep modelValue as alias) to match <T> and the example
modelValue as the translation key is unintuitive and breaks the provided example. Support keyName (preferred) and keep modelValue as a backward‑compatible alias; guard when both are missing.
Apply:
@@
export const T2 = defineComponent({
name: 'T2',
props: {
- modelValue: { type: String as PropType<TranslationKey>, required: true },
- params: Object as PropType<TranslateParams>,
+ // Prefer keyName for consistency with <T>, accept modelValue as an alias
+ keyName: { type: String as PropType<TranslationKey> },
+ modelValue: { type: String as PropType<TranslationKey> },
+ params: { type: Object as PropType<TranslateParams> },
defaultValue: String as PropType<string>,
@@
render() {
- // Get the translation text first
- const params: TranslateProps = {
- key: this.$props.modelValue,
+ // Pick key from keyName first, then modelValue (alias)
+ const resolvedKey = (this.$props as any).keyName ?? this.$props.modelValue;
+ if (!resolvedKey) {
+ if (process.env.NODE_ENV !== 'production') {
+ console.warn('[T2] keyName or modelValue is required.');
+ }
+ return null;
+ }
+ const params: TranslateProps = {
+ key: resolvedKey,
params: this.$props.params,
defaultValue: this.$props.defaultValue,
noWrap: this.$props.noWrap,
ns: this.$props.ns,
language: this.$props.language,
};Also applies to: 30-37
🤖 Prompt for AI Agents
In packages/vue/src/T2.ts around lines 14-23 (and also update similarly at lines
30-37), the component props use modelValue as the primary translation key which
conflicts with the <T> API and the example; add a new prop keyName: { type:
String as PropType<TranslationKey> } as the preferred prop and keep modelValue
as a backward-compatible alias (same type) while ensuring runtime logic reads
keyName first and falls back to modelValue, and throw or warn if both are
missing; update types and prop validation accordingly in both prop blocks
mentioned.
| <T2 | ||
| key-name="hello" | ||
| default-value="Pro vytváření <strong>Hostů</strong> je třeba mít <link-to-help>ověřenou doménu</link-to-help>" | ||
| > |
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.
🧩 Analysis chain
Fix prop name: T2 currently requires modelValue, not keyName.
Example uses key-name, so required prop is missing and key becomes undefined. Either:
- Change usage to
model-value="hello", or - Update T2 to accept
keyName(preferred for consistency with<T>).
See proposed T2 diff below to support both.
Run to locate other usages:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 --type=vue '<T2\b[^>]*\b(key-name|model-value)\b' testapps/vue || trueLength of output: 27
🏁 Script executed:
#!/bin/bash
# Find T2 component definition
fd -e vue . testapps/vue --exec grep -l 'export.*T2\|name.*T2\|component.*T2' {} \; 2>/dev/null | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for T2 component definition in components directory
fd -e vue . testapps/vue -path '*component*' -o -path '*T2*' 2>/dev/null | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find all T2 usages without type filter
rg -n '<T2' testapps/vue --max-count 50Length of output: 57
🏁 Script executed:
#!/bin/bash
# Look at the specific file mentioned in review
cat testapps/vue/src/TranslationMethods.vue | sed -n '115,125p'Length of output: 426
🏁 Script executed:
#!/bin/bash
# Get imports and script section from TranslationMethods.vue
head -30 testapps/vue/src/TranslationMethods.vueLength of output: 682
🏁 Script executed:
#!/bin/bash
# Search for T2 export or definition across entire codebase
rg -n 'export.*component.*T2|export default.*T2|<script.*setup|name:\s*["\x27]T2' --max-count 50Length of output: 1005
🏁 Script executed:
#!/bin/bash
# Look for T2 in components or other common locations
fd T2 . --type f --type d 2>/dev/null | head -20Length of output: 24
🏁 Script executed:
#!/bin/bash
# Search for T component definitions (might clarify pattern)
rg -n '<T\b|<T1\b|component.*\bT[0-9]' testapps/vue/src --max-count 20Length of output: 965
🏁 Script executed:
#!/bin/bash
# View T2 component definition
cat packages/vue/src/T2.tsLength of output: 3208
🏁 Script executed:
#!/bin/bash
# View T component definition for comparison
cat packages/vue/src/T.tsLength of output: 1245
🏁 Script executed:
#!/bin/bash
# Check for vue-i18n-ally package main export
cat packages/vue/src/index.ts | head -50Length of output: 347
Fix T2 prop: use model-value instead of key-name.
T2 requires the modelValue prop (rendered as model-value in templates), but the usage provides key-name. This leaves modelValue undefined, breaking the translation. Either:
- Change to
model-value="hello", or - Update T2 to accept
keyNamefor consistency with the<T>component.
🤖 Prompt for AI Agents
In testapps/vue/src/TranslationMethods.vue around lines 118 to 121, the T2 usage
passes key-name but T2 expects modelValue (model-value in templates), so the
translation is broken; fix by replacing key-name="hello" with
model-value="hello" in the T2 tag, or alternatively update the T2 component to
accept a keyName prop (map it to modelValue internally) to keep parity with the
<T> component — choose the model-value change for the quickest fix or implement
the prop alias in T2 if you need backwards compatibility.
Summary by CodeRabbit