Skip to content

Return namespace in cimode with appendNamespaceToCIMode option#863

Merged
jamuhl merged 1 commit into
i18next:masterfrom
aray12:master
Jan 21, 2017
Merged

Return namespace in cimode with appendNamespaceToCIMode option#863
jamuhl merged 1 commit into
i18next:masterfrom
aray12:master

Conversation

@aray12

@aray12 aray12 commented Jan 19, 2017

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/Translator.js

// return key on CIMode
let lng = options.lng || this.language;
if (lng && lng.toLowerCase() === 'cimode') return keys[keys.length - 1];

@aray12 aray12 Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My tests for when appendNamespaceToCIMode: false failed initially before changes. This appears to be returning everything including the namespace if it is passed directly in the key (it is called before this.extractFromKey. I am not exactly sure why because this is the behavior I get in my application.

Below I started using key from this.exctractFromKey. Let me know if that was a bad idea.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.8%) to 66.437% when pulling a7b4380 on aray12:master into f888eb7 on i18next:master.

@jamuhl

jamuhl commented Jan 19, 2017

Copy link
Copy Markdown
Member

Thanks for the PR...your change is ok...just need to think if i need to make this a breaking version - as we get different results now. For sure like this more as no random ns:key or key only depending on the calling code.

@aray12

aray12 commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

We use i18next and we make heavy use of namespaces to break files up. The weird thing is I don't get inconsistent behavior in our application even though we specify namespace in the key a lot.

e.g. in cimode

t('ns:key') => key

I looked at versions as well as file history and I have no idea why. Anyway I am glad you approve of this approach. Since the behavior is different I would recommend a major version bump in case anyone depends on existing behavior. (unfortunately)

@aray12

aray12 commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

I also created a PR for i18next.com here:

i18next/i18next.com#17

@jamuhl

jamuhl commented Jan 21, 2017

Copy link
Copy Markdown
Member

just published i18next@6.0.0

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