Bug Tracker

Opened 6 years ago

Closed 6 years ago

#14227 closed bug (duplicate)

$.isPlainObject improperly returns true for moment.js in 2.x

Reported by: dcherman Owned by: Rick Waldron
Priority: undecided Milestone: None
Component: unfiled Version: 2.0.3
Keywords: Cc:
Blocked by: Blocking:

Description

In the 2.x branch, $.isPlainObject() returns true for a moment instance ( http://momentjs.com/ ) where in 1.x it returns false ( which is expected )

Test Case: http://jsfiddle.net/N5UBJ/1/

Change History (5)

comment:1 Changed 6 years ago by Rick Waldron

Owner: set to Rick Waldron
Status: newassigned

comment:2 Changed 6 years ago by Rick Waldron

Resolution: wontfix
Status: assignedclosed

This isn't a bug in jQuery 2.x, it's a bug in Moment that was allowed by jQuery 1.x. In the moment.js source, where moment.prototype is defined, it's the constructor is being paved over and never restored.

https://github.com/moment/moment/blob/05028e20e526b5f38dde3748833e98d7c8e846e8/moment.js#L1133-L1468

A reduced case:

function C(opts) {
  this.p = opts && opts.p;
}
// This is actually _defining_ the value of C.prototype to be a plain object, with a method called "clone"
C.prototype = {
  clone: function() {
    return new C(this);
  }
};

This can be fixed by restoring the constructor:

function C(opts) {
  this.p = opts && opts.p;
}
// This is actually _defining_ the value of C.prototype to be a plain object.
C.prototype = {
  // But restoring constructor also restores the [[Prototype]] chain for all instances of C
  constructor: C, 
  clone: function() {
    return new C(this);
  }
};

Notice that even jQuery does this for its own prototype: https://github.com/jquery/jquery/blob/master/src/core.js#L76-L80

For more information, read through: http://bugs.jquery.com/ticket/13571 specifically: http://bugs.jquery.com/ticket/13571#comment:4

comment:3 Changed 6 years ago by scottgonzalez

If this is a wontfix, then does that mean we're intentionally allowing a difference in behavior between 1.x and 2.x?

comment:4 Changed 6 years ago by Rick Waldron

Resolution: wontfix
Status: closedreopened

comment:5 Changed 6 years ago by Rick Waldron

Resolution: duplicate
Status: reopenedclosed

Duplicate of #13571.

Note: See TracTickets for help on using tickets.