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

[feature] Check whether input is date before checking if format is array #3530

Closed
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
5 participants
@john-crisostomo
Contributor

john-crisostomo commented Oct 26, 2016

Fixes the issue as demonstrated here: https://jsfiddle.net/r6wpn9un/

@maggiepint maggiepint changed the title from fixes issue #3529 to Check whether input is date before checking if format is array Oct 26, 2016

@maggiepint

This comment has been minimized.

Member

maggiepint commented Oct 26, 2016

This code is fine, but you need to add unit tests.

@john-crisostomo

This comment has been minimized.

Contributor

john-crisostomo commented Oct 26, 2016

Okay I will add some for this case. Thank you.

@maggiepint

This comment has been minimized.

Member

maggiepint commented Oct 28, 2016

Your test logic is good, but you cannot pass strings other than ISO8601 Extended with Offset to the date constructor and get a reliable result. The format you are using is not a defined format, thus each browser may parse it differently. While it works on a machine with a US locale, it may be parsed as invalid in any country using DD/MM/YYYY format.

Instead, use explicit syntax when constructing your test dates:

new Date(2016,10,28)
@mj1856

This comment has been minimized.

Member

mj1856 commented Oct 28, 2016

👍 to @maggiepint's comment. But also, make sure to recognize that months in that syntax are zero-based.

@mj1856

This comment has been minimized.

Member

mj1856 commented Oct 28, 2016

Oh, and don't edit moment.js directly. It will be generated as part of the build.

@john-crisostomo

This comment has been minimized.

Contributor

john-crisostomo commented Oct 29, 2016

Thank you for the guidance, I am still new at this and I need all the help I can get. :) I will fix them today.

@ichernev

This comment has been minimized.

Contributor

ichernev commented Nov 6, 2016

Merged in 03d7831

@ichernev ichernev changed the title from Check whether input is date before checking if format is array to [feature] Check whether input is date before checking if format is array Nov 6, 2016

@ichernev ichernev closed this Nov 6, 2016

ichernev added a commit that referenced this pull request Nov 6, 2016

Merge pull request #3530 from johncrisostomo:develop
[feature] Check whether input is date before checking if format is array

@mj1856 mj1856 added this to the 2.16.0 milestone Nov 10, 2016

@mj1856 mj1856 added Bug-fix and removed Pending Next Release labels Nov 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment