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

3763 parse iso fix #3780

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JustyProgrammmer
Copy link

@JustyProgrammmer JustyProgrammmer commented Apr 24, 2024

Problema:

parseISO parses number '30' on a date with the year 3000.

const { parseISO} = require('date-fns');
console.log(parseISO('30')); // prints '3000-01-01T08:00:00.000Z'

Soluction:

Since the parseISO function in its documentation receives dates in full format (YYYY-MM-DDTHH:mm:ss.sssZ
) or dates in partial format (YYYY-MM-DD, YYYY-MM, YYYY) a code was added that sends all years with less than 4 digits to 4-digit years, this way the function correctly returns the dates as happens with entries 0001, 0030, 0999 which translates into 0001-01-01T00:00:00, 0030-01-01T00:00:00, 0999-01-01T00:00:00 respectively.

And as a reference, tests being implemented with the same purpose in Go language respond as invalid input and only allow dates in the full format.: Test in Go Lang

it("parses YY", () => {
const result = parseISO("20");
expect(result).toEqual(new Date(2000, 0 /* Jan */, 1));
expect(result.getUTCFullYear()).toEqual(19);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please compare with a full date like before, as it prevents other date components from being parsed incorrectly.

year = '0' + year;
}
return year;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use addLeadingZeros from src/_lib/addLeadingZero/index.ts

date.setFullYear(-2000, 0 /* Jan */, 1);
date.setHours(0, 0, 0, 0);
expect(result).toEqual(date);
const result = parseISO("20", { additionalDigits: 0 });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep the tests as is and update/add only what is required. Here, it needs to be clarified how these changes relate to the PR goal.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the feedback. I will apply all of the recomendations, and submit again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Triaging
Development

Successfully merging this pull request may close these issues.

None yet

2 participants