tsoa icon indicating copy to clipboard operation
tsoa copied to clipboard

Default date generated in local time

Open Cerber-Ursi opened this issue 5 years ago • 5 comments

I'm working in Novosibirsk (UTC +7), and I've noticed the following. In one of the tests there is the following code:

  @Get('paramaterImplicitDate')
  public async implicitDate(@Query() date = new Date(2018, 1, 15)): Promise<void> {
    //
  }

And here's the parameters definition generated in routes.ts:

      const args={
        date: { "default": "2018-01-14", "in": "query", "name": "date", "dataType": "date", "validators": { "isDate": { "errorMsg": "date" } } },
      };

Note that date is generated in local time, but then placed in default field in UTC, shifting one day back. This can potentially lead to erroneous behavior.

I'm not sure how this should be fixed (hence an issue and not a PR), but it seems that the simplest way is to use hand-made stringifier for dates instead of standard toISOString().

Cerber-Ursi avatar Oct 31 '18 08:10 Cerber-Ursi

I think the fix for this would be that the dates should be parsed as UTC, not local time. Thoughts @Cerberuser?

lukeautry avatar Dec 09 '18 17:12 lukeautry

In the mentioned case, the date on spec generation is not parsed at all. I'm not sure how to deal properly with data objects in this case.

Cerber-Ursi avatar Dec 10 '18 05:12 Cerber-Ursi

The date isn't exactly parsed, but check out resolveType.ts. When the routes are getting generated, we're actually looking at the string being passed into the Date constructor and then passing it along to a new Date. I think since this is getting formatted down the road for the routes generator (and, I assume, the spec generator), that is always going to be done using local time, which seems wrong.

lukeautry avatar Dec 10 '18 06:12 lukeautry

Hi @lukeautry. I love what you've accomplished with tsoa. Thank you for the hard work. So, I would be happy to take on this PR, but I'm not sure how I could do it in a way that isn't a non-breaking change.

Basically, I agree with your sentiment above that the dates should be parsed as UTC; however, not all service developers agree with the notion that dates should be passed in UTC only. And although most seasoned developers agree that services should only pass UTC and accept UTC, we can't necessarily always require that.

So I suppose there are a few potential approaches:

  1. update DateTimeValidator to include a new option. Something like "isUtc", which upon validation would allow tsoa to throw a ValidationError if the string does not end in a "Z" character.
  2. always require the ISO string to end in a "Z". This would be a breaking change.

Either way, my understanding of MomentJS is that as long as the input string ends in a "Z", then the produced object will be parsed as UTC. I would think that would solve the error. Of course, I would verify that with a unit test if I were to pick up this PR.

Anywho, let me know which approach you'd like me to take @lukeautry (breaking or non-breaking), and I can start the work.


P.S. if you select option 1, there are still two ways to approach it:

// Option 1a
export interface DateTimeValidator {
  isDateTime?: { errorMsg?: string, mustBeUtc: boolean };
  minDate?: { value: string, errorMsg?: string };
  maxDate?: { value: string, errorMsg?: string };
}
// Option 1b
export interface DateTimeValidator {
  isDateTime?: { errorMsg?: string };
  mustBeUtc? : { value: string, errorMsg?: string };
  minDate?: { value: string, errorMsg?: string };
  maxDate?: { value: string, errorMsg?: string };
}

dgreene1 avatar Feb 11 '19 17:02 dgreene1

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

github-actions[bot] avatar Aug 30 '19 00:08 github-actions[bot]