Repository: i18next/i18next-parser
Pull request #332 (2021-05-29 01:05:40)

Improve behaviour for missing defaults
### Why am I submitting this PR

I think it would be better if warnings were not generated for multiple instances of the same key where one is missing a default value.  Furthermore, there is an asymmetry in the current behaviour where default values only appear in the json if they are discovered last. Consider input A and input B.

#### input A
```ts
const myKeyNoDefault = t('my.key');
const myKey = t('my.key', 'foobar');
```

#### output A (current behaviour)

```
  [read]    /path/to/file.ts
  [warning] Found same keys with different values: my.key
```

```json
{
    "my": {
        "key": "foobar"
    }
}
```

#### input B
```ts
const myKey = t('my.key', 'foobar');
const myKeyNoDefault = t('my.key');
```

#### output B (current behaviour)

```
  [read]    /path/to/file.ts
  [warning] Found same keys with different values: my.key
```

```json
{
    "my": {
        "key": ""
    }
}
```

I think it would be better if it didn't generate a warning, and that you always got the default value regardless of the order.

#### output B (changed in this PR)

```
  [read]    /path/to/file.ts
```

```json
{
    "my": {
        "key": "foobar"
    }
}
```

#### input C

Of course you should still get a warning for doing something like this.

```ts
const myKey = t('my.key', 'foobar');
const myKeyNoDefault = t('my.key');
const myKeyDifferentDefault = t('my.key', 'bazquux');
```

#### output C (behaviour unchanged)
```
  [read]    /path/to/file.ts
  [warning] Found same keys with different values: my.key
```

```json
{
    "my": {
        "key": "bazquux"
    }
}
```
### Does it fix an existing ticket?

No

### Checklist

- [x] only relevant code is changed (make a diff before you submit the PR)
- [x] tests are included and pass (see [details here](https://github.com/i18next/i18next-parser/blob/master/docs/development.md#tests))
- [ ] documentation is changed or added
- [x] I ran `yarn build` to compile and prettify the code


codecov-commenter: # [Codecov](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next) Report
> Merging [#332](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next) (7dd4e5a) into [master](https://codecov.io/gh/i18next/i18next-parser/commit/a7075bcbe19b4f7925b2324bed029e503f00d535?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next) (a7075bc) will **increase** coverage by `0.05%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/i18next/i18next-parser/pull/332/graphs/tree.svg?width=650&height=150&src=pr&token=CJ74Vps41L&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next)](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next)

```diff
@@            Coverage Diff             @@
##           master     #332      +/-   ##
==========================================
+ Coverage   93.20%   93.26%   +0.05%     
==========================================
  Files           9        9              
  Lines         589      594       +5     
==========================================
+ Hits          549      554       +5     
  Misses         40       40              
```


| [Impacted Files](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next) | Coverage Δ | |
|---|---|---|
| [src/helpers.js](https://codecov.io/gh/i18next/i18next-parser/pull/332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next#diff-c3JjL2hlbHBlcnMuanM=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next). Last update [a7075bc...7dd4e5a](https://codecov.io/gh/i18next/i18next-parser/pull/332?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=i18next).


Patch:
diff --git a/README.md b/README.md
index b24072d..af614a0 100644
--- a/README.md
+++ b/README.md
@@ -346,7 +346,14 @@ The transform emits a `error:json` event if the JSON.parse on json files fail:
 
 The transform emits a `warning` event if the file has a key that is not a string litteral or an option object with a spread operator:
 
-`.pipe( i18next().on('warning:variable', (path, key) => {}) )`
+`.pipe( i18next().on('warning', (path, key) => {}) )`
+
+Here is a list of the warnings:
+
+- **Key is not a string literal**: the parser cannot parse variables so if your code contains something like `t(variable)`, the parser will through a warning.
+- **Found same keys with different values**: if your code contains the same key with different value in different places of your code, like `t('key', {defaultValue: 'foo'})` and `t('key', {defaultValue: 'bar'})`. The parser will select the latest one one but will throw a warning for you to fix this.
+- **Found translation key already mapped to a map or parent of new key already mapped to a string**: This happen in this kind of situation `t('parent', {defaultValue: 'foo'})` and `t('parent.child', {defaultValue: 'bar'})`. `parent` both be a translation and an object for `child`.
+
 
 ## Contribute
 
diff --git a/dist/helpers.js b/dist/helpers.js
index 16ac7e9..80d3689 100644
--- a/dist/helpers.js
+++ b/dist/helpers.js
@@ -83,7 +83,15 @@ function dotPathToHash(entry) {var target = arguments.length > 1 && arguments[1]
   var lastSegment = segments[segments.length - 1];
   var oldValue = inner[lastSegment];
   if (oldValue !== undefined && oldValue !== newValue) {
-    conflict = (0, _typeof2["default"])(oldValue) !== (0, _typeof2["default"])(newValue) ? 'key' : 'value';
+    if ((0, _typeof2["default"])(oldValue) !== (0, _typeof2["default"])(newValue)) {
+      conflict = 'key';
+    } else if (oldValue !== '') {
+      if (newValue === '') {
+        newValue = oldValue;
+      } else {
+        conflict = 'value';
+      }
+    }
   }
   duplicate = oldValue !== undefined || conflict !== false;
 
diff --git a/src/helpers.js b/src/helpers.js
index dc8184d..2743120 100644
--- a/src/helpers.js
+++ b/src/helpers.js
@@ -8,7 +8,7 @@
  * is `"key"` if a parent of the key was already mapped to a string (e.g. when
  * merging entry {one: {two: "bla"}} with target {one: "bla"}) or the key was
  * already mapped to a map (e.g. when merging entry {one: "bla"} with target
- * {one: {two: "bla"}}), `"value"` if the same key already exists swith a
+ * {one: {two: "bla"}}), `"value"` if the same key already exists with a
  * different value, or `false`.
  */
 function dotPathToHash(entry, target = {}, options = {}) {
@@ -83,7 +83,15 @@ function dotPathToHash(entry, target = {}, options = {}) {
   const lastSegment = segments[segments.length - 1]
   const oldValue = inner[lastSegment]
   if (oldValue !== undefined && oldValue !== newValue) {
-    conflict = typeof oldValue !== typeof newValue ? 'key' : 'value'
+    if (typeof oldValue !== typeof newValue) {
+      conflict = 'key'
+    } else if (oldValue !== '') {
+      if (newValue === '') {
+        newValue = oldValue
+      } else {
+        conflict = 'value'
+      }
+    }
   }
   duplicate = oldValue !== undefined || conflict !== false
 


Test patch:
diff --git a/test/helpers/dotPathToHash.test.js b/test/helpers/dotPathToHash.test.js
index 198c9a0..1dc8620 100644
--- a/test/helpers/dotPathToHash.test.js
+++ b/test/helpers/dotPathToHash.test.js
@@ -128,4 +128,26 @@ describe('dotPathToHash helper function', () => {
     assert.equal(conflict, 'key')
     done()
   })
+
+  it('uses old value when there is no new value and does not conflict', (done) => {
+    const { target, duplicate, conflict } = dotPathToHash(
+      { keyWithNamespace: 'one.two.three' },
+      { one: { two: { three: 'old' } } }
+    )
+    assert.deepEqual(target, { one: { two: { three: 'old' } } })
+    assert.equal(duplicate, true)
+    assert.equal(conflict, false)
+    done()
+  })
+
+  it('uses new value when there is no old value and does not conflict', (done) => {
+    const { target, duplicate, conflict } = dotPathToHash(
+      { keyWithNamespace: 'one.two.three', defaultValue: 'new' },
+      { one: { two: { three: '' } } }
+    )
+    assert.deepEqual(target, { one: { two: { three: 'new' } } })
+    assert.equal(duplicate, true)
+    assert.equal(conflict, false)
+    done()
+  })
 })