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

parseISO - parsing invalid string into a valid date #1748

Closed
MBelniak opened this issue Apr 28, 2020 · 8 comments · Fixed by #1791
Closed

parseISO - parsing invalid string into a valid date #1748

MBelniak opened this issue Apr 28, 2020 · 8 comments · Fixed by #1791
Assignees
Labels

Comments

@MBelniak
Copy link

MBelniak commented Apr 28, 2020

Hi.
I've come across this issue while using date-fns.
Current behaviour:
parseISO('2016-03-16T08 ooo') // returns Date(Wed Mar 16 2016 08:00:00 GMT+0100 (CET))
Expected behaviour (at least momentjs and some online regex-based parsers):
parseISO('2016-03-16T08 ooo') // returns Invalid Date
Is this actually a bug or for some reason current bahaviour is the expected one?

@imballinst
Copy link
Contributor

@kossnocorp is this intended behavior? From a quick glance on parseISO, it uses RegEx to parse the date and time. I think in this case, it only "partially parses" the invalid string. CMIIW!

@kossnocorp
Copy link
Member

I've no strong opinion regarding this, to be honest. As I see, not only Moment.js behaves like that:

new Date('2016-03-16T08 ooo')
//=> Invalid Date
Date.parse('2016-03-16T08 ooo')
//=> NaN

So I guess this is a bug.

@imballinst
Copy link
Contributor

Alright, thanks for the clarification! I'll try attempting on this issue.

@imballinst imballinst self-assigned this May 18, 2020
@kossnocorp
Copy link
Member

@imballinst thanks 🙏

@dojchek
Copy link

dojchek commented Jun 25, 2020

I think there may be another 'case' of this issue. It successfully parses the two digits number.
parseISO('33') // Fri Jan 01 3300 00:00:00 GMT+0100 (Central European Standard Time)

'33' is not a valid ISO string (https://en.wikipedia.org/wiki/ISO_8601), so I think it should fail and return 'Invalid Date'
Interestingly, one digit or three or more digits numbers fail to parse with 'Invalid Date' being returned.

parseISO('3')     // Invalid Date
parseISO('333')  // Invalid Date

@imballinst
Copy link
Contributor

@dojchek noted -- thanks for the information! I'll try handling that case as well in my PR.

@imballinst
Copy link
Contributor

@dojchek it seems like the 2 digits are used to represent centuries, so I guess that's intended?

it('parses century 1 BC - 99 AD', () => {
const result = parseISO('00')
const date = new Date(0, 0 /* Jan */, 1)
date.setFullYear(0)
assert.deepEqual(result, date)
})
it('parses centuries after 9999 AD', () => {
const result = parseISO('+0123')
assert.deepEqual(result, new Date(12300, 0 /* Jan */, 1))
})
it('allows to specify the number of additional digits', () => {
const result = parseISO('-20', { additionalDigits: 0 })
const date = new Date(-2000, 0 /* Jan */, 1)
date.setFullYear(-2000)
assert.deepEqual(result, date)
})

CMIIW @kossnocorp

@dojchek
Copy link

dojchek commented Jun 26, 2020

@imballinst Yes, I also think that's intended - although not very intuitive..
I found only one reference to this format in wiki article which says:
One may simply write "1981" to refer to that year or "19" to refer to the century from 1900 to 1999 inclusive

So it is in fact a valid ISO date. I still need this to "fail" in my use case - so I will check it with some regex or simply check the length of string before doing parseISO, since I only work with dates like 2020-06-26 and 2020-06-26T04:33:26+00:00.

Thank you very much for checking this out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants