-
Notifications
You must be signed in to change notification settings - Fork 461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update constructor and Symbol.toStringTag properties on Iterator.prototype #3970
Conversation
test/built-ins/Iterator/prototype/Symbol.toStringTag/weird-setter.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coming back to this after the winter break, I don't see a revert of this on the agenda among the other iterator topics for February 2024, so I think we should assume it's not changing in the short term and should merge it.
59176fc
to
d6572a7
Compare
enumerable: false, | ||
configurable: true, | ||
}); | ||
verifyConfigurable(Iterator.prototype, Symbol.toStringTag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test and the corresponding one for constructor
property are broken. verifyConfigurable()
calls isConfigurable()
that deletes the property. So, the subsequent parts of this test fail because the property is not there anymore.
If you want to use verifyConfigurable()
then it should be the last statement of the test.
|
||
// 1. If _this_ is not an Object, then | ||
// 1. Throw a *TypeError* exception. | ||
assert.throws(() => set.call(undefined, '')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test and the corresponding one for constructor
property are broken. The first argument of assert.throws()
should be the expected error (i.e. TypeError
in this case).
@iamstolis Thanks for catching these, would you mind opening an issue, otherwise the comments will probably get lost 😄 |
Done: #3993. |
Updates tests for these two properties to match tc39/proposal-iterator-helpers#287.