Use setPrototypeOf
wesleytodd opened this issue · 14 comments
Hey, just perusing the repo's around here and noticed in this one it is setting __proto__. Not sure what the end use of this module will be, but we had a discussion around this here because that breaks some browser support. Also, there is a new and standardized api to do this, Object.setPrototypeOf. So the router is using this module to get a nice fallback.
If you want I can PR that in, but I wanted to open this to make sure I wasn't misunderstanding the use case for this module :)
When @dougwilson has previously tested this, __proto__ was the most efficient in V8 / node.
@wesleytodd what browsers don't support __proto__? As far as I understand all current ones do and will continue to do so due to usage.
IE10 was the main one, but I guess since MS just dropped support for anything below 11, maybe this is a moot point. Technically according to MDN __proto__ has been standardized as legacy:
Warning: While Object.prototype.proto is supported today in most browsers, its existence and exact behavior has only been standardized in the ECMAScript 6 specification as a legacy feature to ensure compatibility for web browsers. For better support, it is recommended that only Object.getPrototypeOf() be used instead.
The original issue I linked to was about a year ago, and I was trying to support back to IE9 with the router. Anyway, totally understand if you want to close this.
EDIT: Or maybe if we are sure the perf is better I could switch that module to prefer the __proto__ version?
@wesleytodd what exactly are you trying to run in a browser that requires this functionality?
Hopefully most what what express is doing. I am currently running just the router, but in the future I would love to be able to run something that looks and functions just like an express app on the backend. That is the only reason I even mentioned this issue here is because I assume this module will take over this (https://github.com/strongloop/express/blob/5.0/lib/express.js#L43-L44) in the future. Am I correct in assuming that?
Correct. If you make a PR I'll review it but I don't really make any guarantees about browser support from myself.
Thats what I'm here for, browser support :)
And FWIW, just ran this jsperf and the perf is comparable in up to date chrome: http://jsperf.com/new-vs-mutable-proto/2
@wesleytodd Two things:
- There will be up to like, 100 properties attached, not just from express, in a busy application.
- They can't really be defined in an object literal I think, most will be defined after the fact, causing some amount of deopt.
- updated that jsperf to add 200 properties, still same perf characteristics. http://jsperf.com/new-vs-mutable-proto/5
- This might be an issue, since the fallback iterates over the properties. Do you have a link to somewhere that does this so I can see?
@wesleytodd Currently in express: no, but that is what this library does because it makes this a usable API.
Some background: setPrototypeOf() seems fairly new, I only heard about it around a week ago. Either way, it's definitely not in node <4. The alternatives had been either __proto__ or some sort of Object.setProperty(). The latter was much slower.
From what I can tell __proto__ and setPrototypeOf() are possibly aliases in some form.
Yeah, I guess its newish, here is the best source for info on it that I know of: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf
It was spec'd out in es2015, but that was the point of that module I linked to originally. To get the new feature while keeping backward compat.
For reference, I am currently working on building a express compatible implementation for the browser. It would hopefully encompass as much of the express api surface area as makes sense run in the browser. So supporting the ability to customize req/res in the same way express allows would be awesome. That is why I care about this.
@wesleytodd are you interested in making a PR for it? :)
Absolutely, sorry, I got distracted and on other things. I can do that tonight or tomorrow when I get home.
See #6 :)
As far as my opinion on this, I'm more aligned with @wesleytodd, as that is the more up-to-date consensus since the last time you were around, @Fishrock123 :) The router discussion has a pretty relevant recent discussion on this.