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

moment() instanceof moment fails #2640

Closed
nsbawden opened this Issue Sep 26, 2015 · 4 comments

Comments

Projects
None yet
5 participants
@nsbawden

nsbawden commented Sep 26, 2015

Can it made so that the javascript standard way of finding out what an object IS works with moment? With the latest version using moment() instanceof moment returns false.

I can put moment.prototype = moment.fn at the beginning of my code and then moment() instanceof moment returns true and everything seems fine in the ways I am using moment. However, there may be some case where this does not work so it would be real handy to have this ability integrated into the library by a professional to ease our testing when a moment is a moment, if it is as simple to do as it appears.

Thanks in advance,
dAc

@jisaacks

This comment has been minimized.

Contributor

jisaacks commented Sep 26, 2015

instanceof is not very reliable in JS. The correct way to check if an object is moment would be to use isMoment:

moment.isMoment( moment() ); // true
@nsbawden

This comment has been minimized.

nsbawden commented Sep 27, 2015

That is an interesting position. instanceof is based on the prototype chain and is very reliable if the prototype chain is properly initialized. It is very fast, being an intrinsic javascript operator, and provides a very easy to remember and standards compliant way to check unknown object types when handling them in a general handler.

The jQuery library documents instanceof as the best and most reliable way to determine if an object is a jQuery object. Considering the widespread use and reliability of jQuery that seems to have been a good choice. Yes, it is possible for someone to deliberately change the prototype chain, as I have done to standardize my use of moment in my code, by fixing the prototype chain. But that being said, it is similarly as easy to replace the isMoment method so not sure how that goes to reliability.

Why not do both and make the library easy to use and standards compliant for those wishing to use instanceof? What does that hurt? It should not be hard to do so ... and run the tests to be sure all is well.

@alburthoffman

This comment has been minimized.

Contributor

alburthoffman commented Sep 27, 2015

this is interesting. I mean it's good to make sure the operator instanceof works for moment. So any reason for not doing this?

Checked the code, It should be easy to do it.

@mj1856 mj1856 added the Enhancement label Oct 2, 2015

alburthoffman added a commit to alburthoffman/moment that referenced this issue Oct 5, 2015

moment#2640: moment() instanceof moment fails
make the operator instanceof support for moment

alburthoffman added a commit to alburthoffman/moment that referenced this issue Oct 5, 2015

moment#2640: moment() instanceof moment fails
make the operator instanceof support for moment (reverted from commit 623d1d2)

alburthoffman added a commit to alburthoffman/moment that referenced this issue Oct 5, 2015

ichernev added a commit that referenced this issue Nov 9, 2015

Merge pull request #2648 from alburthoffman:instanceof
fix issue #2640: support instanceof operator
@icambron

This comment has been minimized.

Member

icambron commented Nov 26, 2015

Fixed in aee4e2f

@icambron icambron closed this Nov 26, 2015

sereysethy added a commit to sereysethy/moment that referenced this issue Mar 14, 2016

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