Skip to content
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

failOnUpdate fails when no update happens, when using namespaces #597

Open
JulioC opened this issue Jun 8, 2022 · 3 comments
Open

failOnUpdate fails when no update happens, when using namespaces #597

JulioC opened this issue Jun 8, 2022 · 3 comments
Labels

Comments

@JulioC
Copy link

JulioC commented Jun 8, 2022

🐛 Bug Report

When using failsOnUpdates flag, the transformers emits an error even when no update is necessary. This only happens when using multiple namespaces.

It appears that the cause is that the namespaces won't be considered for the original number of tokens when calculating the change count (see below).

if (this.options.failOnUpdate) {
const addCount = uniqueCount[namespace] - mergeCount
if (addCount + restoreCount + oldCount !== 0) {
this.parserHadUpdate = true
continue
}
}

To Reproduce

Consider the JS below (or any code you have)

t('ns1:key1', 'Key 1');
t('ns2:key2', 'Key 1');

Now run the CLI against it to generate the JSON files, without failOnUpdate. This should run fine.

Finally, run the CLI once again, but this time add --fail-on-update. This will fail even without changes in JS source.

You can check that no update is necessary by add running the CLI without that flag once again.

Also, run with --fail-on-update --verbose to see the incorrect totals.

Expected behavior

It should only emit an error when update is necessary

Your Environment

  • runtime version: i.e. node v16
  • i18next version: i.e. 6.4.0
@karellm karellm added the Bug label Jun 9, 2022
@karellm
Copy link
Member

karellm commented Jun 9, 2022

@JulioC Could you make a PR with proper testing to fix this? Thanks for reporting.

@JulioC
Copy link
Author

JulioC commented Jun 13, 2022

@karellm I've added a new test in #600, they should be failing as described in this thread.


As an additional note, I've also had some trouble running the CLI test suite on Windows because it's trying to run yarn test:cli, which has two commands sequentially run using &&. This is not valid in powershell nor in cmd.exe.

I suggest moving the yarn -s build part of that command to a beforeAll block for the CLI tests. This should also improve test speeds as it would be run only once.

@Laityned
Copy link

Laityned commented Oct 5, 2022

Using the latest state of this repo, this issue can not be replicated. I wrote test cases directly in the parser.test.js as well. What I did found out is that this behaviour is probably related #634 . The number of added keys is wrong in case of an empty namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants