bug report

Affected Package

zone.js

Is this a regression?

Yes, but super back in the past! It broke with v0.6.24.

Description

ZoneJS has this super old legacy Object.defineProperty patch that basically partially re-implements
the Object.defineProperty functionality in order to resolve some custom elements issue. I'm confident that these issues are no longer surfacing and patching Object.defineProperty is not the right solution these days.

The issue is that this overwritten Object.defineProperty function behaves different to the native implementation. i.e. by default defined properties are not configurable. In ZoneJS, with the patched version, properties are always configurable. This means that:

1. The default configurable property descriptor attribute is not respected.
2. Defined properties are by default always configurable: true. This doesn't match the native implementation.

Due to these inconsistencies, ZoneJS could potentially hide app issues that would usually cause errors in production. e.g. when using differential loading, legacy patches might be used in tests, but not always in production. So, calling defineProperty multiple times in your app might work with the legacy patches in tests and legacy browsers, but otherwise cause a Uncaught TypeError: Cannot redefine property: X error in evergreen browsers.

We saw this issue in the Angular Material test harnesses: angular/components#19440

Here is the commit that introduced this logic back in the old repository:
angular/zone.js@383b479. And here is the commit that caused the incorrect configurable default, and the flag to be not respected at all: angular/zone.js@7b7258b.

Minimal Reproduction

const x = {}
Object.defineProperty(x, 'defaultPrevented', { get: () => true, configurable: false });
console.error(Object.getOwnPropertyDescriptor(x, 'defaultPrevented'));

This prints configurable: false for the defaultPrevented property without ZoneJS. If the ZoneJS legacy patches are loaded, this is not the case though and configurable is always true.

Exception or Error

Object.defineProperty behaves different depending on whether ZoneJS legacy patches are loaded or not. ZoneJS could hide app failures in tests as Object.defineProperty never sets configurable: false while in reality, defining a non-configurable property again, causes a Cannot redeclare property X runtime error in browsers.

Your Environment

Angular Version:

Angular v10.0.0-rc.2.
zonejs: 0.10.

Anything else relevant?

We should audit if we still need this legacy patch. I'm confident, we wouldn't. I'm hoping we can remove this as it's prone to inconsistencies with the native Object.defineProperty behavior.