Skip to content

Commit

Permalink
fix(zone.js): read Symbol.species safely (#45369)
Browse files Browse the repository at this point in the history
We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
may be an object without a prototype (created through `Object.create(null)`); thus
`this.constructor` will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
object and copies promise properties into that object (within the `getOrCreateLoad`
function). The zone.js then checks if the resolved value has the `then` method and invokes
it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
properties of undefined (reading 'Symbol(Symbol.species)')`.

PR Close #45369
  • Loading branch information
arturovt authored and dylhunn committed Mar 25, 2022
1 parent 7a9c6f5 commit e2eaac3
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
13 changes: 11 additions & 2 deletions packages/zone.js/lib/common/promise.ts
Expand Up @@ -463,7 +463,15 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
onFulfilled?: ((value: R) => TResult1 | PromiseLike<TResult1>)|undefined|null,
onRejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>)|undefined|
null): Promise<TResult1|TResult2> {
let C = (this.constructor as any)[Symbol.species];
// We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
// may be an object without a prototype (created through `Object.create(null)`); thus
// `this.constructor` will be undefined. One of the use cases is SystemJS creating
// prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
// object and copies promise properties into that object (within the `getOrCreateLoad`
// function). The zone.js then checks if the resolved value has the `then` method and invokes
// it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
// properties of undefined (reading 'Symbol(Symbol.species)')`.
let C = (this.constructor as any)?.[Symbol.species];
if (!C || typeof C !== 'function') {
C = this.constructor || ZoneAwarePromise;
}
Expand All @@ -483,7 +491,8 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
}

finally<U>(onFinally?: () => U | PromiseLike<U>): Promise<R> {
let C = (this.constructor as any)[Symbol.species];
// See comment on the call to `then` about why thee `Symbol.species` is safely accessed.
let C = (this.constructor as any)?.[Symbol.species];
if (!C || typeof C !== 'function') {
C = ZoneAwarePromise;
}
Expand Down
26 changes: 26 additions & 0 deletions packages/zone.js/test/common/Promise.spec.ts
Expand Up @@ -116,6 +116,32 @@ describe(
expect(new MyPromise(() => {}).then(() => null) instanceof MyPromise).toBe(true);
});

it('should allow subclassing without Symbol.species if properties are copied (SystemJS case)',
() => {
let value: any = null;
const promise = Promise.resolve();
const systemjsModule = Object.create(null);

// We only copy properties from the `promise` instance onto the `systemjsModule` object.
// This is what SystemJS is doing internally:
// https://github.com/systemjs/systemjs/blob/main/src/system-core.js#L107-L113
for (const property in promise) {
const value: any = promise[property as keyof typeof promise];
if (!(value in systemjsModule) || systemjsModule[property] !== value) {
systemjsModule[property] = value;
}
}

queueZone.run(() => {
Promise.resolve().then(() => systemjsModule).then((v) => (value = v));
flushMicrotasks();
// Note: we want to ensure that the promise has been resolved. In this specific case
// the promise may resolve to different values in the browser and on the Node.js side.
// SystemJS runs only in the browser and it only needs the promise to be resolved.
expect(value).not.toEqual(null);
});
});

it('should allow subclassing with Symbol.species', () => {
class MyPromise extends Promise<any> {
constructor(fn: any) {
Expand Down

0 comments on commit e2eaac3

Please sign in to comment.