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

Make shim-array.js less breaking world around #173

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

a-x-
Copy link

@a-x- a-x- commented Feb 8, 2017

closes #169 collections breaks Array.from with Iterables

@hthetiot
Copy link
Contributor

lgtm

@hthetiot hthetiot added this to the 5.0.x milestone May 5, 2017
@hthetiot hthetiot self-requested a review May 5, 2017 19:51
@hthetiot hthetiot self-assigned this Dec 7, 2017
@hthetiot
Copy link
Contributor

hthetiot commented Dec 7, 2017

Can you rebase on master @a-x- ?

@a-x-
Copy link
Author

a-x- commented Dec 7, 2017

done

@hthetiot
Copy link
Contributor

hthetiot commented Dec 7, 2017

Some test still break I will check why.

@hthetiot hthetiot modified the milestones: 5.1.x, 6.x Dec 28, 2017
@hthetiot
Copy link
Contributor

Need refactor to use internals if not extended.

@a-x-
Copy link
Author

a-x- commented Jan 15, 2018

could you add an example?

As I understand someone may keep native, say, window.Array, but change its Array.prototype.some method.
And you propose to replace whole Array In this case.

Am I correct?

@hthetiot
Copy link
Contributor

hthetiot commented Feb 16, 2018

@a-x- Sorry I really like to have that merge for fixing #196 but your changes break the tests.

As I understand someone may keep native, say, window.Array, but change its Array.prototype.some method.
And you propose to replace whole Array In this case.

No.

What I propose is make sure collections internally use his Array functions if there have a difference with Native Array or use private functions if needed.

@hthetiot
Copy link
Contributor

hthetiot commented Feb 16, 2018

@a-x- I cannot merge if test fails.

to run test:

npm run test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collections breaks Array.from with Iterables
2 participants