Spittal / vue-i18n-extract

Manage vue-i18n localization with static analysis
https://pixari.github.io/vue-i18n-extract/#what-is-it
MIT License
310 stars 85 forks source link

Does not actually remove keys #202

Open Stanzilla opened 1 year ago

Stanzilla commented 1 year ago

When i run npx vue-i18n-extract report --remove --vueFiles './src/components/**/*.?(js|vue)' --languageFiles './i18n/*.json' on my codebase the report correctly identifies the unused keys and says it would delete them but doesn't actually do it.

senyaak commented 1 year ago

Works for me. Would be nice if you could give the link to the existing source(best if it would be codesandbox or similar)

Stanzilla commented 1 year ago

Works for me. Would be nice if you could give the link to the existing source(best if it would be codesandbox or similar)

You can try with this repo https://github.com/WeakAuras/WeakAuras-Companion

senyaak commented 1 year ago

The vue-i18n-extract is missing in this repository... If I run your command npx vue-i18n-extract report --remove --vueFiles './src/components/**/*.?(js|vue)' --languageFiles './i18n/*.json' then I hav an output "The unused keys have been removed from your language files" and the keys are removed

Stanzilla commented 1 year ago

The vue-i18n-extract is missing in this repository... If I run your command npx vue-i18n-extract report --remove --vueFiles './src/components/**/*.?(js|vue)' --languageFiles './i18n/*.json' then I hav an output "The unused keys have been removed from your language files" and the keys are removed

Sadly not, they don't get removed which is what I'm reporting here.

senyaak commented 1 year ago

As I said, they are - problem on your side.

Stanzilla commented 1 year ago

As I said, they are - problem on your side.

Can you show a screenshot of the diff? Because it does not here, only changes the new-line at the end of the file

senyaak commented 1 year ago

I already deleted the repo. But here what I did:

Stanzilla commented 1 year ago

I already deleted the repo. But here what I did:

  • added a new key into the about.vue.
  • executed npx...
  • It landed in the en.json file.
  • removed new translation from about.vue
  • executed npx...
  • the new translation vanished

Problem there is that it should not even be needed to add a new key, there are unused keys in there that should be removed when you run the command, but they don't

senyaak commented 1 year ago

The plugin told me that there were found 4xx(I thing it was 415, but not sure now) unused keys, I didn't paid attention to them, since I'm not familiar with the project.

Stanzilla commented 1 year ago

It seems to be a little confused anyway, app.about.author for example is reported as unused but it is used for example image

senyaak commented 1 year ago

The comment probably is the issue here. Try to remove it and use just $t("app.about.author")

Stanzilla commented 1 year ago

$t("app.about.author")

no luck

Phlogistique commented 1 year ago

I have the same issue on my internal codebase. Will try to debug.

Phlogistique commented 1 year ago

So here's the issue on my side: my language files are simple key-value pairs in dot notation (hereafter "flat file"):

{
    "foo.bar": "Hello world",
    "foo.baz": "Goodbye",
    "foo.qux": "Thankyou"
}

vue-extract-i18n expects language files to be hierarchical instead (hereafter "hierarchical file"):

{
    "foo": {
        "bar": "Hello world",
        "baz": "Goodbye",
        "qux": "Thankyou"
    }
}

Internally, vue-i18n-extract uses the dot-object library to convert the hierarchical format to the flat data model.

The report creation accidentally works on "flat" language files because Dot.dot is a no-op on them. But when vue-i18n-extract tries to actually remove the key, it calls Dot.delete on the flat file, which does not work, as Dot expect a hierarchical file.

Vue-i18n-extract's code does not check the return value from Dot.delete, so it does not notice that it did not actually delete anything.

Phlogistique commented 1 year ago

One fix would be to update the removeUnusedFromLanguageFiles function to also work on flat files:

export function removeUnusedFromLanguageFiles (parsedLanguageFiles: SimpleFile[], unusedKeys: I18NItem[], dot: DotObject.Dot = Dot): void {
  parsedLanguageFiles.forEach(languageFile => {
    const languageFileContent = JSON.parse(languageFile.content);

    unusedKeys.forEach(item => {
      if (item.language && languageFile.fileName.includes(item.language)) {
        if (languageFileContent.hasOwnProperty(item.path)) {
          delete languageFileContent[item.path]
        } else if (dot.delete(item.path, languageFileContent) === undefined) {
          console.warn("Failed to remove key", item.path, "from", languageFile.fileName)
        }
      }
    });

    writeLanguageFile(languageFile, languageFileContent);
  });
}

However this is not ideal as using the "add missing keys" feature of vue-i18n-extract will change the format of the files.

Would you accept such a patch?

Phlogistique commented 1 year ago

Hah! another probable workaround: set the "separator" option to a random string.

Stanzilla commented 12 months ago

oh wow, great investigation

Stanzilla commented 10 months ago

@senyaak @Spittal can we get this fixed?