es-shims / es5-shim

ECMAScript 5 compatibility shims for legacy (and modern) JavaScript engines
MIT License
7.12k stars 899 forks source link

errors with Object.defineProperty under ie8 since 4.6.3 #479

Closed krzyko closed 2 years ago

krzyko commented 2 years ago

Since version 4.6.3 es5-shim seems broken under IE 8. With this browser 4.6.2 test suite gives 16 failures, while later 4.6.3 and 4.6.4 gives 351 failures and most of them are Type Error failures.

Browser console on 4.6.3+ tests gives me error: SCRIPT445: Object doesn't support this action. es5-shim.js (135,9)

Which is probably because IE 8 (as written here: https://caniuse.com/?search=es5) "has virtually no ES5 support, but does support Object.defineProperty [...]" which is a conditional trigger for IE8-incompatible code since this commit: https://github.com/es-shims/es5-shim/commit/780abbaa0efea8e5eb713e6cda05c38c04d7f12a

Also https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty states that: "In Internet Explorer 8, this was only supported on DOM objects and with some non-standard behaviors. This was later fixed in Internet Explorer 9."

Also there are some notes: here: https://kangax.github.io/compat-table/es5/#define-property-ie-note and here: https://kangax.github.io/compat-table/es5/#get-own-property-descriptor-ie-note

CircleTan commented 2 years ago

你的邮件我已收到了,谢谢。

ljharb commented 2 years ago

Thanks, i should be able to get this fixed today.

ljharb commented 2 years ago

@krzyko can you confirm that if you change the if ($Object.defineProperty) { line to if ($Object.defineProperty && supportsDescriptors) {, it fixes your issue?

ljharb commented 2 years ago

I've verified that IE 8's test failures drop from "basically all of them" to "just 18" with this fix, so I'll go with it.

krzyko commented 2 years ago

I've noticed that supportsDescriptors already tests for $Object.defineProperty so changing if ($Object.defineProperty) { to if (supportsDescriptors) { will simplify code a bit. But anyways, mentioned commit has fixed my issue. Thanks.

ljharb commented 2 years ago

Very true :-) but this way will be a good reminder for me. Thanks for reporting!