#13715 closed bug (duplicate)
isPlainObject won't work correctly when constructor property is changed
Reported by: | fhcj2 | Owned by: | |
---|---|---|---|
Priority: | undecided | Milestone: | None |
Component: | unfiled | Version: | 1.9.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
When you instantiate a non Object class (not a plain object), and add it its own constructor property, the function will take it for a Plain Object.
See it here: http://jsfiddle.net/fhcj2/J8Yrx/
Change History (11)
comment:3 Changed 8 years ago by
Actually this issue is about jQuery 2.0 (had just cloned it from the repository, but forgot to change it here, sry..)
But anyway this is an issue that happens both in 2.0 and 1.9.1 versions of jQuery, and I think it's not related to "Ticket #13571", since it covers a specific situation when the object is added a "constructor" property. Would you explain why you considered it as a duplicate? =\
comment:4 Changed 8 years ago by
Your jsFiddle specificely uses jQuery 1.9.1, not 2.0.x. Also, if you change the version, your test no longer returns true in the last case.
Are you sure you have a problem with specifically jQuery 2.0? Because it doesn't seem so.
comment:5 Changed 8 years ago by
Just noticed that now that you said... But I even recloned it right now and the error still occurs on the compiled (by grunt) version from the repository. It says in the code:
/*! * jQuery JavaScript Library v2.0.0-pre * http://jquery.com/ * * Includes Sizzle.js * http://sizzlejs.com/ * * Copyright 2005, 2013 jQuery Foundation, Inc. and other contributors * Released under the MIT license * http://jquery.org/license * * Date: 2013-04-03 */
Would that "2.0.0b1" or "2.x" from jsfiddle be any different from this one? Ty for the reply btw =]
comment:6 Changed 8 years ago by
jQuery edge 2.x is code from code.jquery.com/jquery-git2.js which is always the latest jQuery compilation from master. If you think your compiled code does sth different, try comparing those files (using, for example, some command line tool like diff
).
Isn't it possible that your browser cached jQuery 1.9? I'd suppose this could be the reason as the jsFiddle clearly shows the problem doesn't exist in jQuery 2.0
comment:7 Changed 8 years ago by
I'm not sure what part is unclear, both are directly related to detecting "plain object"-ness when an object's constructor has been changed—either by defining the prototype with an object (and thereby changing the prototype.constructor property) or by adding an own property that happens to be called "constructor". Once you modify a special property, such as "constructor", all bets are off.
comment:8 Changed 8 years ago by
Sry, guys.. My mistake. That example would only cover the error in 1.x really. Made another jsfiddle script using the 2.x jQuery version. http://jsfiddle.net/fhcj2/cNNA4/ Wouldn't it be an error? The change I proposed would fix it too...
comment:9 Changed 8 years ago by
Your change moves the check one level below. It can fix your specific use case but it's not a golden solution. For one, your code will completely break when invoked on object created via Object.create(null)
- null prototype doesn't have a constructor field, obviously.
comment:10 Changed 8 years ago by
Didn't remember that function.. But I think perhaps a simple "!= null" would solve that problem, right? since it takes an object to reach this code.
var objProto = Object.getPrototypeOf(obj); if (objProto != null && !core_hasOwn.call(objProto.constructor.prototype, "isPrototypeOf") ) { return false; }
Although it wouldn't cover another (possible)problem which also happens in jQuery 2.x: http://jsfiddle.net/fhcj2/jwpqK/
Since an object created this way seems to behave exactly as a "plain object", I'm not sure if it's not right for it to be identified as such. In the case it is not considered a bug, maybe this code would suffice?
comment:11 Changed 8 years ago by
Well, this additional check doesn't help in all cases since you just moved it one level lower. If instead of assigning MyClass.prototype
to null
you assign it to Object.create(null)
, the code will throw. Sure, it's catched later but using exceptions for flow control is not a good practice. Not to mention your code no longer recognizes {}
as a plain object which kind of misses the point...
Like rwaldron said, when you change the prototype field of the constructor function or the constructor field of an object, all weird kinds of things could happen.
P.S. There is no need to type $(document).ready
by hand in jsFiddle, since you chose onLoad
in the left jsFiddle pane all the code is invoked after window load event occured (and, thus, jQuery document ready, too).
I think it's cause of this block of code:
Since every object has its own prototype, which leads to the constructor who created it, I think it could be changed for:
I tested it and it fixed the problem. What do you guys think? Is it ready for a pull request?