Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript generated code for type: 'date' generates bad values #520

Closed
danzel opened this issue Oct 18, 2017 · 12 comments
Closed

Typescript generated code for type: 'date' generates bad values #520

danzel opened this issue Oct 18, 2017 · 12 comments

Comments

@danzel
Copy link
Contributor

danzel commented Oct 18, 2017

I feel bad reporting this because it's my bug! haha :)
Follows #492 #493

The code I added for this is wrong (when your date isn't in utc?)

I'll revisit this tomorrow.
For moment we can use .format('YYYY-MM-DD'), will have to figure out Date object.

moment('2017-10-18').toDate()
  Wed Oct 18 2017 00:00:00 GMT+1300 (New Zealand Summer Time)
moment('2017-10-18').toISOString()
  "2017-10-17T11:00:00.000Z"
moment('2017-10-18').format('YYYY-MM-DD')
  "2017-10-18"

moment('2017-10-18 12:00').toISOString()
  "2017-10-17T23:00:00.000Z"
moment('2017-10-18 12:00').toDate()
  Wed Oct 18 2017 12:00:00 GMT+1300 (New Zealand Summer Time)
moment('2017-10-18 12:00').format('YYYY-MM-DD')
  "2017-10-18"

new Date('2017-10-18')
  Wed Oct 18 2017 13:00:00 GMT+1300 (New Zealand Summer Time)
new Date('2017-10-18').toISOString()
  "2017-10-18T00:00:00.000Z"

new Date('2017-10-18 12:00')
  Wed Oct 18 2017 12:00:00 GMT+1300 (New Zealand Summer Time)
new Date('2017-10-18 12:00').toISOString()
  "2017-10-17T23:00:00.000Z"
@RicoSuter
Copy link
Owner

Can you provide a sample?

@danzel
Copy link
Contributor Author

danzel commented Oct 18, 2017

In #493 I made date format data work, we produce code like this:

data["myDate"] = this.myDate ? this.myDate.toISOString().slice(0, 10)

However, toISOString converts the date to UTC, then gets the date portion. Which means we can shift the date (as in the bits I posted above).
We should instead get the year/month/date in whatever timezone the moment/Date object already is in.

@RicoSuter
Copy link
Owner

RicoSuter commented Oct 18, 2017

So for Date we need:

d.getFullYear() + "-" + d.getDate() + "-" + d.getDay()

and for momentJS we need

m.format("YYYY-MM-DD")

?

@danzel
Copy link
Contributor Author

danzel commented Oct 18, 2017

I think we might need to pad it for Date.
But for moment that is correct 👍

It is meant to be full-date format as in https://xml2rfc.tools.ietf.org/public/rfc/html/rfc3339.html#anchor14

@RicoSuter
Copy link
Owner

Whats the difference to:

full-date       = date-fullyear "-" date-month "-" date-mday

?

@danzel
Copy link
Contributor Author

danzel commented Oct 18, 2017

They should look like:
2017-01-01
Not:
2017-1-1

date-fullyear   = 4DIGIT
   date-month      = 2DIGIT  ; 01-12
   date-mday       = 2DIGIT  ; 01-28, 01-29, 01-30, 01-31 based on
                             ; month/year

Says they need to be certain lengths.

something dodgy like:

d.getFullYear() + '-' + (d.getMonth() < 9 ? ('0' + (d.getMonth()+1)) : (d.getMonth()+1)) + '-' + (d.getDate() < 10 ? ('0' + d.getDate()) : d.getDate())

getMonth is 0-11 because JS...

@RicoSuter
Copy link
Owner

Oh shit... :)

@danzel
Copy link
Contributor Author

danzel commented Oct 18, 2017

hahaha :)

@RicoSuter
Copy link
Owner

should be <= 9

@danzel
Copy link
Contributor Author

danzel commented Oct 18, 2017

Nah, cause if it is 9, you +1 it, then it is 10 and doesn't need padding.

@RicoSuter
Copy link
Owner

ah yes :)

danzel added a commit to danzel/NJsonSchema that referenced this issue Oct 21, 2017
danzel added a commit to danzel/NJsonSchema that referenced this issue Oct 21, 2017
@danzel
Copy link
Contributor Author

danzel commented Oct 21, 2017

Have pushed up a start to this, see #528

RicoSuter added a commit that referenced this issue Oct 31, 2017
Working towards fixing date formatting #520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants