i18next / i18next-browser-languageDetector

language detector used in browser environment for i18next
MIT License
874 stars 90 forks source link

Option to not cache when the value comes from navigator or query #187

Closed Spiralis closed 5 years ago

Spiralis commented 5 years ago

I suggest that an option is added to control when the detected language is cached and not.

I.e. my users run the app, and the app detects their chosen language and uses that. If they change their browser lang-preference then the expected thing for them is that the application chooses their new language (given that it exists of course). Also, lets say a user opens the application, but their chosen language is not yet available. The detection mechanism might fallback to English. But, then the developer add their locale to the list of localisations, so when the user opens the application the expected behaviour is to now get the new language. However, the way it currently works now, the user will be "stuck" with the locale that they originally got when the application was opened. And, they will have to manually choose a new language, given that the developer has not created a way for them to select.

I saw that #186 referenced partly the same issue above.

Actually, not caching for query and navigator is what I would consider the "auto" behaviour. Only use localStorage, when the user has manually selected a value.

Workaround: What I do know is to set caches: [] in the detection-config, plus order: ['localStorage', 'navigator', 'htmlTag'],. I then provide the user with an option to change the language. When the user do change the language I manually do a window.localStorage.setItem('i18nextLng', newLang) call in addition to the i18next.changeLanguage(newLocale) call. You could also add a "default" language, or a way to unset it, bu having a separate choice that unsets the value from localStorage, but which requires the user to re-load the page.

Maybe this could be done better in this plugin directly? The default would be that if navigator and query doesn't cache then we are off to a good start. Then, maybe an extension method to do set it, and finally a way of clearing it, while re-detecting the value (instead of reload). I don't know the codebase so I am not sure if this would be hard or not. Just a thought...

jamuhl commented 5 years ago

Like you said:

Actually, not caching for query and navigator is what I would consider the "auto" behaviour.

This is ok like it is for thousands of user - so do not expect us to change that behaviour breaking.

You can create your own language detection plugin with ease: https://www.i18next.com/misc/creating-own-plugins#languagedetector

Spiralis commented 5 years ago

Oh, sorry for suggesting anything at all. Not really the feedback that gets anyone interested in contributing, TBH. I was actually thinking about spending time on cloning and seeing if I could contribute a PR, but, nevermind...

jamuhl commented 5 years ago

Look...If I sounded rude...that wasn't the intention.

186 is not the same...that behaviour described their does not happen...cache not set it will not store and loading order is always respected...just look the code:

so #186 is not reproducible - cleaning localStorage, changing the settings like he showed could not result in what he described...

So now to your issue:

Actually, not caching for query and navigator is what I would consider the "auto" behaviour. Only use localStorage, when the user has manually selected a value.

So you suggest not saving (using the caches set) if detection comes from query or navigator:

Ok so let's say we in this case only change behaviour for navigator - as that is always set.

So next problem...https://github.com/i18next/i18next-browser-languageDetector/blob/master/src/index.js#L89 gets called on languageChange in i18next -> so at the point this gets called you do not know if the lng comes from the detection and even less from what detector...

Ok so we save the detector and lng detected on detection and check on cache if it was detected from navigator and lng to be cached is the same...if so we do not cache.

Now for cookie we still have to cache as the cookie is used to sync language between client and server...

ok so we only do this for localStorage...as users might read from localStorage (yes they do) we have to do this conditionally by a config option - to being backwards compatible we opt out per default...

So to summarize:

So this is the long version which might more explain why I'm not yet convinced this is a regular use case that happens often bringing significant improvement justifying the added complexity.

Spiralis commented 5 years ago

Thanks for the clarification.

I see what you mean about the problem with languageChange. I don't think that opening the app with a query should be cached, but obviously - that is what the config is for, so fair enough.

What I have done in my app works fine, and may be the best practice for my/this scenario (instead of codebase changes in this library):

Thanks again for following up on this.