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

fix(lookup): fixed keys with dots #63

Closed
wants to merge 1 commit into from
Closed

fix(lookup): fixed keys with dots #63

wants to merge 1 commit into from

Conversation

Matergi
Copy link
Contributor

@Matergi Matergi commented May 23, 2023

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller
    PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into
    two PRs otherwise).
  • This PR's title starts is concise and descriptive.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or
    fixes.
  • I've updated any docs, .md files, etc… affected by this change.

What

Added support for the character '.' in the translation key

Why

Some translation keys can accurately reflect the translated English sentence, for example: 'Only available for Desktop. On the Mobile App, the display ecc...', so support for all punctuation characters is needed

The 3.9.2 version worked fine with dots.

Info

The lodash 'get' function splits by char '.' and looks for the keys, like dictionary[splitKey[0]][splitKey[1]][...];

// doesn't support this:
{
    "standard": {
        "Only available for Desktop. On the Mobile App, the display etc...": "translation",
    }
}

The problem is when a key contains a '.' because the lodash 'get' function interprets it not as part of the key but as a separator.
So, the solution is to use a reduce function to iterate through all the keys in the path generated by:

scope = getFullScope(i18n, scope, options)
.split(i18n.defaultSeparator)
.map((component) => i18n.transformKey(component))

@Matergi Matergi requested a review from fnando as a code owner May 23, 2023 13:15
@Matergi
Copy link
Contributor Author

Matergi commented May 23, 2023

fix this issue: #59

@mattias-persson
Copy link

@fnando can this be merged? Having the same issue...

@suparngp
Copy link

@fnando, facing the same issue. Can this be merged?

@fnando
Copy link
Owner

fnando commented Sep 12, 2023

@Matergi I rebased and added some additional changes required by this on #76. Could you please take a look at that branch and test everything to ensure we're not missing anything?

@fnando
Copy link
Owner

fnando commented Sep 12, 2023

I'm closing this in favor of #76. Please post new comments there.

@fnando fnando closed this Sep 12, 2023
Repository owner locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants