fix(browser): make Object.defineProperty patch safer (#392)

marclaval · mhevery · commit 597c634467fb · 2016-08-11T11:28:25.000-07:00
Fixes #391
diff --git a/lib/browser/define-property.ts b/lib/browser/define-property.ts
@@ -14,10 +14,11 @@ export function propertyPatch() {
     if (isUnconfigurable(obj, prop)) {
       throw new TypeError('Cannot assign to read only property \'' + prop + '\' of ' + obj);
     }
+    const originalConfigurableFlag = desc.configurable;
     if (prop !== 'prototype') {
       desc = rewriteDescriptor(obj, prop, desc);
     }
-    return _defineProperty(obj, prop, desc);
+    return _tryDefineProperty(obj, prop, desc, originalConfigurableFlag);
   };
 
   Object.defineProperties = function (obj, props) {
@@ -46,8 +47,9 @@ export function propertyPatch() {
 };
 
 export function _redefineProperty(obj, prop, desc) {
+  const originalConfigurableFlag = desc.configurable;
   desc = rewriteDescriptor(obj, prop, desc);
-  return _defineProperty(obj, prop, desc);
+  return _tryDefineProperty(obj, prop, desc, originalConfigurableFlag);
 };
 
 function isUnconfigurable (obj, prop) {
@@ -65,4 +67,23 @@ function rewriteDescriptor (obj, prop, desc) {
   return desc;
 }
 
+function _tryDefineProperty (obj, prop, desc, originalConfigurableFlag) {
+  try {
+    return _defineProperty(obj, prop, desc);
+  }
+  catch(e) {
+    if (desc.configurable) {
+      // In case of errors, when the configurable flag was likely set by rewriteDescriptor(), let's retry with the original flag value
+      if (typeof originalConfigurableFlag == 'undefined') {
+        delete desc.configurable;
+      } else {
+        desc.configurable = originalConfigurableFlag;
+      }
+      return _defineProperty(obj, prop, desc);
+    } else {
+      throw e;
+    }
+  }
+}
+
 
diff --git a/test/browser/define-property.spec.ts b/test/browser/define-property.spec.ts
@@ -0,0 +1,8 @@
+describe('defineProperty', function () {
+
+  it('should not throw when defining length on an array', function () {
+    var someArray = [];
+    expect(() => Object.defineProperty(someArray, 'length', {value: 2, writable: false})).not.toThrow();
+  });
+
+});
diff --git a/test/browser_entry_point.ts b/test/browser_entry_point.ts
@@ -13,6 +13,7 @@ import './test-env-setup';
 // List all tests here:
 import './common_tests';
 import './browser/browser.spec';
+import './browser/define-property.spec';
 import './browser/element.spec';
 import './browser/FileReader.spec';
 import './browser/HTMLImports.spec';
