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

"Go to Definition" does not work for nested pluralization keys #952

Closed
mkevinosullivan opened this issue Jun 23, 2023 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@mkevinosullivan
Copy link
Contributor

mkevinosullivan commented Jun 23, 2023

Describe the bug
With the incoming feature addition of displaying the first available pluralization key of a nested configuration (#950), right clicking on the key results in a "No definition found" message

Extension Version
i18n Ally (v2.9.1)

Framework/i18n package you are using
react-i18next (will also apply to yet-to-be-released i18next-shopify framework, based on react-i18next)

To Reproduce
Steps to reproduce the behavior:

  1. Go to a JSX file that uses a nested pluralization key
    • Locale configuration
    {
      "ProductsCard": {
        "productsCreatedToast": {
          "one": "{{count}} product created!",
          "other": "{{count}} products created!"
        },
      }
    }
    Screenshot 2023-06-23 at 11 58 29 AM
  2. Click on the line with the key, then right click to select "Go to Definition"
  3. See error
    NoDefnFound

Device Infomation

  • OS: Mac OS
  • Version: Ventura 13.4
  • VS Code Version: 1.79.2

Extension Log
Go to View -> Output -> i18n Ally, and paste the content below.

💼 Workspace root changed to "/Users/XXXXXXXXXX/src/shopify_apps/test-php-dependencies/web/frontend"
📦 Packages file "package.json" found
🕳 Packages file "pubspec.yaml" not exists
🕳 Packages file "composer.json" not exists
🕳 Packages file "Gemfile" not exists
🌞 Enabled
🧩 Enabled frameworks: React I18next, General
🧬 Enabled parsers: json, yaml, json5

📈 Telemetry id: 135ba03d-a31d-47be-bb68-22da5751c001
[telemetry] enabled: undefined
[telemetry] user: {"version":"2.9.1","feature_auto_detection":true,"feature_annotation_in_place":true,"feature_annotations":true,"feature_disable_path_parsing":false,"feature_extract_auto_detect":false,"feature_keep_fulfilled":false,"feature_prefer_editor":false,"feature_review_enabled":true,"feature_has_path_matcher":false,"feature_has_custom_framework":false}
🚀 Initializing loader "/Users/XXXXXXXXXX/src/shopify_apps/test-php-dependencies/web/frontend"
📂 Directory structure: file
🗃 Path Matcher Regex: /^(?<locale>[\w-_]+)\.(?<ext>json|ya?ml|json5)$/

📂 Loading locales under /Users/XXXXXXXXXX/src/shopify_apps/test-php-dependencies/web/frontend/locales
	📑 Loading (de) de.json [1686592188689.944]
	📑 Loading (en) en.json [1686680747853.0444]
	📑 Loading (fr) fr.json [1686592188686.6013]

👀 Watching change on /Users/XXXXXXXXXX/src/shopify_apps/test-php-dependencies/web/frontend/locales
✅ Loading finished

NB: incoming fix at Shopify#5

@trishrempel
Copy link
Contributor

This was resolved by #970

@GitHelge
Copy link

GitHelge commented Jul 21, 2023

Hey guys, I am using v2.10.0 but still have the warning for missing translations. However, I am not using the nested json structure:

Bildschirmfoto 2023-07-21 um 11 43 37
DE EN
Bildschirmfoto 2023-07-21 um 11 44 11 Bildschirmfoto 2023-07-21 um 11 44 31

This does give the correct preview:

"hello": {
      "one": "HelloOne",
      "other": "HelloOther"
}

This does not:

"hello_one": "HelloOne", 
"hello_other": "HelloOther"

Probably the "not-nested" version was not part of the issue and therefore not part of the fix. Should I open a new issue for this specific usage?

@trishrempel
Copy link
Contributor

Probably the "not-nested" version was not part of the issue and therefore not part of the fix. Should I open a new issue for this specific usage?

Correct, the pluralization format we use is a little different from i18next, and we didn't address fixing pluralization for the key_one, key_other style of pluralization keys. I recommend opening a separate issue for this specific usage.

Perhaps the solution introduced for this issue could be made more generic by referencing the derivedKeyRules property for each framework.

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

Successfully merging a pull request may close this issue.

3 participants