Crash when Object.prototype is added to

[JonathanILevi]

I am working on adding TWGL to my project but right away I get errors. I have added functions to Object.prototype in my JavaScript application and TWGL does not like that. This is a bug; your code should not be so easily affected by what I do in mine.

It crashes in the setAttributes and setUniforms because of the line for (var name in buffers) {. for in should not be used because it also takes from the prototype. The proper way in ES6 is for of on a list--your line should be for (var name of Object.keys(buffers)) { or for pre-ES6 iterate indexes with Object.keys().length.

It is not quite as pretty but it makes your code literally incompatible with mine.

If you are okay with this change I can create a pull-request for it.

Thanks,

[greggman]

Sorry but in this case no for in is the correct thing to do IMO. The code in question is a hotspot for perf and for in is 2x to 15x faster than for of Object.keys

Test: https://jsperf.com/for-in-vs-for-of-keys/1

firefox

safari

chrome

Even checking Object.hasOwnProperty is generally ~2x slower which is unaccaptable for a function called in the inner loop of rendering 1000 to 2000 objects 60 times a second.

I'm sorry if that's not compatible with modifying Object.prototype

In normal use I use for of object.keys but in this case it would be the wrong solution IMO.

[JonathanILevi]

Wow, yeah, that sucks but you are right. I guess that is the kind of thing you get with a messy language like JavaScript.

I will either use a local modification, avoid those functions, or change the use of the other incompatible code.

[greggman]

just fyi, you might be able to add things to the Object prototype using defineProperty

Object.defineProperty(Object.prototype, 'test',  {
  value: function() {
    console.log('test:', this);
  },
});

Then for in won't see it (note: I didn't test across browsers but I'm guessing it would work)

[JonathanILevi]

Oh, thanks! Reading the spec, the effect you are describing does seem correct. I will do some testing on different browsers. I didn't know that was a thing. I think non-enumerable ought to be the default for prototypes but that is nice that it can be configured.

What you have might actually be technically correct anyway—although less fool proof. (: