Add a ParseDatePipe to the common library

[davidgonmar]

Is there an existing issue that is already proposing this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe it

Currently, NestJS does not provide a ParseDatePipe in the common library.

Describe the solution you'd like

Implementing a ParseDatePipe. I am open to opening a PR with the changes!

Teachability, documentation, adoption, migration strategy

Including the new pipe in https://docs.nestjs.com/pipes#built-in-pipes.

What is the motivation / use case for changing the behavior?

I think it is common enough to want to parse dates, so including it in the common library can save some time:)

[micalevisk]

it would parse and validate an string in ISO 8601 into Date obj?

[davidgonmar]

it would parse and validate an string in ISO 8601 into Date obj?

Right now on my app I am not even bothering with validating valid ISO 8601 dates, I just do

@Injectable()
export class ParseDatePipe implements PipeTransform {
  transform(value: (string | Date) | (() => string | Date) | undefined | null) {
    if (!value) throw new BadRequestException('Date is required');
    if (typeof value === 'function') {
      value = value();
    }
    const transformedValue = new Date(value);
    if (isNaN(transformedValue.getTime())) {
      throw new BadRequestException('Invalid date');
    }
    return transformedValue;
  }
}

But I guess validating first ISO 8601 compliance and then parsing will be a better option for library code. Something I want to point out is that since Date is an object, if you want to accept a default value that changes over time, for example,

@Query('date', new DefaultValuePipe(() => new Date()), ParseDatePipe)

to indicate 'default is now', you need to be able to pass a function to ParseDatePipe. Doing new DefaultValuePipe(new Date()) would not work at all since it is run just once when the app starts. Because of that, I think it's also a good idea to accept functions that return a date.

[micalevisk]

got you

but I'm not sure about changing the DefaultValuePipe because there's no way to know which behavior the dev want to have. I mean, since we are supporting functions in DefaultValuePipe already, devs out there may be using it to have functions as default values. So changing this behavior to act as a 'deferred value' instead, would break them as well as we'll loose a feature (in favor of another one)...

So it should be yet another pipe like DeferredDefaultValuePipe

[davidgonmar]

got you

but I'm not sure about changing the DefaultValuePipe because there's no way to know which behavior the dev want to have. I mean, since we are supporting functions in DefaultValuePipe already, devs out there may be using it to have functions as default values. So changing this behavior to act as a 'deferred value' instead, would break them as well as we'll loose a feature (in favor of another one)...

So it should be yet another pipe like DeferredDefaultValuePipe

I don't get what you mean. DefaultValuePipe currently supports any value, including functions. It would be the responsability of ParseDatePipe to check if the value passed is a function and consequently run it, right? In my example I am already using the library's DefaultValuePipe and it works fine.

[davidgonmar]

I mean I kind of get you but my setup of DefaultValuePipe returns function, then ParseDatePipe checks if value is function, and if it is, it executes the function. If it is not, it just takes the value and validates it. What you mean is DeferredDefaultValuePipe actually calling the function?

[micalevisk]

I am already using the library's DefaultValuePipe and it works fine.

yeah but that's because your ParseDatePipe is aware of that, which is something too specific. I mean, people could use our ParseDatePipe without the built-in DefaultValuePipe, and so on. Your solution is too opinatitive to me (which is fine to have as a 3rd-party lib)

DeferredDefaultValuePipe (not sure if this is a good name) would be the same as our current DefaultValuePipe but to deal with functions instead. So we don't need to change DefaultValuePipe at all

[davidgonmar]

I am already using the library's DefaultValuePipe and it works fine.

yeah but that's because your ParseDatePipe is aware of that, which is something too specific. I mean, people could use our ParseDatePipe without the built-in DefaultValuePipe, and so on. Your solution is too opinatitive to me (which is fine to have as a 3rd-party lib)

DeferredDefaultValuePipe (not sure if this is a good name) would be the same as our current DefaultValuePipe but to deal with functions instead. So we don't need to change DefaultValuePipe at all

Got you. Yeah, I agree it makes more sense to make ParseDatePipe behave like other existing parsing pipes, and where you need to have a value generated each request you can use something like a DeferredDefaultValuePipe and pass a function to it.

[davidgonmar]

Is it fine if I start working on a PR for that?

[micalevisk]

@davidgonmar I'd await for Kamil

[micalevisk]

@davidgonmar if you don't mind on writing a PR that can be rejected, go ahead :)

[davidgonmar]

@davidgonmar if you don't mind on writing a PR that can be rejected, go ahead :)

Will probably write it anyways when I have some free time to do it!:)

[kamilmysliwiec]

While I understand the need for the ParseDatePipe, are we sure we want to add yet another pipe to the common package? Isn't this something that could be published as a separate OSS npm library?

[davidgonmar]

While I understand the need for the ParseDatePipe, are we sure we want to add yet another pipe to the common package? Isn't this something that could be published as a separate OSS npm library?

I think it is 'common enough' (since there are other pipes for usual types like UUID, Boolean, etc(I even got confused there wasn't an included ParseDatePipe) that it might be worth it to add it, but I am not sure on the usage it would get.

[micalevisk]

tbh I don't know if we should or not do that. I'll leave this decision up to Kamil.

What I found a bit off is that we have one for UUID while we don't for Date, which is built-in in JS [edit: uuid are native on nodejs via crypto now]. At same time, looks like dates are commonly parsed by object transformation libs like Joi, Zod, class-transformation. So I don't have any strong opinions

[davidgonmar]

tbh I don't know if we should or not do that. I'll leave this decision up to Kamil.

What I found a bit off is that we have a parser for UUID. While we don't for Date, that is built-in in JS. At same time, looks like dates are commonly parsed by object transformation libs like Joi, Zod, class-transformation. So I don't have any strong opinions

Yeah I also found it a bit off since other common 'stringifiable' types, but obviously I respect the decision:)

[scr4bble]

I believe this pipe should be inside nestjs directly to make the set of offered pipes more complete. This is very common query argument type and it's also a built-in in Javascript so I think the case is strong enough.

[amirhoseinZare]

another version of this pipe which can handle both required and not rw=equired usecases

for my use-case I needed a pipe which only take care of the required values and leave not required fields alone

import { BadRequestException, Injectable, PipeTransform } from "@nestjs/common";

@Injectable()
export class ParseDatePipe implements PipeTransform<string | Date | undefined | null> {
    constructor(private readonly required: boolean = true) { }

    transform(value: string | Date | undefined | null | (() => string | Date) | undefined | null): Date {
    if (!this.required && !value) {
        return value as undefined | null;
    }

    if (!value) {
        throw new BadRequestException('Date is required');
    }

    if (typeof value === 'function') {
        value = value();
    }

    const transformedValue = new Date(value);

    if (isNaN(transformedValue.getTime())) {
        throw new BadRequestException('Invalid date');
    }

    return transformedValue;
}
}

you can use it this way:

 async yourController(
   @Query('fromDate', new ParseDatePipe(false)) fromDate?: string | undefined,
   @Query('toDate', new ParseDatePipe(false)) toDate?: string | undefined
  ): Promise<YourResponseDto> {
return;
}

[TrejGun]

@davidgonmar @amirhoseinZare

I wonder in what case can you get value as a function when it comes from query string or post data, even if it is JSON? The only case I can imagine is a manual validation of a manually created object but my paranoia is not so strong to validate after myself

[davidgonmar]

In my case it was something like this @TrejGun

@Query('date', new DefaultValuePipe(() => new Date()), ParseDatePipe)

To obtain the default date as 'now'. Passing new Date() does not work because it just gets evaluated when the app starts running and remains 'constant'