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

ReferenceError: window is not defined (2.0.0) #81

Closed
wubzz opened this issue Dec 14, 2017 · 5 comments
Closed

ReferenceError: window is not defined (2.0.0) #81

wubzz opened this issue Dec 14, 2017 · 5 comments

Comments

@wubzz
Copy link

wubzz commented Dec 14, 2017

As specified in the release notes the library now uses userLocale and it defaults to properties on the window object. This naturally throws an error in Node environment:

ReferenceError: window is not defined

While this (at least in my case) is not an issue as I can just submit it through the options object, I still think it should be made clear in usage examples that userLocale must be provided when using Node.

@jsmreese
Copy link
Owner

Sigh. Thank you!

@jsmreese
Copy link
Owner

@wubzz are there localization modules you commonly use? Like locale?

Or would it be better to parse process.env.LANG directly?
http://man7.org/linux/man-pages/man7/environ.7.html

It make sense to default to en if nothing is available.
I'm obviously haven't used node in years as I totally forgot to test there...

@wubzz
Copy link
Author

wubzz commented Dec 14, 2017

@jsmreese I think just overall that relying on moment.locale() getter would make most sense. By default it's en. If the user has imported momentjs with locale support, then generally one would configure moment.locale('sv') prior to using moment-duration-format.

One issue with the current default-value implementation for browser based applications is that the lib will yield different results from the same input data depending on the visitors browser settings, since window.navigator.language is normally the selected language of the browser. Edit: Unless that was the intention all along?

@jsmreese
Copy link
Owner

jsmreese commented Dec 14, 2017

@wubzz thanks for the input.

I had a reason for using the browser's locale a few weeks ago, something to do with the way toLocaleString was behaving with a locale of en... but after exploring the config options for toLocaleString again just now, I don't see anything amiss. Most of my work on the new version has been in the middle of the night bouncing a baby who wouldn't stay asleep, so I'll chalk this up to that.

I'll change the default to point to moment's locale and get a new version published shortly!

@jsmreese
Copy link
Owner

https://github.com/jsmreese/moment-duration-format/releases/tag/2.0.1

Please let me know if you see any further issues in Node!

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