Skip to content

Conversation

@shvaikalesh
Copy link
Contributor

@shvaikalesh shvaikalesh commented Jun 17, 2018

Previous behavior: toMillis calls valueOf.

This PR aligns Luxon's DateTime#valueOf with it's very own Duration#valueOf (introduced in #261), implementation in moment.js, and the native Date#valueOf. They all have logicless valueOf that just calls some other method. And no other method calls valueOf directly, only JavaScript does.


test('Invalid DateTimes return invalid Dates', () => {
expect(organic1.toJSDate().valueOf()).toBeFalsy();
expect(organic1.toJSDate().valueOf()).toBe(NaN);
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised this works, actually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't work initially, was fixed by jestjs/jest#4917, that's why I updated jest.

@icambron
Copy link
Contributor

This looks good. The build error is probably because I haven't pinned down the version of prettier. I bet if you npm update, then npm run format, then push that up, it will resolve. I'll fix that issue soon.

@shvaikalesh
Copy link
Contributor Author

BTW, that lint! convention for reformatting looks very nice.

@icambron icambron merged commit b016fe7 into moment:master Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants