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

Extracting text into i18n message into a namespace mistakenly create a property with the namespace name #920

Closed
andrejoaquim-dc opened this issue May 2, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@andrejoaquim-dc
Copy link

Describe the bug
Extracting text into i18n messages has a bug that creates an additional property to the JSON file equal to the namespace's name.
This bug is not present in version 2.8.1.

Extension Version
i18n-ally 2.9.0 for Visual Studio Code

Framework/i18n package you are using
react-i18next

To Reproduce
Steps to reproduce the behavior:

  1. Create a two translation files e.g. locales/en/indexPage.json and locales/en/aboutUsPage.json
  2. Extract some test in a component using "Extract text into i18n messages" into aboutUsPage namespace
  3. Check that the file contains "aboutPage" key.

I've the following configuration:

{
    "i18n-ally.defaultNamespace": "global",
    "i18n-ally.enabledFrameworks": ["react-i18next"],
    "i18n-ally.keystyle": "nested",
    "i18n-ally.localesPaths": "public/locales",
    "i18n-ally.pathMatcher": "{locale}/{namespaces}.json",
    "i18n-ally.namespace": true
}

Device Infomation

  • OS: macOS 13.0.1 (22A400) (Ventura)
  • Version: 2.9.0
  • VS Code Version: Version: 1.77.3 (Universal)

Extension Log
Go to View -> Output -> i18n Ally, and paste the content below. You should mask any sensitive information

Output > `i18n Ally`
🈶 Activated, v2.9.0

――――――

💼 Workspace root changed to "/Users/andre.joaquim/Documents/GitHub/next-marketing-app"
🌞 Enabled
🧩 Enabled frameworks: React I18next
🧬 Enabled parsers: json, yaml, json5

📈 Telemetry id: 3418a043-a095-4271-9017-483653ad3c18
🚀 Initializing loader "/Users/andre.joaquim/Documents/GitHub/next-marketing-app"
📂 Directory structure: dir
🗃 Custom Path Matcher: {locale}/{namespaces}.json
🗃 Path Matcher Regex: /^(?<locale>[\w-_]+)\/(?<namespace>.+)\.json$/

📂 Loading locales under /Users/andre.joaquim/Documents/GitHub/next-marketing-app/public/locales
	📑 Loading (en) en/aboutUsPage.json [1683043450818.9702]
	📑 Loading (en) en/indexPage.json [1683042949933.0496]

👀 Watching change on /Users/andre.joaquim/Documents/GitHub/next-marketing-app/public/locales
✅ Loading finished


――――――

💾 Writing /Users/andre.joaquim/Documents/GitHub/next-marketing-app/public/locales/en/aboutUsPage.json
✅ Loading finished

🔄 File changed (change) en/aboutUsPage.json
	📑 Loading (en) en/aboutUsPage.json [1683043493383.912]
✅ Loading finished


Screenshots

v2.9.0

Screen.Recording.2023-05-02.at.16.57.30.mov

v2.8.1 (where the issue's not happening)

Screen.Recording.2023-05-02.at.16.58.35.mov
@andrejoaquim-dc andrejoaquim-dc added the bug Something isn't working label May 2, 2023
@kibertoad
Copy link
Collaborator

@andrejoaquim-dc Thank you for the report! Would you be open to send a fix pull request for this?

@ocadoret
Copy link

ocadoret commented May 3, 2023

I think that this issue comes from the changes made in the getPathWithoutNamespace method

It used to check only on the presence of the namespace as a prefix, but now checks on namespace + delimiter (which is : with i18next framework) on a rewritten key (with all delimiters are normalized to .).

I'm not familiar enough with this project, so I might be wrong on that :)

@andrejoaquim-dc
Copy link
Author

@kibertoad Yes, I can try taking a look at it. @ocadoret thanks for flagging, I'll check if that's the case. Is there any test suite for this behaviour that I could check?

@andrejoaquim-dc
Copy link
Author

Hey @Swiftwork! Would you mind helping me out a bit in understanding where this bug might come from? Thanks in advance!

@Swiftwork
Copy link
Contributor

Swiftwork commented May 3, 2023

@andrejoaquim-dc unfortunately I don't have a deep insight into the project and it's been a while since I contributed that PR. I assume that it's probably just a statement somewhere that should have another check but I couldn't tell you where. Probably easiest is to add debug breakpoints and see why the namespace isn't removed. Might be that the extract text and inline resolve (the translation replacement) use the same function but need two different outputs. @ocadoret might be right in checking that. Unfortunately I don't have time to assist. Good luck hunting!

@jeansantodb1
Copy link

Yes, I'm facing the same issue right now. There is someone current working on this? Maybe with the right instructions i could try open a PR.

@andrejoaquim-dc
Copy link
Author

Hey @jeansantodb1, I still haven't find time to work on this, unfortunately :(

terales added a commit that referenced this issue May 13, 2023
…pace (#920)

* Re-enable adding strings to the proper namespace
* Re-enable navigation to strings in the proper namespace
* Remove a new line

---------

Co-authored-by: Alex Terehov <[email protected]>
@terales
Copy link
Collaborator

terales commented May 15, 2023

A fix for the issue is released in v2.9.1

@andrejoaquim-dc @jeansantodb1 can you check whether it was fully solved?

@terales terales closed this as completed May 15, 2023
@ocadoret
Copy link

v2.9.1 is working for me, thanks a lot !

@andrejoaquim-dc
Copy link
Author

Yes! Thanks a lot! 💪

@Swiftwork
Copy link
Contributor

@terales I assume there was a reason I added those additional delimiters but if the code works without them and no other issues arise, great! Glad to see that some more work on this great plugin is being done.

huacnlee pushed a commit to huacnlee/i18n-ally that referenced this issue Aug 28, 2023
…pace (lokalise#920)

* Re-enable adding strings to the proper namespace
* Re-enable navigation to strings in the proper namespace
* Remove a new line

---------

Co-authored-by: Alex Terehov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

6 participants