Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

feat(patch): fix #499, let promise instance toString active like native #734

Merged
merged 1 commit into from
Apr 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/browser/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {patchTimer} from '../common/timers';
import {patchFuncToString} from '../common/to-string';
import {patchFuncToString, patchObjectToString} from '../common/to-string';
import {findEventTask, patchClass, patchEventTargetMethods, patchMethod, patchPrototype, zoneSymbol} from '../common/utils';

import {propertyPatch} from './define-property';
Expand Down Expand Up @@ -154,6 +154,8 @@ if (_global['navigator'] && _global['navigator'].geolocation) {

// patch Func.prototype.toString to let them look like native
patchFuncToString();
// patch Object.prototype.toString to let them look like native
patchObjectToString();

// handle unhandled promise rejection
function findPromiseRejectionHandler(evtName: string) {
Expand Down
10 changes: 10 additions & 0 deletions lib/common/to-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,13 @@ export function patchFuncToString() {
return originalFunctionToString.apply(this, arguments);
};
}

export function patchObjectToString() {
const originalObjectToString = Object.prototype.toString;
Object.prototype.toString = function() {
if (this instanceof Promise) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the reason why Promise shims are in use because Promise is not available in all common browsers today? So wouldn't this line throw

ReferenceError: Can't find variable: Promise

for projects & browsers that haven't window.Promise (neither native nor shim)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zone.js has provided Promise shim, so it will be ok even some browser or other environment doesn't provide native Promise

return '[object Promise]';
}
return originalObjectToString.apply(this, arguments);
};
}
4 changes: 3 additions & 1 deletion lib/node/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import './events';
import './fs';

import {patchTimer} from '../common/timers';
import {patchFuncToString} from '../common/to-string';
import {patchFuncToString, patchObjectToString} from '../common/to-string';
import {findEventTask, patchMacroTask, patchMicroTask, zoneSymbol} from '../common/utils';

const set = 'set';
Expand All @@ -38,6 +38,8 @@ handleUnhandledPromiseRejection();

// patch Function.prototyp.toString
patchFuncToString();
// patch Object.prototyp.toString
patchObjectToString();

// Crypto
let crypto: any;
Expand Down
4 changes: 4 additions & 0 deletions test/common/Promise.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ describe(
expect(String(Promise).indexOf('[native code]') >= 0).toBe(true);
});

it('should use native toString for promise instance', () => {
expect(Object.prototype.toString.call(Promise.resolve())).toEqual('[object Promise]');
});

it('should make sure that new Promise is instance of Promise', () => {
expect(Promise.resolve(123) instanceof Promise).toBe(true);
expect(new Promise(() => null) instanceof Promise).toBe(true);
Expand Down