fix(zone.js): swallow the error when the element callback is not patchable (#45400)

arturovt · dylhunn · commit 4ea70e36b998 · 2022-03-25T16:31:09.000-07:00
The `patchCallbacks` is used for patching the `document.registerElement` and `customElements.define`. We explicitly wrap the patching code into try-catch since callbacks may be already patched by other web components frameworks (e.g. LWC), and they make those properties non-writable. This means that patching callback will throw an error `cannot assign to read-only property`. See this code as an example: https://github.com/salesforce/lwc/blob/master/packages/@lwc/engine-core/src/framework/base-bridge-element.ts#L180-L186 We don't want to stop the application rendering if we couldn't patch some callback, e.g. `attributeChangedCallback`. PR Close #42546 PR Close #45400
diff --git a/packages/zone.js/lib/browser/browser-util.ts b/packages/zone.js/lib/browser/browser-util.ts
@@ -17,16 +17,29 @@ export function patchCallbacks(
       callbacks.forEach(function(callback) {
         const source = `${targetName}.${method}::` + callback;
         const prototype = opts.prototype;
-        if (prototype.hasOwnProperty(callback)) {
-          const descriptor = api.ObjectGetOwnPropertyDescriptor(prototype, callback);
-          if (descriptor && descriptor.value) {
-            descriptor.value = api.wrapWithCurrentZone(descriptor.value, source);
-            api._redefineProperty(opts.prototype, callback, descriptor);
+        // Note: the `patchCallbacks` is used for patching the `document.registerElement` and
+        // `customElements.define`. We explicitly wrap the patching code into try-catch since
+        // callbacks may be already patched by other web components frameworks (e.g. LWC), and they
+        // make those properties non-writable. This means that patching callback will throw an error
+        // `cannot assign to read-only property`. See this code as an example:
+        // https://github.com/salesforce/lwc/blob/master/packages/@lwc/engine-core/src/framework/base-bridge-element.ts#L180-L186
+        // We don't want to stop the application rendering if we couldn't patch some
+        // callback, e.g. `attributeChangedCallback`.
+        try {
+          if (prototype.hasOwnProperty(callback)) {
+            const descriptor = api.ObjectGetOwnPropertyDescriptor(prototype, callback);
+            if (descriptor && descriptor.value) {
+              descriptor.value = api.wrapWithCurrentZone(descriptor.value, source);
+              api._redefineProperty(opts.prototype, callback, descriptor);
+            } else if (prototype[callback]) {
+              prototype[callback] = api.wrapWithCurrentZone(prototype[callback], source);
+            }
           } else if (prototype[callback]) {
             prototype[callback] = api.wrapWithCurrentZone(prototype[callback], source);
           }
-        } else if (prototype[callback]) {
-          prototype[callback] = api.wrapWithCurrentZone(prototype[callback], source);
+        } catch {
+          // Note: we leave the catch block empty since there's no way to handle the error related
+          // to non-writable property.
         }
       });
     }
