fix: readonly property should not be patched (#860)

Author: JiaLiPassion

lib/common/utils.ts

@@ -33,6 +33,10 @@ export function patchPrototype(prototype: any, fnNames: string[]) {
     const name = fnNames[i];
     const delegate = prototype[name];
     if (delegate) {
+      const prototypeDesc = Object.getOwnPropertyDescriptor(prototype, name);
+      if (!isPropertyWritable(prototypeDesc)) {
+        continue;
+      }
       prototype[name] = ((delegate: Function) => {
         const patched: any = function() {
           return delegate.apply(this, bindArguments(<any>arguments, source + '.' + name));
@@ -44,6 +48,22 @@ export function patchPrototype(prototype: any, fnNames: string[]) {
   }
 }
 
+export function isPropertyWritable(propertyDesc: any) {
+  if (!propertyDesc) {
+    return true;
+  }
+
+  if (propertyDesc.writable === false) {
+    return false;
+  }
+
+  if (typeof propertyDesc.get === 'function' && typeof propertyDesc.set === 'undefined') {
+    return false;
+  }
+
+  return true;
+}
+
 export const isWebWorker: boolean =
     (typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope);
 

test/common/util.spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {patchMethod, patchProperty, zoneSymbol} from '../../lib/common/utils';
+import {patchMethod, patchProperty, patchPrototype, zoneSymbol} from '../../lib/common/utils';
 
 describe('utils', function() {
 
@@ -79,4 +79,167 @@ describe('utils', function() {
       expect(!desc.get).toBeTruthy();
     });
   });
+
+  describe('patchPrototype', () => {
+    it('non configurable property desc should be patched', () => {
+      'use strict';
+      const TestFunction: any = function() {};
+      const log: string[] = [];
+      Object.defineProperties(TestFunction.prototype, {
+        'property1': {
+          value: function Property1(callback: Function) {
+            Zone.root.run(callback);
+          },
+          writable: true,
+          configurable: true,
+          enumerable: true
+        },
+        'property2': {
+          value: function Property2(callback: Function) {
+            Zone.root.run(callback);
+          },
+          writable: true,
+          configurable: false,
+          enumerable: true
+        }
+      });
+
+      const zone = Zone.current.fork({name: 'patch'});
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property1(() => {
+          log.push('property1' + Zone.current.name);
+        });
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property1<root>', 'property2<root>']);
+      log.length = 0;
+
+      patchPrototype(TestFunction.prototype, ['property1', 'property2']);
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property1(() => {
+          log.push('property1' + Zone.current.name);
+        });
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property1patch', 'property2patch']);
+    });
+
+    it('non writable property desc should not be patched', () => {
+      'use strict';
+      const TestFunction: any = function() {};
+      const log: string[] = [];
+      Object.defineProperties(TestFunction.prototype, {
+        'property1': {
+          value: function Property1(callback: Function) {
+            Zone.root.run(callback);
+          },
+          writable: true,
+          configurable: true,
+          enumerable: true
+        },
+        'property2': {
+          value: function Property2(callback: Function) {
+            Zone.root.run(callback);
+          },
+          writable: false,
+          configurable: true,
+          enumerable: true
+        }
+      });
+
+      const zone = Zone.current.fork({name: 'patch'});
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property1(() => {
+          log.push('property1' + Zone.current.name);
+        });
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property1<root>', 'property2<root>']);
+      log.length = 0;
+
+      patchPrototype(TestFunction.prototype, ['property1', 'property2']);
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property1(() => {
+          log.push('property1' + Zone.current.name);
+        });
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property1patch', 'property2<root>']);
+    });
+
+    it('readonly property desc should not be patched', () => {
+      'use strict';
+      const TestFunction: any = function() {};
+      const log: string[] = [];
+      Object.defineProperties(TestFunction.prototype, {
+        'property1': {
+          get: function() {
+            if (!this._property1) {
+              this._property1 = function Property2(callback: Function) {
+                Zone.root.run(callback);
+              };
+            }
+            return this._property1;
+          },
+          set: function(func: Function) {
+            this._property1 = func;
+          },
+          configurable: true,
+          enumerable: true
+        },
+        'property2': {
+          get: function() {
+            return function Property2(callback: Function) {
+              Zone.root.run(callback);
+            };
+          },
+          configurable: true,
+          enumerable: true
+        }
+      });
+
+      const zone = Zone.current.fork({name: 'patch'});
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property1(() => {
+          log.push('property1' + Zone.current.name);
+        });
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property1<root>', 'property2<root>']);
+      log.length = 0;
+
+      patchPrototype(TestFunction.prototype, ['property1', 'property2']);
+
+      zone.run(() => {
+        const instance = new TestFunction();
+        instance.property1(() => {
+          log.push('property1' + Zone.current.name);
+        });
+        instance.property2(() => {
+          log.push('property2' + Zone.current.name);
+        });
+      });
+      expect(log).toEqual(['property1patch', 'property2<root>']);
+    });
+  });
 });