immer
immer copied to clipboard
immerjs
feature: Support copy-on-write in TypedArrays
This is an attempt to fix #696
This MR adds support for copy-on-write for TypedArrays (e.g. Uint16Array).
It correctly handles TypedArrays with the same underlying ArrayBuffer, so when copying 2 arrays with the same underlying buffer, the drafts will also have the same underlying buffer.
I was not sure how to handle ProxyTypes and Archtypes, so feel free to suggest another way to handle those. Also, do not hesitate listing things I could have done better 🙂
Limitations
I did not spend enough time to fix those. The known limitations/missing features are:
- No support for patches
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/subarray will not return a draft
Possible optimizations
- Not copying the array when the value is set to the same number
Testing
Aside from the added automated tests, I did not test this feature extensively.
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 1f703607d8c93a0ed54877bf71c6feab49397a31:
| Sandbox | Source |
|---|---|
| Immer sandbox | Configuration |
![codesandbox-ci[bot] avatar](https://avatars.githubusercontent.com/in/38409?v=4 "Published by a pub.dev verified publisher"){kind=small}
I have just noticed a mistake: instanceof and ArrayBuffer.isView will not work on the draft.
I'll add a test to ensure that those work and try to fix those
I have added a few more tests and commits, and changed the approach for what the proxy target should be in the case of typed arrays. I believe it should behave as expected
I'm looking forward to receiving feedback 🙂
Sorry, currently not having a lot of bandwidth. Since this is quite a meaty change, probably will only review it in January or something, hope that isn't too much of a problem.
I don't mind not having this feature in immer. I am wondering what do you mean by saying that this feature never landed. What feature never landed?
Oh. It is disappointing to see a PR closed because it was not merged without any more justification. Especially that there was no delay and no asks from me at any point. I feel like my time was wasted. https://github.com/immerjs/immer/issues/696 is not closed and still encourages implementing this idea.
Yeah sorry about that! Currently Immer has 5 million dependent packages on Github, and the idea has 3 upvotes, so it seems hard to justify at this moment to ship the additional code and increased maintenance complexity to everyone. At this moment it'd become a classic example of feature creep I think :)