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

intl: configure default locale #28099

Closed
bnoordhuis opened this issue Jun 6, 2019 · 6 comments
Closed

intl: configure default locale #28099

bnoordhuis opened this issue Jun 6, 2019 · 6 comments
Labels
feature request Issues that request new features to be added to Node.js. i18n-api Issues and PRs related to the i18n implementation. semver-major PRs that contain breaking changes and should be released in the next major version. stale

Comments

@bnoordhuis
Copy link
Member

Right now Node.js (by omission) leaves it up to ICU to figure out the locale and configure itself accordingly.

That's problematic for a number of reasons, see e.g. #27856 (comment) - tl;dr environment variables affect the output of Date#toString(). Googling for variations of 'nodejs lc_all' suggests the issue pops up more often.

Since one of Node.js's goals is to provide a consistent experience across platforms and environments, I think it would be good to explicitly configure the locale at startup instead of leaving it to happenstance.

There are two ways to accomplish that and they're not mutually exclusive:

  1. setenv("LC_ALL", "..."); setenv("LC_MESSAGES", "..."); // etc.
  2. icu::Locale::setDefault(...)

I'm adding the semver-major label because there is some risk of breaking existing applications. (Libraries not so much since the average library already runs in a variety of environments.)

It would be good to have some discussion on the merits of both (and alternative?) approaches.

@bnoordhuis bnoordhuis added semver-major PRs that contain breaking changes and should be released in the next major version. i18n-api Issues and PRs related to the i18n implementation. labels Jun 6, 2019
@sam-github
Copy link
Contributor

@nodejs/intl

@silverwind
Copy link
Contributor

silverwind commented Jun 7, 2019

Agree, things like Date#toString output must be stable. The variants that support locale like Date#toLocale​String should of course factor in locale settings.

@silverwind
Copy link
Contributor

Would setenv affect other libraries like V8? If so, the icu APIs are probably safer to use.

@bnoordhuis
Copy link
Member Author

Other libraries: yes. V8: probably not since it uses ICU.

The "other libraries" part is arguably a pre because you want different parts of Node.js to agree on the locale.

FWIW, the ICU-only patch for people who want to try it:

diff --git a/src/node.cc b/src/node.cc
index e168d62c51..08826678bb 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -93,6 +93,7 @@
 #include <sys/types.h>
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
+#include <unicode/locid.h>
 #include <unicode/uvernum.h>
 #endif
 
@@ -883,6 +884,13 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) {
   // Hack around with the argv pointer. Used for process.title = "blah".
   argv = uv_setup_args(argc, argv);
 
+#ifdef NODE_HAVE_I18N_SUPPORT
+  {
+    UErrorCode status = U_ZERO_ERROR;
+    icu::Locale::setDefault(icu::Locale::getUS(), status);
+  }
+#endif  // NODE_HAVE_I18N_SUPPORT
+
   InitializationResult result;
   result.args = std::vector<std::string>(argv, argv + argc);
   std::vector<std::string> errors;
(The US-centricity makes it kind of blergh but one thing at a time.)

@jasnell jasnell added the feature request Issues that request new features to be added to Node.js. label Jun 19, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Mar 8, 2022
@targos targos moved this from Pending Triage to Stale in Node.js feature requests Mar 8, 2022
@targos targos moved this to Pending Triage in Node.js feature requests Mar 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2022

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. i18n-api Issues and PRs related to the i18n implementation. semver-major PRs that contain breaking changes and should be released in the next major version. stale
Projects
None yet
Development

No branches or pull requests

4 participants