fix(parse): dirty checking support for objects with null prototype #9568
Conversation
… order not to inherit from Object
| @@ -1092,7 +1092,8 @@ function $ParseProvider() { | |||
| // attempt to convert the value to a primitive type | |||
| // TODO(docs): add a note to docs that by implementing valueOf even objects and arrays can | |||
| // be cheaply dirty-checked | |||
| newValue = newValue.valueOf(); | |||
| typeof newValue.valueOf !== 'function' && (newValue = Object.valueOf.apply(newValue)) || | |||
There was a problem hiding this comment.
Can you cache Object.prototype.valueOf as valueOfObject in src/Angular.js instead, and just always use newValue = valueOfObject.call(newValue); ? thanks
There was a problem hiding this comment.
I was not sure to do that in case valueOf may have been overriden, i'll update it right away!
|
need a test for this change which fails without it |
|
Working on the test |
|
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
| @@ -1092,7 +1092,7 @@ function $ParseProvider() { | |||
| // attempt to convert the value to a primitive type | |||
| // TODO(docs): add a note to docs that by implementing valueOf even objects and arrays can | |||
| // be cheaply dirty-checked | |||
| newValue = newValue.valueOf(); | |||
| newValue = valueOfObject.call(newValue); | |||
There was a problem hiding this comment.
I don't think this is right. Won't this always use Object.prototype's method ?
The point is to only use it when there is no newValue.valueOf() method.
(The same goes for the other changes below.)
…Mayorga who provided the test case!
|
Pull request updated with test case and CLA signed. Special thanks to Julian Mayorga who provided the test case! |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
HeberLZ
changed the title
| if(typeof value.valueOf !== 'function'){ | ||
| return Object.prototype.valueOf.call(value); | ||
| } | ||
| return value.valueOf(); |
There was a problem hiding this comment.
how about:
var objectValueOf = Object.prototype.valueOf;
function getValueOf(value) {
return isFunction(value.valueOf) ? value.valueOf() : objectValueOf(value);
}
There was a problem hiding this comment.
I'll update it right away!
|
@caitp can you get this in please? |
…e changes requested
|
PR updated with the recommendations from @IgorMinar |
|
I'll get it in |
| var objectValueOf = Object.prototype.valueOf.call; | ||
|
|
||
| function getValueOf(value) { | ||
| return isFunction(value.valueOf) ? value.valueOf() : objectValueOf(value); |
There was a problem hiding this comment.
You can't do this this way, it needs to be
var objectValueOf = Object.prototype.valueOf;
// ...
objectValueOf.call(value);
// ...
|
If you fix those up, the tests should be green and I'll land it for you =) |
|
I'll update it right away! On Mon, Oct 13, 2014 at 10:42 PM, Caitlin Potter notifications@github.com
Heber López | Google Developers Group Lead Organizer, Mendoza | +HeberLZ |
HeberLZ
changed the title
|
lgtm, will land when travis finishes (it looks like it will be green already) |
|
PR updated with the comments! Thanks @gkalpak @caitp @IgorMinar and @JulianMayorga for the help on my first contribution to AngularJS!! |
|
Merged in 28661d1, thanks! |
With this changes, objects initially created with Object.create(null) can be bind to the view. This is because if the valueOf function is not available, it will use Object.valueOf.apply instead