fix(core): fix #1153, ZoneTask.toString should always be a string #1166
Conversation
JiaLiPassion
force-pushed
the
tostring
branch
from
30ed0cf
to
db24753
Compare
lib/zone.ts
Outdated
| @@ -1240,7 +1240,7 @@ const Zone: ZoneType = (function(global: any) { | |||
|
|
|||
| 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.
JiaLiPassion
force-pushed
the
tostring
branch
from
db24753
to
78ffe47
Compare
|
@gkalpak, thanks for the review, I didn't implement the |
JiaLiPassion
force-pushed
the
tostring
branch
from
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.