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

Does not actually remove keys #202

Open
Stanzilla opened this issue Apr 2, 2023 · 18 comments
Open

Does not actually remove keys #202

Stanzilla opened this issue Apr 2, 2023 · 18 comments

Comments

@Stanzilla
Copy link

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
Copy link

senyaak commented Oct 9, 2023

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
Copy link
Author

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
Copy link

senyaak commented Oct 24, 2023

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
Copy link
Author

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
Copy link

senyaak commented Oct 24, 2023

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

@Stanzilla
Copy link
Author

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
Copy link

senyaak commented Oct 24, 2023

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

@Stanzilla
Copy link
Author

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
Copy link

senyaak commented Oct 24, 2023

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
Copy link
Author

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
Copy link

senyaak commented Oct 24, 2023

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

@Stanzilla
Copy link
Author

$t("app.about.author")

no luck

@Phlogistique
Copy link

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

@Phlogistique
Copy link

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
Copy link

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
Copy link

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

@Stanzilla
Copy link
Author

oh wow, great investigation

@Stanzilla
Copy link
Author

@senyaak @Spittal can we get this fixed?

Stanzilla added a commit to WeakAuras/WeakAuras-Companion that referenced this issue May 8, 2024
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

No branches or pull requests

3 participants