fix(core): fix #1153, ZoneTask.toString should always be a string#1166
fix(core): fix #1153, ZoneTask.toString should always be a string#1166mhevery merged 1 commit intoangular:masterfrom
Conversation
30ed0cf to
db24753
Compare
gkalpak
left a comment
There was a problem hiding this comment.
BTW, if you want to retain behavior similar to the original one, you could also implement valueOf() (to return a number).
(But, tbh, I can't think of any valid usecase for that 😁)
| public toString() { | ||
| if (this.data && typeof this.data.handleId !== 'undefined') { | ||
| return this.data.handleId; | ||
| return Object.prototype.toString.call(this.data.handleId); |
There was a problem hiding this comment.
I don't think this will have the expected behavior, as it will always return [object Number] (which would cause #341 again).
I think it should be this.data.handleId.toString() here.
There was a problem hiding this comment.
Yeah, you are right, thanks!
| expect(typeof macroTask.toString()).toEqual('string'); | ||
| expect(macroTask.toString()).toEqual(id.toString()); | ||
| expect(typeof microTask.toString()).toEqual('string'); | ||
| }); |
There was a problem hiding this comment.
I think it would be valueable to also verify that two different macrotasks have different string representations (just in case).
There was a problem hiding this comment.
Got it, case added.
db24753 to
78ffe47
Compare
|
@gkalpak, thanks for the review, I didn't implement the |
78ffe47 to
654da32
Compare
| expect(macroTask1Str).toEqual(id1.toString()); | ||
| expect(typeof macroTask2Str).toEqual('string'); | ||
| expect(macroTask2Str).toEqual(id2.toString()); | ||
| if (macroTask1.data && typeof macroTask1.data.handleId === 'number') { |
There was a problem hiding this comment.
Yeah, the reason is this case will also run in nodejs environment, in nodejs the handleId will be a TimerObject, in that case, we will not check their equality.
fix #1153.