Skip to content

Conversation

@WilcoSp
Copy link
Contributor

@WilcoSp WilcoSp commented May 12, 2024

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 create new Date().

currently this draft has only isAfter, isBefore and diffMilliseconds to 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 how

current progress (ts, tests, jsdoc):

  • is*
  • diff*
  • add*
  • same*
  • *end
  • *start
  • nearestDay
  • monthDays
  • dayOfYear
  • yearDays
  • offset
  • fill
  • createPartMap
  • removeOffset
  • tzDate

added:

  • isPast
  • isFuture

(the only part that only needs to be changed still is the website docs)

@vercel
Copy link

vercel bot commented May 12, 2024

@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
Copy link
Contributor

@ghiscoding ghiscoding May 16, 2024

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 given

Copy link
Contributor Author

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

@justin-schroeder
Copy link
Member

@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.

@WilcoSp
Copy link
Contributor Author

WilcoSp commented Jun 30, 2024

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.

@WilcoSp WilcoSp changed the title (discussion) proposal pr: optional arguments for functions feature: optional date arguments for functions Jul 31, 2024
@WilcoSp WilcoSp marked this pull request as ready for review July 31, 2024 09:43
@WilcoSp
Copy link
Contributor Author

WilcoSp commented Jul 31, 2024

@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 isPast & isFuture as I think it would make the code more understandable than isAfter(date) or isBefore(date).

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.

@justin-schroeder justin-schroeder merged commit a748d5b into formkit:main Aug 21, 2024
@justin-schroeder
Copy link
Member

Thanks @WilcoSp!

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

Successfully merging this pull request may close these issues.

3 participants