search

jsmreese / moment-duration-format

Format function plugin for the Moment Duration object.
MIT License
969 stars 120 forks source link

ReferenceError: window is not defined (2.0.0) #81

Closed wubzz closed 6 years ago

wubzz commented 6 years ago

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 commented 6 years ago

Sigh. Thank you!

jsmreese commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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

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