Repository: moment/luxon
Pull request #1354 (2022-12-29 18:21:32)

preserve language tags
Fixes: https://github.com/moment/luxon/issues/1335

linux-foundation-easycla[bot]:  <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19772872/47118129/1354/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328"></a><br/><br /><ul><li><a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19772872/47118129/1354/#/?version=2' target='_blank'>:x:</a> - login: @devsnek / name: snek . The commit (3511882f4b57f30e334f800a99e8a8dabc14e296) is not authorized under a signed CLA. <a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19772872/47118129/1354/#/?version=2' target='_blank'>Please click here to be authorized</a>. For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul>
diesieben07: This is unfortunately not correct when the language tag contains a private use subtag (`-x-`). For example `en-US-x-twain` must turn into `en-US-u-nu-hanidec-x-twain` when combined with `nu-hanidec`. Your code would turn it into `en-US-x-twain-u-nu-hanidec`, which has a different meaning.
devsnek: @diesieben07 do you have any suggestion for what should be done? Does this need to support private use subtags? 
diesieben07: > do you have any suggestion for what should be done?

You could either detect the `-x-` and insert the `-u-` at the appropriate position, but I think this would require more extensive changes of the code.
Alternatively you could strip out any `-x-` before appending (or throw an error), because I don't think the browser cares about any `-x-` tags. The problem with the current approach is that there won't be any error, the appended `-u-` will just be ignored by the browser.

> Does this need to support private use subtags?
Probably not. But currently this code is a regression, because previously they were just ignored and now they cause the `outputCalendar` and `numberingSystem` to stop working.
icambron: Looks good. Thanks!

Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index b01cdc70..d1eeb7c7 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -15,7 +15,7 @@ Here are some vague notes on Luxon's design philosophy:
 
 ## Building and testing
 
-Building and testing is done through npm scripts. The tests run in Node and require Node 10+ with full-icu support. This is because some of the features available in Luxon (like internationalization and time zones) need that stuff and we test it all. On any platform, if you have Node 10 installed with full-icu, you're good to go; just run npm scripts like `npm run test`. But you probably don't have that, so read on.
+Building and testing is done through npm scripts. The tests run in Node and require Node 18 with full-icu support. This is because some of the features available in Luxon (like internationalization and time zones) need that stuff and we test it all. On any platform, if you have Node 18 installed with full-icu, you're good to go; just run `scripts/test`. But you probably don't have that, so read on.
 
 ### OSX
 
diff --git a/src/impl/locale.js b/src/impl/locale.js
index d3cb3474..eb8c2bea 100644
--- a/src/impl/locale.js
+++ b/src/impl/locale.js
@@ -70,27 +70,39 @@ function parseLocaleString(localeStr) {
   // b) if it does, use Intl to resolve everything
   // c) if Intl fails, try again without the -u
 
+  // private subtags and unicode subtags have ordering requirements,
+  // and we're not properly parsing this, so just strip out the
+  // private ones if they exist.
+  const xIndex = localeStr.indexOf("-x-");
+  if (xIndex !== -1) {
+    localeStr = localeStr.substring(0, xIndex);
+  }
+
   const uIndex = localeStr.indexOf("-u-");
   if (uIndex === -1) {
     return [localeStr];
   } else {
     let options;
-    const smaller = localeStr.substring(0, uIndex);
+    let selectedStr;
     try {
       options = getCachedDTF(localeStr).resolvedOptions();
+      selectedStr = localeStr;
     } catch (e) {
+      const smaller = localeStr.substring(0, uIndex);
       options = getCachedDTF(smaller).resolvedOptions();
+      selectedStr = smaller;
     }
 
     const { numberingSystem, calendar } = options;
-    // return the smaller one so that we can append the calendar and numbering overrides to it
-    return [smaller, numberingSystem, calendar];
+    return [selectedStr, numberingSystem, calendar];
   }
 }
 
 function intlConfigString(localeStr, numberingSystem, outputCalendar) {
   if (outputCalendar || numberingSystem) {
-    localeStr += "-u";
+    if (!localeStr.includes("-u-")) {
+      localeStr += "-u";
+    }
 
     if (outputCalendar) {
       localeStr += `-ca-${outputCalendar}`;


Test patch:
diff --git a/test/datetime/create.test.js b/test/datetime/create.test.js
index 446ad9e6..016266a6 100644
--- a/test/datetime/create.test.js
+++ b/test/datetime/create.test.js
@@ -663,7 +663,7 @@ test("DateTime.fromObject accepts a locale", () => {
 
 test("DateTime.fromObject accepts a locale with calendar and numbering identifiers", () => {
   const res = DateTime.fromObject({}, { locale: "be-u-ca-coptic-nu-mong" });
-  expect(res.locale).toBe("be");
+  expect(res.locale).toBe("be-u-ca-coptic-nu-mong");
   expect(res.outputCalendar).toBe("coptic");
   expect(res.numberingSystem).toBe("mong");
 });
@@ -677,7 +677,7 @@ test("DateTime.fromObject accepts a locale string with weird junk in it", () =>
       }
     );
 
-    expect(res.locale).toBe("be");
+    expect(res.locale).toBe("be-u-ca-coptic-ca-islamic");
 
     // "coptic" is right, but some versions of Node 10 give "gregory"
     expect(res.outputCalendar === "gregory" || res.outputCalendar === "coptic").toBe(true);
@@ -695,7 +695,7 @@ test("DateTime.fromObject overrides the locale string with explicit settings", (
     }
   );
 
-  expect(res.locale).toBe("be");
+  expect(res.locale).toBe("be-u-ca-coptic-nu-mong");
   expect(res.outputCalendar).toBe("islamic");
   expect(res.numberingSystem).toBe("thai");
 });
@@ -811,3 +811,18 @@ test("DateTime.fromObject takes a undefined to mean {}", () => {
   const res = DateTime.fromObject();
   expect(res.year).toBe(new Date().getFullYear());
 });
+
+test("private language subtags don't break unicode subtags", () => {
+  const res = DateTime.fromObject(
+    {},
+    {
+      locale: "be-u-ca-coptic-nu-mong-x-twain",
+      numberingSystem: "thai",
+      outputCalendar: "islamic",
+    }
+  );
+
+  expect(res.locale).toBe("be-u-ca-coptic-nu-mong");
+  expect(res.outputCalendar).toBe("islamic");
+  expect(res.numberingSystem).toBe("thai");
+});
diff --git a/test/datetime/format.test.js b/test/datetime/format.test.js
index 823dce35..23dcfd77 100644
--- a/test/datetime/format.test.js
+++ b/test/datetime/format.test.js
@@ -415,6 +415,12 @@ test("DateTime#toLocaleString uses locale-appropriate time formats", () => {
   expect(dt.reconfigure({ locale: "es" }).toLocaleString(DateTime.TIME_24_SIMPLE)).toBe("9:23");
 });
 
+test("DateTime#toLocaleString() respects language tags", () => {
+  expect(dt.reconfigure({ locale: "en-US-u-hc-h23" }).toLocaleString(DateTime.TIME_SIMPLE)).toBe(
+    "09:23"
+  );
+});
+
 test("DateTime#toLocaleString() accepts a zone even when the zone is set", () => {
   expect(
     dt.toLocaleString({