Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Angular have problems with Object.prototype #10469

Closed
Akxe opened this issue Dec 15, 2014 · 13 comments
Closed

Angular have problems with Object.prototype #10469

Akxe opened this issue Dec 15, 2014 · 13 comments

Comments

@Akxe
Copy link

Akxe commented Dec 15, 2014

The bug stated (and is not yet fixed as of 1.3.6) when upgraded from 1.3.0-rc.0 to 1.3.0-rc.1. Sorry to report this late, but I couldn't figure it out.
The bug is easily reproduced by:
Object.prototype.x = function(){};

I think this is a bug, since prevent the user from using prototypes.

@caitp
Copy link
Contributor

caitp commented Dec 15, 2014

1.3.0-rc.1 is ancient, numerous similar bugs have since been fixed. Please try with 1.3.6 and report back =)

@Akxe
Copy link
Author

Akxe commented Dec 15, 2014

Well updated the original issue, the bug still persists.

@caitp
Copy link
Contributor

caitp commented Dec 15, 2014

okay, we're releasing today and there was one more fix for this that landed recently (I think it landed anyway...), so you might want to try 1.3.7 in a few hours --- in the mean time, please provide a minimal reproduction of your bug with v1.3.6

@Akxe
Copy link
Author

Akxe commented Dec 15, 2014

Took original plunker from AngularJS - form added 1 line (14.) and error exist (edited plunker)

@caitp
Copy link
Contributor

caitp commented Dec 15, 2014

Okay, so unrelated to filtering --- that doesn't make a lot of sense though.

@caitp
Copy link
Contributor

caitp commented Dec 15, 2014

I am not sure exactly why this is happening, because all iterations over $error seems to be in forEach(), which checks for own properties --- but it is fixed by using createMap() when initializing $error. I'll send a PR

caitp added a commit to caitp/angular.js that referenced this issue Dec 15, 2014
caitp added a commit to caitp/angular.js that referenced this issue Dec 15, 2014
caitp added a commit to caitp/angular.js that referenced this issue Dec 16, 2014
@pkozlowski-opensource pkozlowski-opensource added this to the 1.3.8 milestone Dec 16, 2014
@btford btford modified the milestones: 1.3.8, 1.3.9 Dec 19, 2014
@Akxe
Copy link
Author

Akxe commented Jan 6, 2015

I saw it is work in progress, I would like to ask how is it going... I can't really do much else

@enigma1
Copy link

enigma1 commented Jan 12, 2015

I am not sure why this is a bug. From what I see you set the native prototype with an enumerable property. This means any object now has the property x and this property can be enumerated.
If you want to see the angular error try the equivalent:

Object.defineProperty(Object.prototype, "x", {enumerable: true})

@Akxe
Copy link
Author

Akxe commented Jan 16, 2015

I have updated plunker to 1.4.0-beta.0. http://plnkr.co/edit/ZDKnJJAOsng5dBlPv7s7?p=preview

@realityking
Copy link
Contributor

Honestly, I don't think it can be expected that code work with enumerable properties on a built in prototype. Especially modifying Object.prototype can cause all kinds of "interesting" issues.

The easiest fix would be to declare the method in the following way:

Object.defineProperty(Object.prototype.x, 'x', {
  value: function(){},
  writable: true,
  configurable: true
});

@Akxe
Copy link
Author

Akxe commented Jan 16, 2015

Well it is a working workaround, but forcing people to use different syntax isn't a really good solution to problem.

@realityking
Copy link
Contributor

Just to be clear, this is a best practice (see for example here) and not specific to Angular. Extending Object.prototype is dangerous but extending it with a enumerable method is just asking for trouble.

To a lesser extend the same holds true for Array.prototype, Date.prototype and all other build-ins.

@Akxe
Copy link
Author

Akxe commented Jan 16, 2015

Allright, I'll redo it, but it should be noted in the docs too.

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