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

Use default namespace from useTranslation hook if supplied #195

Merged
merged 3 commits into from
Jan 13, 2020
Merged

Use default namespace from useTranslation hook if supplied #195

merged 3 commits into from
Jan 13, 2020

Conversation

heivo
Copy link
Contributor

@heivo heivo commented Jan 10, 2020

No description provided.

@heivo
Copy link
Contributor Author

heivo commented Jan 10, 2020

This is a fix for issue #161

Copy link
Member

@karellm karellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, I added a small comment.

I don't use this feature but I don't have any objection to add this. @vincaslt wdyt?

src/lexers/javascript-lexer.js Outdated Show resolved Hide resolved
@heivo
Copy link
Contributor Author

heivo commented Jan 13, 2020

I just saw that there is also a option to override the namespace (see #192 ), trying to fix this in this PR, too.

@heivo
Copy link
Contributor Author

heivo commented Jan 13, 2020

I added a feature to evaluate namespace overrides in the t-function options (see docs here: https://www.i18next.com/translation-function/essentials).

@hschmied
Copy link

@heivo if you could just help me out for a minute, I looked at the commit and see that your code checks if a namespace is given in useTranslation 'string' as well as an array of namespaces, in which case the first one is considered default. (lines 80-82 in your javascript-lexer.js commit)

we are using 1.0.6 of the parser, but for whatever reason, when using a namespace array when initializing 't', the parser is not parsing correctly keys without given namespace (falling back on default).

const {t, i18n} = useTranslation('localization', {useSuspense: true});
...
<Text>{t('test_fig')}</Text>

... correctly puts it into localization.json ...

const {t, i18n} = useTranslation(['localization', 'app', 'common'], {useSuspense: true});
...
<Text>{t('test_fig')}</Text>

... does not. instead it writes it into translation.json


Btw -- the test included with the commit only checks for useTranslate with namespace-string, but not array.

Thanks & best regards!
-H

@heivo
Copy link
Contributor Author

heivo commented May 18, 2020

@hschmied I think the problem is more related to lines 90-92: when an array is supplied as the first argument the namespace cannot be read from node.arguments[0].text but from node.arguments[0].elements[0].text.

Something like this should work:

if (node.expression.escapedText === "useTranslation" && node.arguments.length) {
	const { text, elements } = node.arguments[0];
	if (text) {
		this.defaultNamespace = text;
	} else if (elements && elements.length) {
		this.defaultNamespace = elements[0].text;
	}
}

@hschmied
Copy link

@heivo jep... that did it and works beautifully! How do you want to proceed to get this change in? I mean you fixed it and should get the credit. :)

@karellm
Copy link
Member

karellm commented May 21, 2020

This should be fixed in 1.0.7.

EDIT: actually you can now move to 2.0.0 which drops support for node 6 and 8 which are EOL.

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

Successfully merging this pull request may close these issues.

3 participants