Skip to content
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

Iterator.prototype.constructor #60

Closed
zloirock opened this issue Oct 25, 2019 · 10 comments

Comments

@zloirock
Copy link
Contributor

@zloirock zloirock commented Oct 25, 2019

Now it's Object, maybe it should be Iterator?

@zloirock

This comment has been minimized.

Copy link
Contributor Author

@zloirock zloirock commented Oct 25, 2019

Theoretically, it could be a breaking change since could affect some old code:

[].values().constructor; // => Object

but unlikely that someone used it.

@michaelficarra

This comment has been minimized.

Copy link
Member

@michaelficarra michaelficarra commented Oct 25, 2019

Can you give some examples of why it would be useful to do this?

@ljharb

This comment has been minimized.

Copy link
Member

@ljharb ljharb commented Oct 25, 2019

Because [].values() instanceof Iterator should be true.

@zloirock

This comment has been minimized.

Copy link
Contributor Author

@zloirock zloirock commented Oct 25, 2019

JS objects contain .constructor and if it's already a part of the objects model, we should not forget about. Someone uses it for identification of objects, someone for subclassing. I'm not a fan of active usage of this property, but since some libraries and developers use it...

@michaelficarra

This comment has been minimized.

Copy link
Member

@michaelficarra michaelficarra commented Oct 25, 2019

@ljharb Iterator[@@hasInstance]?

@devsnek

This comment has been minimized.

Copy link
Member

@devsnek devsnek commented Oct 25, 2019

OrdinaryHasInstance of o and C uses C.prototype, not o.constructor. That being said, I don't really see any reason not to update the constructor property. I seriously doubt it would break anyone.

@michaelficarra

This comment has been minimized.

Copy link
Member

@michaelficarra michaelficarra commented Oct 25, 2019

Okay so is there any tangible benefit to making this change? Is it just for consistency?

@zloirock

This comment has been minimized.

Copy link
Contributor Author

@zloirock zloirock commented Oct 25, 2019

@michaelficarra since we must respect the concepts that are already used in the language?

@ljharb

This comment has been minimized.

Copy link
Member

@ljharb ljharb commented Oct 25, 2019

Consistency is a tangible benefit.

(yes, Symbol.hasInstance would help, but why when we could fix the prototype and the constructor)

@zloirock

This comment has been minimized.

Copy link
Contributor Author

@zloirock zloirock commented Oct 25, 2019

@ljharb @@hasInstance is not required, the prototype already added and your example already works with this proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.