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

feat: Always use remove command in generateArrayPatches() #415

Merged

Conversation

jsdevtom
Copy link
Contributor

@jsdevtom jsdevtom commented Sep 6, 2019

Fixes #208
Please look for things I haven't tested. I found the part of the code that I was reading very hard to understand due to the lack of descriptive names and unexpected swapping of those variable names that I did understand (see https://github.com/immerjs/immer/blob/master/src/patches.js#L15). For this reason I am not sure that I have not introduced unexpected behavior.

@mweststrate
Copy link
Collaborator

Looks great at first sight! Will study it in more detail when cutting next release

@mfix22
Copy link

mfix22 commented Sep 11, 2019

Such a cool PR! Fixing bugs by deleting code. Looks like the right move 🎉

@mweststrate
Copy link
Collaborator

Looks correct to me!

@mweststrate mweststrate merged commit ac61b8d into immerjs:master Sep 11, 2019
@aleclarson
Copy link
Member

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Patch for array.pop() replaces "length"
4 participants