fix: accept dates as params for Date methods

[pkuczynski]

Fixes #198

[netlify]

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: c03bd39

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61eb4621ab6f360008224c99

😎 Browse the preview: https://deploy-preview-200--vigilant-wescoff-04e480.netlify.app

[Shinigami92]

Due to this is a runtime change, I think we will do this in v6.1, because we try to focus on stability of the project itself first (CI, tests, docs etc)

[pkuczynski]

Due to this is a runtime change, I think we will do this in v6.1, because we try to focus on stability of the project itself first (CI, tests, docs etc)

I only got to this change, because my current code start failing after the upgrade. So this is not strictly a new feature, but restoring functionality working with previous version of faker.

[Shinigami92]

Speaking about unit tests, could you add one?

[pkuczynski]

Speaking about unit tests, could you add one?

Done. If you would be using jest I could do it.each to run the same tests with different examples. Without it, I just had to duplicate some tests

[pkuczynski]

Btw. good that you asked about tests, as I discovered a bug, where original date was incorrectly mutated...

[LawrenceWooding]

I only got to this change, because my current code start failing after the upgrade. So this is not strictly a new feature, but restoring functionality working with previous version of faker.

I had a similar experience to @pkuczynski - attempting to replace the old faker-js package with this new one caused errors in previously working code. I am confident that most (if not all) of the Date functions used to support Date() fields. This is supported by the fact that in the documentation for Dates all of the functions take Date objects instead of strings (or at least the ones that were already documented).

Speaking of which the docs should probably be updated in this branch to better reflect the type definitions (e.g string | Date as opposed to Date). I am happy to do this if I get a spare moment.

Thank you @pkuczynski for looking into this issue, it is the primary roadblock between my organisation switching our frontend repository to the community faker-js package.

[LawrenceWooding]

I have made a pull request in @pkuczynski 's fork with the updated documentation. :)

[pkuczynski]

I have made a pull request in @pkuczynski 's fork with the updated documentation. :)

And I just merged it, thanks :)

[pkuczynski]

@Shinigami92 Is there anything else you want me to do to get this PR accepted, merged and released?

[Shinigami92]

Done. If you would be using jest I could do it.each to run the same tests with different examples. Without it, I just had to duplicate some tests

You will love our next PR incoming :P
See #235

[pkuczynski]

You will love our next PR incoming :P See #235

Citing their docs: "It's not recommended to migrate your current testing setups yet" ;) So I am not sure :) Is it jest based?

[Shinigami92]

Citing their docs: "It's not recommended to migrate your current testing setups yet" ;) So I am not sure :) Is it jest based?

I'm one of Vite core members and in direct contact with Anthony Fu 🙂
Also yes, vitest is a somewhat swap-in-out replacement for jest. But written in more modern syntax and stripped out old legacy stuff. So it's jest but in fast ^^

[pkuczynski]

@Shinigami92 rebased and feedback applied. Let me know if anything is still missing?

[pkuczynski]

Added missing jsdoc to these new functions, after you converted them from arrow. Anything else @Shinigami92?

[Shinigami92]

Anything else @Shinigami92?

I'm in bed now ^^
Will look tomorrow, thx for the JSDocs

[codecov]

Codecov Report

Merging #200 (7632415) into main (825a8c8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 7632415 differs from pull request most recent head 720a35d. Consider uploading reports for the commit 720a35d to get more accurate results

@@           Coverage Diff           @@
##             main     #200   +/-   ##
=======================================
  Coverage   99.33%   99.33%           
=======================================
  Files        1923     1923           
  Lines      176857   176860    +3     
  Branches      908      913    +5     
=======================================
+ Hits       175687   175690    +3     
  Misses       1114     1114           
  Partials       56       56           

Impacted Files	Coverage Δ
src/helpers.ts	99.12% <ø> (-0.01%)	⬇️
src/date.ts	100.00% <100.00%> (ø)
src/vendor/unique.ts	93.54% <0.00%> (-1.62%)	⬇️
src/vendor/user-agent.ts	98.87% <0.00%> (+0.56%)	⬆️