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

Coral Datetime may reference non-existent moment locale #128

Open
zbrownell opened this issue Aug 26, 2020 · 3 comments
Open

Coral Datetime may reference non-existent moment locale #128

zbrownell opened this issue Aug 26, 2020 · 3 comments

Comments

@zbrownell
Copy link
Contributor

zbrownell commented Aug 26, 2020

Expected Behavior

Correctly set locale to US English, if browser locale is set to this locale

Actual Behavior

Fails to set locale correctly, as moment version used does not contain a moment for the code used in many browsers (e.g. - Chrome uses en-us on a Macbook Pro, whereas moment refers to this locale as simply en)

Runtime error encountered: Error: Cannot find module './en-US'

Reproduce Scenario (including but not limited to)

Run your unit tests in debug mode with the option to "Pause on caught exceptions" selected? That's how I hit this situation when testing our project locally (which, includes latest Coral; 4.10.0)

Steps to Reproduce

(see above)

Browser name/version/os (ie Chrome Version 62.0.3202.94 (Official Build) (64-bit) MacOS)

Browser: Chrome Version 85.0.4183.83 (Official Build) (64-bit)
OS: (Mac) Version 10.15.6 (19G73)

Coral Spectrum version

This code (https://github.com/adobe/coral-spectrum/blob/master/coral-datetime/index.js) hasn't changed in 16 months and I'm not sure that moment ever supported an en-us locale, as their rationale for not including seems to amount to "that's our language, so we just call it English" (kind of disappointing)

Sample Code that illustrates the problem (use the Playground if possible)

n/a

Screenshots (if applicable)

IMG_20200826_131602

@zbrownell
Copy link
Contributor Author

Ideally, Moment would address on their side, as their whole purpose amounts to making it easier to work with date/time Objects across locales. However, since this has been brought up to them before (moment/moment#3569), I doubt they're going to address.

Instead, we could handle this logic in Coral Spectrum by changing from:

window.moment.locale(document.documentElement.lang || window.navigator.language || 'en');

...to:

let locale = document.documentElement.lang || window.navigator.language || 'en';
if (locale.toUpperCase() === 'EN-US') {
  locale = 'en';
}
window.moment.locale(locale);

@zbrownell
Copy link
Contributor Author

Also, this may not actually cause runtime problems (i.e. - fallback to default/en is likely, with some exceptions), but when stepping through to debug code, the exception will be thrown every time and you'll have to step over repeatedly.

@icaraps
Copy link
Contributor

icaraps commented Aug 27, 2020

@zbrownell Sounds good to me. Feel free to open the PR. I also would have expected Moment to handle this use case ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants