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

Add fromNow #100

Closed
moeriki opened this issue Oct 16, 2017 · 14 comments
Closed

Add fromNow #100

moeriki opened this issue Oct 16, 2017 · 14 comments

Comments

@moeriki
Copy link

moeriki commented Oct 16, 2017

This is a very common pattern in my code. Not sure about how other people use it.

Date.now() + ms('2h')

I'd love to add fromNow as shortcut.

const inTwoHours = ms.fromNow('2h'); // 1508165682168 ;)

Yay or nay?

@mikemfleming
Copy link

mikemfleming commented Oct 17, 2017

I like the idea of that. I'll take a crack at it now.

Wouldn't want to mess with existing implementations that rely on exporting a function instead of an ms object with methods, though.

Maybe the call would look something more like ms('2h', { from: Date.now() });?

module.exports = function(val, options) {
  options = options || {};
  var type = typeof val;
  if (type === 'string' && val.length > 0) {
    return parse(val);
  } else if (type === 'number' && isNaN(val) === false) {
    return options.long ? fmtLong(val) : fmtShort(val);
  }
  throw new Error(
    'val is not a non-empty string or a valid number. val=' +
      JSON.stringify(val)
  );
};

@yoavmmn
Copy link
Contributor

yoavmmn commented Oct 24, 2017

+1 on the idea. And I totally agree with @mikemfleming, a from option will be easier and faster to implement.
I hope to sit and work on a Proof Of Concept in the next day or two.

@moeriki
Copy link
Author

moeriki commented Oct 25, 2017

{ from: … } is great as it allows users to get milliseconds relative to any date, not just now. 👍

@yoavmmn
Copy link
Contributor

yoavmmn commented Oct 25, 2017

I think I'm done. I'll write some tests and open a PR later today.

@yoavmmn
Copy link
Contributor

yoavmmn commented Oct 25, 2017

Opened PR #101, please review and let me know what you think.

@mikemfleming
Copy link

Awesome! I just checked it out locally with just the 2h example from above and it worked perfectly. Nice.

@jeteon
Copy link

jeteon commented Nov 19, 2017

I'm having trouble seeing the gain in going from:

Date.now() + ms('2h')

to:

ms('2h', { from: Date.now() })

What about making the above equivalent to this instead?:

ms('+2h')

@yoavmmn
Copy link
Contributor

yoavmmn commented Nov 19, 2017

@jeteon you gain readability. ms('2h', { from: Date.now() }); is more readable and understandable than Date.now() + ms('2h'); for begginers and newcomers.
The problem with ms('+2h'); is that it'll return only the time from Date.now();. using from option allows to get the time from any Date given.

@TooTallNate
Copy link
Member

But isn't Date.now() + ms('2h') nice and explicit already? Reads as "right now plus 2 hours". I'm not sure if an additional helper is necessary in this case.

@yoavmmn
Copy link
Contributor

yoavmmn commented Nov 28, 2017

@TooTallNate take a look at my PR. I think we all agree that a { from: ... } option is great for this and other use-cases.

@jeteon
Copy link

jeteon commented Nov 29, 2017

What are the other use cases?

@moeriki
Copy link
Author

moeriki commented Nov 29, 2017

In retrospect I agree the gain is rather small. I don't think this is a good idea anymore. I might make a ms wrapper package should I still be in need of something alike.

@jeteon
Copy link

jeteon commented Nov 29, 2017

I guess having a utility function like this would fix your use case?

function msAfter(duration) {
   return Date.now() + ms(duration);
}

@TooTallNate
Copy link
Member

We're going to pass on this feature request for now. Using idiomatic JavaScript (that is, + and - operators) is more explicit and readable, especially to somebody who is not already familiar with the semantics of this ms module.

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

5 participants