Locale should be returned if locale is default

[Bramzor]

Possible fix for issue #15 , where locale was undefined in case the default value was not part of the allowed array. Now it does also check if locale is default value if default is set.

Use case that is not covered by this pull request is when default value is not set (so should default to 'en_GB' as per default.js) and missing from allowed.

[smhg]

Thank you for this PR. This clarifies your issue.

I don't think it is a good idea to change this however.
By default, there is no whitelist (allowed property). Once you start using a whitelist, it sounds good (to me) to explicitly list all locales your application allows.
Once you magically add locales to this whitelist it loses that explicit nature. Also, you could argue we should add the locales listed in the hostname or map lookups too in that case.

Please feel free to disagree. I'm always open to discussion and reopen.

[Bramzor]

But does it make sense that it returns undefined once you have a default value and using that default value, even though it is missing from allowed? I think it should NEVER return undefined as this is an unexpected value. It took me some time to troubleshoot this while in fact it is an easy fix. In fact in my opinion it is a no brainer that the default is always allowed by default.

In my defense, you cannot expect the user from doing everything right at once so it can help to just accept these stupid mistakes.

If you really do not want to (fine by me), it might also be an idea to just add a warning message that default is missing in allowed. In that case, there is no "magic" solving it but the user is still notified and can fix it. The only problem left in that case is that the server still receives an undefined instead of an object....

[smhg]

Locale detection has such broad applications that it is hard to assume defaults. It will (should) probably always be instantiated with a custom configuration based on the projects' requirements. When you assume it to fill in some gaps, these gaps are exactly what other's build on.

I think it should NEVER return undefined as this is an unexpected value.

undefined can very much be an expected value. e.g. in projects only supporting a fixed list of locales and redirecting all others to some your region/language is not supported page. undefined represents express-locale being unable to detect a locale.

I definitely support the idea of showing a warning in non-production environments when 'conflicting' configuration options are detected. This should also apply to map and hostname.

Would you want create a new PR for this?

[smhg]

Sidenote: the current default config might indeed not be the most optimal.

{
  "priority": ["accept-language", "default"],
  "default": "en_GB"
}

When you only add the allowed whitelist, you are immediately faced with your scenario.
I'm reluctant to change this at this point, but in the end no default config might be better.

[Bramzor]

Actually when I noticed that I was receiving undefined while I did set a default value, I though the middleware plugin was broken as I was expecting to see at least a return of the default value. I think there is no use case where you could return an undefined while a default is set. But yes, it is one of those use cases maybe not many people ever notice. I actually switched from npm locale to yours because I wanted to get locale and language information. That is probably why my initial config was not set correctly from the first time. But the undefined almost made me give up the plugin while when I see the code, it actually is pretty nice.