[Object.defineProperty] possible to contribute a fix on 3.x series?
Opened this issue · 7 comments
What
The Object.defineProperty polyfill correctly makes use of the native version when exists, but only on very specific objects. Thus, for random general usage the polyfill blatantly ignores the native Object.defineProperty. I can't seem to find a reason to not use the native version always if exists (why limit it to window, document, or Element ?).
We find this problem when trying to force-load the bind polyfill with:
https://polyfill.io/v3/polyfill.js?features=Function.prototype.bind|always
Details
We would need this fixed on the 3.x line, given that the latest 4.x line also removes the Function.prototype.bind which is required for our use case.
I've prepared a branch, but I can't contribute a PR to create a new 3.x branch in this repository. AFAIK the creation of that new branch would be required to introduce changes based on 3.111.0:
https://github.com/Financial-Times/polyfill-library/compare/v3.111.0...the-hotels-network:3.x?expand=1
Please let me know if I can help this advance.
Thanks a lot @JakeChampion and contributors.
is required for our use case.
Can you elaborate a bit on this?
Which browsers do you need to support?
is required for our use case.
Can you elaborate a bit on this? Which browsers do you need to support?
We are not using it based on the user-agent (hence the always argument), but based on stumbling upon faulty/incomplete bind implementations. That is the case of websites that use PrototypeJS and MooTools, which unfortunately are still included on actual sites (and worse, used by thirdparty libraries like jotforms).
I must clarify that we self-host an instance of the polyfill-service and we could fork and fix our own version. Still, when inspecting the actual "problem" I thought the change would make sense for everyone.
Thanks
I wonder if this would be resolved with the gated flag?
https://polyfill.io/v3/polyfill.js?features=Function.prototype.bind|always&flags=gated
This adds feature detection around the polyfill.
The line you referred to isn't intended as feature detection as far as I can tell.
I wonder if this would be resolved with the
gatedflag?https://polyfill.io/v3/polyfill.js?features=Function.prototype.bind|always&flags=gated
This adds feature detection around the polyfill.
Good thought. Unfortunately that also gates bind, which makes my intention to redefined Function.prototype.bind with always futile.
Based on your proposal, I thought this could work:
https://polyfill.io/v3/polyfill.js?features=Object.defineProperty|always|gated,Function.prototype.bind|always
But seems like Object.defineProperty gets pulled in as a dep of bind and loses the original gated param.
The line you referred to isn't intended as feature detection as far as I can tell.
Absolutely agree. I really can't make my head around that line. I'd like to understand why would make sense limiting in there the nativeDefineProperty usage to window, document, element, etc? There is nothing explained in the tests, and the line seems to be there since forever...
Thanks guys
I'd like to understand why would make sense limiting in there the nativeDefineProperty usage to window, document, element, etc?
The issue with the native Object.defineProperty is in Internet explorer 8, Object.defineProperty could only work on DOM elements, and not on any type of object.
The feature detection test confirms whether Object.defineProperty can work on any type of object
I'd like to understand why would make sense limiting in there the nativeDefineProperty usage to window, document, element, etc?
The issue with the native Object.defineProperty is in Internet explorer 8, Object.defineProperty could only work on DOM elements, and not on any type of object.
Ouuuuch. Good insight! All finally makes sense, and nothing should be changed on 3.x then....
I definetily need to push the bind polyfill free of the Object.defineProperty dep, but It can't be done with the current polyfill-service. And shouldn't, since that is a too specific use case. I will have to solve the situation by other means.
Thanks a lot for your help @JakeChampion @romainmenke 👏 ❤️
For future reference, there is a parameter excludes (not officially declared in the API docs) where a list of features can be passed to be excluded by force. So, finally, forcing a given polyfill (bind) and avoiding a certain dependency (Object.createProperty) can be achieved with:
https://polyfill.io/v3/polyfill.js?features=Function.prototype.bind|always&excludes=Object.defineProperty