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

Ability to set Array.prototype as prototype for array-like object wrappers #721

Merged

Conversation

lahma
Copy link
Collaborator

@lahma lahma commented Apr 11, 2020

  • when we see object implementing ICollection or read-only like interface it should be safe to access as array like
  • should not assign indexers which are invalid for usage (property is string and indexer uses int)
  • assign Array.prototype to array-like objects allowing filter etc, only used when object doesn't have anything suitable itself

@czema
Copy link
Contributor

czema commented Apr 11, 2020

This is great, thanks a lot.

I can't quite tell, but this doesn't seem to support IEnumerable. I realize that some array features make no sense in IEnumerable, but they don't make sense on ReadonlyCollection either.

@lahma
Copy link
Collaborator Author

lahma commented Apr 11, 2020

The key here is that the accessed object needs to be ICollection, there are very few things that really are only IEnumerable, all (that I can think of) BCL collection types implement some more functional interface than just IEnumerable. And yes, some of the prototypes are mutating which don't make much sense for read-only, but filter, some, every etc can be nice to have.

I don't like much the magic here. LINQ/JS/CLR object bridge has some conceptual problems I guess.

@lahma lahma changed the title Set Array.prototype as prototype for array-like object wrappers Ability to set Array.prototype as prototype for array-like object wrappers Apr 11, 2020
@lahma lahma merged commit dce29b4 into sebastienros:dev Apr 13, 2020
@lahma lahma deleted the array-prototype-for-array-like-object-wrappers branch April 13, 2020 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants