Wrong param and return types on `time.recent`

[xDivisionByZerox]

We will not add a default case. Instead we adjust the method signature to allow for unkown string params and return dates in v6.0.

recent(format: LiteralUnion<'abbr' | 'wide' | 'unix' | 'date'> = 'unix'): string | number | Date { ... }

Previous description

The switch statement in time.recent is missing the a default switch case.

https://github.com/faker-js/faker/blob/1058e17ebb24207a5aad959b5613cc2d1de849d7/src/time.ts#L21-L31

FFR: Documented by @Shinigami92

[xDivisionByZerox]

This issue could use the help wanted and good first issue labels since it really easy and gives people who what to help an easy entrance into the project.

[ehtishama]

@xDivisionByZerox @Shinigami92 What should be the expected behaviour when an invalid format value is passed, like faker.time.recent("xyz"), should the date get default UNIX format? or " format invalid "?

I am new to open source contribution and want to work on this issue.

[xDivisionByZerox]

I am sure the reason why we wanted the default cause in the switch statement was to ensure that any invalid parameters will be handled as unix input.

[xDivisionByZerox]

But @Shinigami92 should approve this statement.

[Shinigami92]

But @Shinigami92 should approve this statement.

https://github.com/faker-js/faker/pull/534#discussion_r811795638

... yes :slightly_smiling_face:

[fzn0x]

Thanks for anyone contributing to the discussion :)

[ST-DDT]

The maintainers will discuss this internally.

[ST-DDT]

Decision: We will not add a default case. Instead we adjust the method signature to allow for unkown string params and return dates in v6.0.

recent(format: LiteralUnion<'abbr' | 'wide' | 'unix' | 'date'> = 'unix'): string | number | Date { ... }

In v6.1 we deprecate the method for removal. In v7.0 we remove the method entirely.

[fzn0x]

Unfortunately there is something urgent in my company to be done with. If anybody can work on this fast, i would likely for anybody to takeover the PR. Instead making the PR be abandon till next week when I'm free, I won't take this as mine-only assignment, so.. yeah help wanted.

[Shinigami92]

This PR will not get merged within the next two weeks! Also if you need the default case, please use new Date().getTime() for now.

The whole recent is just a wrapper of some Date instance methods called on a new Date() We even plan to get rid of this: https://github.com/faker-js/faker/issues/557

[fzn0x]

This PR will not get merged within the next two weeks! Also if you need the default case, please use new Date().getTime() for now.

The whole recent is just a wrapper of some Date instance methods called on a new Date() We even plan to get rid of this: #557

Cool, thanks for the information.