Skip non-element VNode in interpolation

[myst729]

Please visit #210 for the details.

fix #210

[kazupon]

Thank you for your PR! 👍

However, In CircleCI, Occurred error. 😞
Could you fix it?

[codecov-io]

Codecov Report

Merging #211 into dev will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev     #211      +/-   ##
==========================================
+ Coverage   96.88%   96.89%   +0.01%     
==========================================
  Files           8        8              
  Lines         545      547       +2     
==========================================
+ Hits          528      530       +2     
  Misses         17       17

Impacted Files	Coverage Δ
src/component.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 475dd21...6c71780. Read the comment docs.

[myst729]

Yes, I also noticed that unit tests fail. And I thought again, this is a bit complicated.

As addressed in #210 , line breaks are considered as VNodes. Follow code produces 3 VNodes: two elements and the line break in between.

<i18n path="msg" tag="div">
  <strong>bold</strong>
  <em>italic</em>
</i18n>

However, following code has exactly the same VNodes parsed: the whitespace is a VNode too.

<i18n path="msg" tag="div">
  <strong>bold</strong> <em>italic</em>
</i18n>

I think what VDom done is right, because it behaves similar as how browsers parse line breaks or contiguous whitespaces between elements. So the question shifts here: are the text nodes inside <i18n> meaningful, or not?

Before component interpolation this is very simple. All text interpolations are passed within an object to the $t method. The user has full control of them, so they're all meaningful.

But for component interpolation, it's not easy to answer. And current test cases are only designed for meaningful text, e.g. data binding. In my opinion, if the text can be determined ahead of runtime, it should be moved to the message string, even though it has the same appearance in different languages.

So my current approach is to check both tag and text of the VNode:

if (child.tag || child.text.trim()) {
  params.push(child)
}

This will skip all text content that consists of white spaces (general, not only %20).

[myst729]

This part is discussion, no implementation yet.

Based on the PR, the last question still remains: as of <i18n> component, shall we give full control of text content back to the user? For, example, some sort of an <i18n-text> child component (which is absolutely not elegant).

<i18n path="msg" tag="div">
  <strong>bold</strong>
  <i18n-text>{{ plainText }}</i18n-text>
</i18n>

Maybe another approach is to add a place attribute for mixed content.

messages: {
  en: {
    msg: 'This is {0} and {1}.'
  }
},

data: {
  plainText: 'plain text'
}

<i18n path="msg" tag="div" :places="{'1': plainText}">
  <strong place="0">bold</strong>
</i18n>

Renders:

<div>This is <strong>bold</strong> and plain text.</div>

For pure element interpolations, the place attribute can be optional (but if being set, must be all-in).

<i18n path="msg" tag="div">
  <strong>bold</strong>
  <em>italic</em>
</i18n>
<i18n path="msg" tag="div">
  <strong place="1">bold</strong>
  <em place="0">italic</em>
</i18n>

Renders:

<div>This is <strong>bold</strong> and <em>italic</em>.</div>
<div>This is <em>italic</em> and <strong>bold</strong>.</div>

I personally prefer this. It also behaves consistently with text interpolations of key value pairs.

[myst729]

As I dig deeper into the source code, I found another issue that may be a barrier of implementing the places approach I mentioned above. The list formatting is implemented individually with named formatting.

[vue-i18n] Type of token 'list' and format of value 'named' don't match!

For messages interpolated with "numbers":

messages: {
  en: {
    msg: 'This is {0} and {1}.'
  }
}

I could only pass an array:

<div>{{ $t('msg', ['bold', 'italic']) }}</div>

If I pass an object:

<div>{{ $t('msg', {'0': 'bold', '1': 'italic'}) }}</div>

The warning message appears, which is fine. However, the translation stops there. This is a bit too strict to the users. And it doesn't behave consistently with JavaScript.

// These two expressions output the same result
Object.keys(['bold', 'italic'])
Object.keys({'0': 'bold', '1': 'italic'})

I've read some other i18n libs or components before, e.g. Dojo, Angular, etc. Under the hood, they all make list formatting syntax sugar of named formatting. To the users, this brings flexibility of formatting messages interpolated with "numbers". To the developers, reuse of such mechanism could be a lot easier to ship features like the "places" idea I proposed.

[kazupon]

Thank you for your fix and consideration!

So the question shifts here: are the text nodes inside meaningful, or not?

Sorry, my poor documentation. 🙇
In currently, text nodes inside <i18n> not meaningful due to assumed the use of the html tag expansion.
In about text nodes, I had not thought about it so much. 😅

shall we give full control of text content back to the user?

In about this, I don't know it, due to cannot imagine use-case. 💭❓
if we'll support it, I prefer place special attribute.

The list formatting is implemented individually with named formatting.
they all make list formatting syntax sugar of named formatting. To the users, this brings flexibility of formatting messages interpolated with "numbers".

Oh! 💡That's good idea!
However, That improvement is big affecting to vue-i18n codes.
I'd like to improve it in next major release.

[myst729]

In currently, text nodes inside <i18n> not meaningful due to assumed the use of the html tag expansion.

If plain text is not under consideration, then I guess this PR is compatible with current design purpose.

In about this, I don't know it, due to cannot imagine use-case. 💭❓

<i18n> is designed for the use cases of html interpolations, I see it a perfect solution for #75 (which, yes, was opened by me 😅). So let's rewrite the demo code in that issue with <i18n>:

const locales = {
  en: {
    info: 'You can {0} until 30 minutes from departure.',
    refund: 'refund the ticket',
    change: 'change your flight'
  },
  cn: {
    info: '办理{0}手续的截止时间为起飞前 30 分钟。',
    refund: '退票'
    change: '改签'
  }
}

<i18n path="info" tag="p">
  <a href="http://example.com">{{ $t('refund') }}</a>
</i18n>

Changing the refund link is not a matter of message string any longer. Also, I can now make it just plain text, if the online service is not available yet. Whether the service is available is not a matter of message string either. Plus I could define more actions at ease. So this is really great!

Let's go one step further. Say, for different actions the time limits vary. How do we do?

const locales = {
  en: {
    info: 'You can {0} until {1} minutes from departure.',
    refund: 'refund the ticket',
    change: 'change your flight'
  },
  cn: {
    info: '办理{0}手续的截止时间为起飞前 {1} 分钟。',
    refund: '退票',
    change: '改签'
  }
}

<i18n path="info" tag="p">
  <a href="http://example.com">{{ $t('change') }}</a>
  15
</i18n>

Or maybe better:

data: {
  limit: {
    refund: 30,
    change: 15
  }
}

<i18n path="info" tag="p">
  <a href="http://example.com">{{ $t('change') }}</a>
  {{ limit.change }}
</i18n>

Mixed content interpolations of element and plain text. But this has a tricky pitfall, what if the user wants to pass an empty string in his use case? How do we know it's by intention, or just another line break? We really couldn't.

if we'll support it, I prefer place special attribute.

With place/places, this could be:

<i18n path="info" tag="p" :places="{'1': limit.change}">
  <a place="0" href="http://example.com">{{ $t('change') }}</a>
</i18n>

If we force the use of places for plain text inside <i18n>, then dealing with text nodes in <i18n> could be so easy - just ignore them (that's what the first commit does)! Since this component is designed for element interpolations in the beginning.

However, That improvement is big affecting to vue-i18n codes.
I'd like to improve it in next major release.

I agree. It's a big architectural change. It does have necessity of thinking twice before move.

[kazupon]

Thank you for detailed explanation!

I could understand about use-cases!
So, we would like to support it.

Could you implement it please?

If you can not implement, I hope you open the issues about that specification as docs. In this case, I merge this PR, after that I sometime implement it.

[myst729]

It may not be very soon, but yep, I'll be glad to involve.

[kazupon]

Thank you for your issue registering!
Okay, then I'll merge your PR.