poppinss / indicative

Indicative is a simple yet powerful data validator for Node.js and browsers. It makes it so simple to write async validations on nested set of data.
https://indicative.adonisjs.com/
MIT License
417 stars 52 forks source link

Replace `moment` with `date-fns` #141

Closed Andreyco closed 7 years ago

Andreyco commented 7 years ago

Notes

Andreyco commented 7 years ago

cc @thetutlage

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 4eaf7fea95f98caf7367c8f36e9c07a21329220b on Andreyco:date-fns into a533d9bed4201aef304c3b1956c3301d146760af on poppinss:develop.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 764ed4838b82f0efb7c19b4960db3aad7fd95441 on Andreyco:date-fns into a533d9bed4201aef304c3b1956c3301d146760af on poppinss:develop.

thetutlage commented 7 years ago

Have been going through the PR and the only breaking change I can see is that date_format will not working just with time anymore and always needs to have a date in front

Andreyco commented 7 years ago

@thetutlage yes, you are correct. Do you think watching for "time" format patterns and evaluating in different code path would be good solution? E.g.:

// Pseudocode
Raw.dateFormat = function (input, formats) {
  const formatsArray = Array.isArray(formats) ? formats : [formats]
  return formatsArray.some(pattern => {
    return isTimeFormat(pattern))
        ? isValidTime(parseTime(input)) && format(input, pattern) === input
        : isValidDate(parseDate(input)) && format(input, pattern) === input
  })
}

Of would you prefer creating new "time_format" rule and keep "date_format" strictly for date validation?

thetutlage commented 7 years ago

Nope, I think it's fine to keep time separate from date and better have a different rule for time.

I am going merge the PR but will wait for a while to release it.