search

tolu / ISO8601-duration

Node/Js-module for parsing and making sense of ISO8601-durations
92 stars 10 forks source link

feat: add toMilliseconds method #37

Closed mariusaarsnes closed 1 month ago

mariusaarsnes commented 2 months ago

Create a method for converting duration object to milliseconds.

The new toMilliseconds method uses the already existing toSeconds method, but also multiplies by 1000 to convert it to milliseconds instead of seconds.

tolu commented 1 month ago

Thanks for the well put-together pull request Marius 🙏

However, I do feel that this functionality is so trivial that it fits better in consuming code rather than in this module, even if it is a small change. I want to keep the module as light as can be with as few updates as possible to spare people the hassle of getting burdened by npm-outdated warnings etc.

And as I've said in the readme, Temporal is the new standard that is about to land (currently in Stage 3 in TC39) so I'd actually suggest using that polyfill to be more future-proof rather than relying on this module that is most likely not going to change.

I do accept arguments for why I'm likely wrong though, in fact I'll be glad to hear your reasoning why 😊

tolu commented 1 month ago

And please say hello for me to any of my old colleagues at Netligt (I left in 2015) 👋

mariusaarsnes commented 1 month ago

Ok, no worries! I just saw that inSeconds from this package is used in one of our projects and wanted a toMilliseconds method instead since we're working with the built in Date object. We're currently just doing the same thing as this PR suggests so no big deal. I'll look into Temporal!

And please say hello for me to any of my old colleagues at Netligt (I left in 2015) 👋

Joel says hi back!