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

3.1.1 Feature or Bug? #254

Closed
dazlious opened this issue May 11, 2017 · 9 comments
Closed

3.1.1 Feature or Bug? #254

dazlious opened this issue May 11, 2017 · 9 comments

Comments

@dazlious
Copy link

@dazlious dazlious commented May 11, 2017

Is it a feature, that only the first namespace should be passed?

Changelog.md states:

fixes issue in fixing t function - pass only first namespace not an array of namespaces

But this is not a minor change. It broke a lot of our translation.

We used it like:

export default translate(['namespace1', 'namespace2'])(Component);

Can you at least release it as major?

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 11, 2017

already to late to release as major. Patch version is out. And will be applied to all ^3.1.0 anyway.

But yes that's an issue - was wrong. The array is to assert all those namespaces are loaded.

The first namespace in the array gets set as default. Before all namespaces passed acted as fallback namespaces -> that would result in missing keys getting added to the last namespace in that list - which is wrong.

to access namespace2 inside your view you would always had to use this.props.t('namespace2:key')

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 11, 2017

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 11, 2017

And sorry for the troubles added to your project - will next time do a major if "grayzone" fix. Was aware of being an issue if someone relied on the fallback to those namespaces - but hoped people to prepend namespace on calling t like documented.

@vpriem

This comment has been minimized.

Copy link
Contributor

@vpriem vpriem commented May 11, 2017

Hi jamuhl,

why not revert it as the next minor and do this as a major? We will not be the only one having this breaking change.

Anyway when I take a look at http://i18next.com/translate/namespace/#basic it seems to be a standard (and nice) feature to pass several namespaces and use the last one as a fallback, without having to prefix it. So honestly I don't understand the benefit of that change/fix or do you mean this is an issue and you will provide a fix?

BTW: translate('namespace1') instead of translate(['namespace1') is nice and should be documented.

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 11, 2017

published 3.1.2 reverting this...

published 4.0.0 - doing this change

added a comment on the translate sample code for non-array call

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 11, 2017

http://i18next.com/translate/namespace/#basic does state i18next.t('key2'); // output: 'key2' (not found without prefix)

doing those fallbacks to other namespaces is not default behaviour and we do not encourage that...like said...using the saveMissing feature we would append to wrong namespace. Anyway if that would be primar thing we do...we could also completely remove namespaces and just use one big file...

but if you prefer this we could add an option on i18next.init eg. translateHOCMode: 'fallbackNS' to make the behaviour same as before?!?

@rosskevin

This comment has been minimized.

Copy link
Collaborator

@rosskevin rosskevin commented May 15, 2017

I understand the rationale of explicit namespacing, and that does make sense especially for diverse or complex components.

For brevity though, it is much more cumbersome. It has been our practice to order the namespaces in the array so that the most specific (index 0) would win in the case of collision. Our components are quite focused and small, our files quite specific, so it is not confusing to determine which namespace it originates from. In this way (with the fallback ns), we use localization much like CSS, in that the most specific wins.

I think an option to enable fallbacks without explicit namespacing has merit.

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 15, 2017

@rosskevin will add a flag asap to enable that behaviour again.

@jamuhl

This comment has been minimized.

Copy link
Member

@jamuhl jamuhl commented May 16, 2017

react-i18next@4.1.0

You can now translate(['ns1', 'ns2'], { nsMode: 'fallback' } to toggle that back to pre v4 behaviour (using passed namespaces as ordered fallbacks).

Or you can set that "globally" by i18next.init({ react: { nsMode: 'fallback' } });

@jamuhl jamuhl closed this May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.