vue-i18n-extract icon indicating copy to clipboard operation
vue-i18n-extract copied to clipboard

Does not actually remove keys

Open Stanzilla opened this issue 2 years ago • 19 comments

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.

Stanzilla avatar Apr 02 '23 00:04 Stanzilla

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

senyaak avatar Oct 09 '23 14:10 senyaak

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

Stanzilla avatar Oct 09 '23 14:10 Stanzilla

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

senyaak avatar Oct 24 '23 19:10 senyaak

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.

Stanzilla avatar Oct 24 '23 19:10 Stanzilla

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

senyaak avatar Oct 24 '23 19:10 senyaak

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

Stanzilla avatar Oct 24 '23 19:10 Stanzilla

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

senyaak avatar Oct 24 '23 19:10 senyaak

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

Stanzilla avatar Oct 24 '23 19:10 Stanzilla

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.

senyaak avatar Oct 24 '23 19:10 senyaak

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

Stanzilla avatar Oct 24 '23 19:10 Stanzilla

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

senyaak avatar Oct 24 '23 19:10 senyaak

$t("app.about.author")

no luck

Stanzilla avatar Oct 24 '23 19:10 Stanzilla

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

Phlogistique avatar Oct 26 '23 07:10 Phlogistique

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 avatar Oct 26 '23 08:10 Phlogistique

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 avatar Oct 26 '23 08:10 Phlogistique

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

Phlogistique avatar Oct 26 '23 08:10 Phlogistique

oh wow, great investigation

Stanzilla avatar Oct 30 '23 05:10 Stanzilla

@senyaak @Spittal can we get this fixed?

Stanzilla avatar Dec 08 '23 03:12 Stanzilla