Do not set default locale when a request is given as an argument #30

Merged
merged 3 commits into from Aug 8, 2012

Projects

None yet

4 participants

@dlongley

The current setLocale() call will set the "defaultLocale" variable even when a request is given as an argument; I believe this is a bug.

If "defaultLocale" is set when a request is given (the 2-argument form of setLocale), then there is an unfortunate side-effect where each subsequent request is treated as if it used the language from the previous one. For example, if the default locale is configured as "de" and a request with "Accept-Language: en" is sent to the server, then any subsequent request from any client that does not have an "Accept-Language" header will be assigned the language "en" instead of "de". Any request that sends an "Accept-Language" header will change the default locale from what was originally configured.

When a request is given as an argument to setLocale, only the locale on the request should be set. Only if no request is given should setLocale modify "defaultLocale".

This pull request also adds a couple of tests to ensure passing a request does not modify the default locale.

@mashpie
Owner

good point... but changes behavior and thus might break some implementations. let's rethink this a bit

@dlongley

Well, an alternative to this that might be a little less heavy-handed would be to just fix the internal handling of requests. So when guessLanguage() is called on a request, it doesn't call setLocale(), but duplicates what it does just for the request.

However, I don't think that anyone really wants their application's site-wide default locale to be changed every time a new request with a different "Accept-Language" comes in. I would imagine that people either haven't noticed that this is happening with their applications because their clients are all sending "Accept-Language" and don't see the side-effect themselves, or they did notice and are having to reset the default locale before every incoming request.

Neither of these is the correct way to solve the problem, IMO. I wouldn't expect many applications to be depending on this behavior.

@mashpie
Owner
msporny added some commits Aug 8, 2012
@msporny

Added a feature to turn off automatic locale file updating. This became an issue for us when we were pushing our application to production. In production mode, we package up the software and install it in a read-only location on the file system. We needed a way of disabling node-i18n's behavior of automatically trying to write i18n changes. The changes include:

  • Addition of a configuration parameter called updateFiles that defaults to true.
  • Updated the documentation to show people how to turn off automatic locale file updates.
@mashpie mashpie merged commit 4826b40 into mashpie:master Aug 8, 2012
@twoi

Any plans on creating a new version that includes the updateFiles feature?

In the meantime, I'll go the non-npm way and get the version from git.

@mashpie
Owner
@mashpie mashpie was assigned Mar 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment