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

Add option to check for duplicate values to identical-keys #33

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rafaelgomesxyz
Copy link

@rafaelgomesxyz rafaelgomesxyz commented Jul 19, 2020

This is really useful when you have a main file, like /en/messages.json, from which all messages are copied to the other files, as it can detect messages that you probably forgot to translate. In cases where the value is expected to be duplicate, one can simply use the setting to ignore keys.

I got an error while running npm test, but I also got it when I tested as soon as I cloned the repo, so maybe it's because I'm on Linux?

Edit: Nope, looks like that's the same error that the Travis CI build reported.

@pnevares
Copy link
Contributor

Seeing the same snapshot error, looks like a dependency's behavior changed within a major version. If I roll back to the versions specified in package.json, snapshots render as expected.

@rafaelgomesxyz
Copy link
Author

Seeing the same snapshot error, looks like a dependency's behavior changed within a major version. If I roll back to the versions specified in package.json, snapshots render as expected.

Interestingly, if I roll back and then install the latest versions again, it seems to work. Weird.

@mayank23
Copy link
Contributor

mayank23 commented Aug 11, 2020

Hey @rafaelgssa , will review this PR and your other PR this week

Thanks!

@mayank23
Copy link
Contributor

Had to get a security release out, but will be reviewing this PR this week. Going one by one through the open PRs 😄 .

@rafaelgomesxyz
Copy link
Author

No problem! There's no rush.

@rafaelgomesxyz
Copy link
Author

Hey, I updated both PRs to pass the checks. Could you please take a look?

@mayank23
Copy link
Contributor

Thanks for the PR :), I have a few questions though, I'm a bit unclear on the use case based on the description.

I think in JSON, duplicate values lead to undefined behavior? I haven't really seen JSON objects with duplicate keys. Because a Javascript object has a unique set of keys, so in the end there is only one value for a property right?

Could you elaborate further on some use cases for having duplicate keys in your translations in the first place and your feature?

@rafaelgomesxyz
Copy link
Author

It's not about duplicate keys, but duplicate values for the same path across files, compared to a reference file. Let's say you have two files like this:

en-US.json

{
  "testingA": "Testing A",
  "testingB": "Testing B",
  "testingC": {
    "testingD": "Testing D",
    "testingE": "Testing E"
  }
}

pt-BR.json

{
  "testingA": "Testando A",
  "testingB": "Testing B",
  "testingC": {
    "testingD": "Testando D",
    "testingE": "Testing E"
  }
}

Where en-US.json is the reference file. I want to be able to detect that there are some values (testingB and testingC.testingE) that are not translated in the other file.

That's what this option does, it will show an error for those paths, because they have duplicate values compared to the reference file.

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

Successfully merging this pull request may close these issues.

None yet

3 participants