silence fallback warnings #510

SzNagyMisu · 2019-01-21T10:21:52Z

aims to fix #139

warnings should be silenced for fallback but not if there is no translation at all.

PR is incomplete as yet, but still have some questions.

* feature(option): add silentFallbackWarn to VueI18n constructor * silence fallback warnings * warn only if no translation is found at all

codecov-io · 2019-01-21T10:25:45Z

Codecov Report

Merging #510 into dev will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev     #510      +/-   ##
==========================================
+ Coverage   96.73%   96.75%   +0.02%     
==========================================
  Files           9        9              
  Lines         674      679       +5     
==========================================
+ Hits          652      657       +5     
  Misses         22       22

Impacted Files	Coverage Δ
src/mixin.js	`96.82% <100%> (+0.05%)`	⬆️
src/index.js	`99.65% <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 e879024...9a9da76. Read the comment docs.

SzNagyMisu · 2019-01-21T10:26:17Z

src/index.js

@@ -230,7 +236,7 @@ export default class VueI18n {
      if (isPlainObject(message)) {
        ret = message[key]
        if (typeof ret !== 'string') {
-          if (process.env.NODE_ENV !== 'production' && !this._silentTranslationWarn) {
+          if (process.env.NODE_ENV !== 'production' && !this._silentTranslationWarn && !(this._silentFallbackWarn && (this._isFallbackRoot() || locale !== this.fallbackLocale))) {


@kazupon is it ok to check locale !== this.fallbackLocale? I got confused because _translate gets fallback as a param, but it always points to this.fallbackLocale.

Also, we have a similar condition in line 246/252. I don't really understand the use case for that (boolean? number?) and I didn't find any tests, so I dared not touch to that. Should it contain the same logic? How do I test?

is it ok to check locale !== this.fallbackLocale?

Yes, that's right!

I got confused because _translate gets fallback as a param, but it always points to this.fallbackLocale.

The current vue-i18n is very complexity due to none of your business. 😭 In next major version, I will to write with full-scratch.

Also, we have a similar condition in line 246/252.

Just as you said, these is similar condition.
In unit test coverage (the following is removed /* istanbul ignore else */), the code of 246/249 is not passed in unit tests.

However, t or $t is returned the value of TranslateResult

vue-i18n/types/index.d.ts

Line 12 in e879024

type TranslateResult = string | LocaleMessages;

If you want to add a test case, we will be very helpful if you do so.
Although it is a bad design as API 😂, We need to maintain backwards compatibility.

In next major version, I'll plan to change this specification.

kazupon

Thank you for your PR!
I've reviewed your PR 👀
Please check it!

kazupon · 2019-01-22T06:09:28Z

src/index.js

@@ -230,7 +236,7 @@ export default class VueI18n {
      if (isPlainObject(message)) {
        ret = message[key]
        if (typeof ret !== 'string') {
-          if (process.env.NODE_ENV !== 'production' && !this._silentTranslationWarn) {
+          if (process.env.NODE_ENV !== 'production' && !this._silentTranslationWarn && !(this._silentFallbackWarn && (this._isFallbackRoot() || locale !== this.fallbackLocale))) {


is it ok to check locale !== this.fallbackLocale?

Yes, that's right!

I got confused because _translate gets fallback as a param, but it always points to this.fallbackLocale.

The current vue-i18n is very complexity due to none of your business. 😭 In next major version, I will to write with full-scratch.

kazupon · 2019-01-22T06:34:42Z

src/index.js

@@ -230,7 +236,7 @@ export default class VueI18n {
      if (isPlainObject(message)) {
        ret = message[key]
        if (typeof ret !== 'string') {
-          if (process.env.NODE_ENV !== 'production' && !this._silentTranslationWarn) {
+          if (process.env.NODE_ENV !== 'production' && !this._silentTranslationWarn && !(this._silentFallbackWarn && (this._isFallbackRoot() || locale !== this.fallbackLocale))) {


Also, we have a similar condition in line 246/252.

Just as you said, these is similar condition.
In unit test coverage (the following is removed /* istanbul ignore else */), the code of 246/249 is not passed in unit tests.

However, t or $t is returned the value of TranslateResult

vue-i18n/types/index.d.ts

Line 12 in e879024

type TranslateResult = string | LocaleMessages;

If you want to add a test case, we will be very helpful if you do so.
Although it is a bad design as API 😂, We need to maintain backwards compatibility.

In next major version, I'll plan to change this specification.

vuepress/api/README.md

Co-Authored-By: SzNagyMisu <szijjartonagy.misu@gmail.com>

* include case when pathRet is not null, undefined, array, plain object or string * provide test case

SzNagyMisu · 2019-01-22T10:19:32Z

test/unit/silent.test.js

+            silentFallbackWarn: true,
+            messages: {
+              en: { winner: 'winner' },
+              hu: { winner: true } // translation value is boolean


@kazupon is it the right use case? boolean, number etc?

is it the right use case?
yes! that's correct! not a string is warning!

kazupon · 2019-01-23T07:20:16Z

test/unit/silent.test.js

+            silentFallbackWarn: true,
+            messages: {
+              en: { winner: 'winner' },
+              hu: { winner: true } // translation value is boolean


is it the right use case?
yes! that's correct! not a string is warning!

kazupon · 2019-01-23T07:21:37Z

@SzNagyMisu Thanks!

SzNagyMisu · 2019-01-23T11:23:06Z

Thanks for the opportunity!

meelie · 2019-02-07T13:37:14Z

Thank you so much for this feature <3 !

jbnv · 2020-01-30T19:48:51Z

Question: How do we silence warning messages when there is no translation at all? I'm using Vue-I18n in an application framework where we probably will never actually need to translate to languages other than English, but Vue-I18n is very useful for loading configuration information. I'm finding that in a lot of cases it's less code to just write $t('Some text here') without defining a translation. But that results in a lot of unneeded warning messages.

SzNagyMisu · 2020-01-31T09:23:11Z

@jbnv
I would recommend considering the constructor option silentTranslationWarn or missing.
There might be other options better tailored to your need, so having a look at each constructor option might lead you to something I missed.

jbnv · 2020-01-31T20:30:21Z

That helps. Thanks.

Szijjártó Nagy Misu added 2 commits January 18, 2019 08:35

⚡ improvement(option): silentFallbackWarn (kazupon#139)

e92b51d

* feature(option): add silentFallbackWarn to VueI18n constructor * silence fallback warnings * warn only if no translation is found at all

adding typescript property declaration

db26d27

pr-triage bot added the PR: unreviewed label Jan 21, 2019

SzNagyMisu commented Jan 21, 2019

View reviewed changes

📝 docs(options): document silentFallbackWarn

2dbd95f

kazupon requested changes Jan 22, 2019

View reviewed changes

pr-triage bot added PR: reviewed-changes-requested and removed PR: unreviewed labels Jan 22, 2019

Update vuepress/api/README.md

8764925

Co-Authored-By: SzNagyMisu <szijjartonagy.misu@gmail.com>

pr-triage bot added PR: unreviewed and removed PR: reviewed-changes-requested labels Jan 22, 2019

⚡ improvement(option): silentFallbackWarn

9a9da76

* include case when pathRet is not null, undefined, array, plain object or string * provide test case

SzNagyMisu commented Jan 22, 2019

View reviewed changes

kazupon approved these changes Jan 23, 2019

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Jan 23, 2019

kazupon merged commit ddc0c79 into kazupon:dev Jan 23, 2019

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Jan 23, 2019

SzNagyMisu deleted the m_fallback_warning branch January 23, 2019 11:25

scriptcoded mentioned this pull request Aug 20, 2019

Disable warnings for fallback to root locale #139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

silence fallback warnings #510

silence fallback warnings #510

SzNagyMisu commented Jan 21, 2019

codecov-io commented Jan 21, 2019 •

edited

Loading

SzNagyMisu Jan 21, 2019

SzNagyMisu Jan 21, 2019

kazupon Jan 22, 2019

kazupon Jan 22, 2019

kazupon left a comment

kazupon Jan 22, 2019

kazupon Jan 22, 2019

SzNagyMisu Jan 22, 2019

kazupon Jan 23, 2019

kazupon Jan 23, 2019

kazupon commented Jan 23, 2019

SzNagyMisu commented Jan 23, 2019

meelie commented Feb 7, 2019

jbnv commented Jan 30, 2020

SzNagyMisu commented Jan 31, 2020

jbnv commented Jan 31, 2020

silence fallback warnings #510

silence fallback warnings #510

Conversation

SzNagyMisu commented Jan 21, 2019

codecov-io commented Jan 21, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazupon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazupon commented Jan 23, 2019

SzNagyMisu commented Jan 23, 2019

meelie commented Feb 7, 2019

jbnv commented Jan 30, 2020

SzNagyMisu commented Jan 31, 2020

jbnv commented Jan 31, 2020

codecov-io commented Jan 21, 2019 •

edited

Loading