fix(date)!: Restore 'format', 'precison', and usage of `moment` for the date validator

adopted-ember-addons / ember-cp-validations

Ember computed property based validations

https://adopted-ember-addons.github.io/ember-cp-validations/

BSD 3-Clause "New" or "Revised" License

442 stars 174 forks source link

fix(date)!: Restore 'format', 'precison', and usage of `moment` for the date validator #726

Open tehhowch opened 2 years ago

tehhowch commented 2 years ago

Resolves https://github.com/adopted-ember-addons/ember-cp-validations/issues/723 .

Changes proposed:

reimplements the date validator that was removed in https://github.com/rwwagner90/ember-validators/pull/100/

I still think it would be better to support a consumer-defined "use luxon" or "use moment" or "use ___", or perhaps to propose an interface in which the consuming app can massage their preferred date library to support determining the before / onOrBefore / after / onOrAfter relationship states.

restores the test suite semantics from before https://github.com/adopted-ember-addons/ember-cp-validations/pull/715 (git diff d6c95d741fa8388ae2c764037bddcd6c91691f09 tests/unit/validators/date-test.js), while adding some additional tests / ensuring the test assertion messages help convey the test intent.

Tasks:

[x] Added test case(s)
[x] Updated documentation

cc: @fsmanuel for visibility

tehhowch commented 2 years ago

It's worth noting that this is a breaking change to only those on 4.0.0-beta.13 or v4 that modified their usage of the date validator to be compatible with the version provided in ember-cp-validations.

Anyone who is still on 4.0.0-beta.12 or lower would probably consider this to remove a breaking change that is currently in v4.

kiwi-josh commented 2 years ago

I still think it would be better to support a consumer-defined "use luxon" or "use moment"

@tehhowch I very much agree with this.

We are currently running the https://github.com/qonto/ember-cp-validations/commit/3767e4114e0c327138287486094e228bd71a45ec in production (as we made efforts to upgrade to ember 4 prior to this addon being adopted).

We have also recently replaced momentjs with dayjs (saving ~25kB's of payload size).

Therefore this PR would be a step backwards for us getting back onto the official repo, as we wouldn't want the extra kB's being reintroduced.

I also completely understand that this PR would undo some unexpected breaking changes for consuming apps that rely on now / precision etc, which I'm all for - this was more a "I agree with your suggestion to let the consumer decide"

fsmanuel commented 2 years ago

@tehhowch I'm on holiday and will have a look after I'm back. I'm still undecided how to move forward with the date validator. I think the addon should not diverge/reimplement too much from ember-validators. Maybe we can provide the old implementation (from this PR) in the docs/readme/migration guide. That way people can use it if they want.

I also agree that a more agnostic solution would be great. Next to moment and dayjs there is also date-fns