feat(isBefore): allow usage of options object

[pixelbucket-dev]

This PR implements steps 1 and 2 of #1874 for isBefore and builds upon #2075.

This PR extracts tests for isBefore into a separate test file ⇾ test/validators/isBefore.test.js (inspired by #1793).

This PR also renames the proposed date option to comparisonDate, because it is more explicit. If verified, it should be implemented in #2075 as well.

Checklist

• PR contains only changes related; no stray files, etc.
• README updated (where applicable)
• Tests written (where applicable)

[codecov]

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (18d652b) compared to base (54d330c).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 18d652b differs from pull request most recent head 57d8945. Consider uploading reports for the commit 57d8945 to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #2088   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          105       105           
  Lines         2324      2323    -1     
  Branches       586       586           
=========================================
- Hits          2324      2323    -1     

Impacted Files	Coverage Δ
src/lib/isAfter.js	100.00% <100.00%> (ø)
src/lib/isBefore.js	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[WikiRik]

Few remarks, thanks for working on this!

[WikiRik]

Few final comments so that this is consistent with my changes to isAfter and the project in general, but apart from these we should be good I think

[pixelbucket-dev]

I think this should be good to go :).

[WikiRik]

Few remarks to be more in line with how isAfter was done, but those should be my last

[WikiRik]

Suggested change

	// accessing 'arguments' for backwards compatibility: isBefore(str [, comparisonDate])
	// eslint-disable-next-line prefer-rest-params
	const comparisonDate = (typeof options === 'object' ? options.comparisonDate : arguments[1]) || Date().toString();
	// For backwards compatibility:
	// isBefore(str [, date]), i.e. `options` could be used as argument for the legacy `date`
	const comparisonDate = options?.comparisonDate || options || Date().toString();

Would this work as well? Copied from isAfter

[pixelbucket-dev]

Nope, that would be a bug :(.

If comparisonDate is undefined, the ternary operator will therefore evaluate to options, which would be { comparisonDate: undefined }. The latter will break when using inside toDate, because it is not a string.

Well spotted anyway. This way I realised that I wanted to add tests to cover this scenario.

[WikiRik]

That's the case in isAfter as well right? So we can fix it for both validators in this PR I think

[pixelbucket-dev]

Yes, that would be the case. I will add a fix.

[pixelbucket-dev]

I've fixed isAfter and added tests. FYI, the last test would fail with the old way in isAfter.

[WikiRik]

I didn't do it for isAfter but maybe we can split this up in two it blocks? One without arguments (or at least that don't use the old nor new syntax like []) and one with arguments. This way it's easier to see what the changes are of the two syntaxes and therefore the scenarios that we need to replicate
Just a new idea, so I'm open for feedback and other ideas

[pixelbucket-dev]

Sorry, I am not sure what you mean. Could you give an example with the new syntax?

[WikiRik]

The new syntax is using the comparisonDate in the object.

So we would have;

describe('isBefore'
   it('should validate dates against the current date'
     test({
       validator: 'isBefore',
       valid: ,
       invalid:
     })
     test({
       validator: 'isBefore',
       args: [],
       valid: ,
       invalid:
      })
   it('should validate dates against an end date'
     test({
       validator: 'isBefore',
       args: [{ comparisonDate: ],
       valid: ,
       invalid:
      })
   describe('(legacy syntax)'
     it('should validate dates against an end date'
       test({
         validator: 'isBefore',
         args: ['08/04/2011'],
         valid: ,
         invalid:
        })

Is that clear?

[pixelbucket-dev]

I've just pushed a proposed refactor to the tests. Let me know what you think :). Here you can also see how important it is to write test cases for all possible scenarios and permutations, ideally upfront, before implementation. That would have made it easier to refactor the implementation to the new syntax. The existing tests probably still lack possible values or permutations. We should probably also test against other nullish values. E.g. should we guard against null? Because the code will break if options === null.