Object.prototype.toString: split Symbol.toStringTag tests into separate files #3856
Conversation
| var toString = Object.prototype.toString; | ||
|
|
||
| var genFn = function* () {}; | ||
| assert.sameValue(toString.call(gen), '[object GeneratorFunction]'); |
| assert.sameValue(toString.call(gen), '[object GeneratorFunction]'); | ||
|
|
||
| var gen = genFn(); | ||
| assert.sameValue(toString.call(gen), '[object Generator]'); |
| Object.defineProperty(genProto, Symbol.toStringTag, { | ||
| get: function() { return {}; }, | ||
| }); | ||
| assert.sameValue(toString.call(gen), '[object Iterator]'); |
There was a problem hiding this comment.
this line was corrected - it was "Object" but it should be "Iterator"
| Object.defineProperty(mapIterProto, Symbol.toStringTag, { | ||
| get: function() { return new String('ShouldNotBeUnwrapped'); }, | ||
| }); | ||
| assert.sameValue(toString.call(mapIter), '[object Iterator]'); |
There was a problem hiding this comment.
this line was corrected - it was Object but should be Iterator
| var toString = Object.prototype.toString; | ||
|
|
||
| var promise = new Promise(function () {}); | ||
| assert.sameValue(toString.call(promise), '[object Promise]'); |
| Object.defineProperty(mapIterProto, Symbol.toStringTag, { | ||
| get: function() { return new String('ShouldNotBeUnwrapped'); }, | ||
| }); | ||
| assert.sameValue(toString.call(mapIter), '[object Iterator]'); |
There was a problem hiding this comment.
this line was corrected from Object to Iterator
| assert.sameValue(toString.call(arrIter), '[object Array Iterator]'); | ||
|
|
||
| Object.defineProperty(arrIterProto, Symbol.toStringTag, {value: null}); | ||
| assert.sameValue(toString.call(arrIter), '[object Iterator]'); |
There was a problem hiding this comment.
this line was corrected from Object to Iterator
There was a problem hiding this comment.
No, the original test was correct. The line right above this one redefines %IteratorPrototype%'s @@toStringTag to be null, so it should be [object Object]. Please also double check the other changed tests.
There was a problem hiding this comment.
It redefines ArrayIteratorPrototype, not IteratorPrototype, i believe.
There was a problem hiding this comment.
Quite right. Sorry, my reasoning was wrong but the original test is still correct.
Object.defineProperty(arrIterProto, Symbol.toStringTag, {value: null}); sets the value of @@toStringTag to null. Steps 14 and 15 of Object.prototype.toString are:
- Let tag be ? Get(O, @@toStringTag).
- If tag is not a String, set tag to builtinTag.
So if it finds null on arrIterProto, finds it unusable, and returns [object Object].
There was a problem hiding this comment.
ahh, ok, without checking the spec or a repl I’d assumed it ignored it instead of breaking the chain lookup. I’ll fix all the relevant lines later this evening.
There was a problem hiding this comment.
Updated - now it verifies its Object when unusable, then deletes it, and then verifies it’s Iterator.
ljharb
force-pushed
the
split-tests
branch
3 times, most recently
from
bcf2add
to
b698fe5
Compare
|
@syg ping on taking another look here |
ljharb
force-pushed
the
split-tests
branch
4 times, most recently
from
835bcfb
to
bbdd74b
Compare
ljharb
force-pushed
the
split-tests
branch
2 times, most recently
from
2d45b32
to
d9e984d
Compare
|
I agreed to review this one. Sorry it has taken me so long to get to it. I expect to have time this week. |
See #3855 (comment)
I also added some test coverage that was previously missing.