-
-
Notifications
You must be signed in to change notification settings - Fork 35
feature: optional date arguments for functions #58
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
feature: optional date arguments for functions #58
Conversation
…llow comparison with `now`
|
@WilcoSp is attempting to deploy a commit to the Formkit Team on Vercel. A member of the Team first needs to authorize it. |
src/isBefore.ts
Outdated
| * | ||
| * @param inputDate - The date that should be before the other one to return true | ||
| * @param dateToCompare - The date to compare with | ||
| * @param dateToCompare - The date to compare with or `now` if nothing given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in JSDocs, an optional param should be wrapped in square bracket (see JSDocs). Also to be really aligned with JSDocs, the type should also be included and come before the param name, so it should be something like
* @param {DateInput} inputDate - The date that should be before the other one to return true
* @param {MaybeDateInput} [dateToCompare] - The date to compare with or `now` if nothing givenThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah we could at use square brackets to indicate a param is optional. although we don't need to add types because we use typescript to provide the types
|
@WilcoSp sorry for the delay. I think this looks great. If you flesh this out for the rest of the fns I’ll gladly merge it. |
|
hey @justin-schroeder it's not an issue, I was also busy the past month. When I've sometime (most likely on Wednesdays) I'll bring optional date arguments to other functions. |
changed isEqual to accept 1 optional date
tests for them are combined with the ones for diff
…al date input added 'the' before current * were missing
|
@justin-schroeder all the functions where possible have given been optional date arguments (check top post for which ones have been changed). I've also added the only thing that needs to be still changed are the website docs for each function, I've looked a bit into how the website docs are made and if you want I could also change those if wanted or if you want you can change them. |
|
Thanks @WilcoSp! |
based on issue #56
is*,diff*and most other functions that use the date function could 1 or more arguments be made optional to allow using the current time (now) without needing to createnew Date().currently this draft has only
isAfter,isBeforeanddiffMillisecondsto give an example how it could look. I would like to first discuss which other functions could allow for optional arguments to use the current time ('now') and howcurrent progress (ts, tests, jsdoc):
added:
(the only part that only needs to be changed still is the website docs)