-
Notifications
You must be signed in to change notification settings - Fork 64
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
Immutable Apply? #26
Comments
I discovered an actual implementation of this idea here WeTransfer@00d10c9 And another here (this one leverages immutability-helper) codedownio@e2f3976 |
It turns out |
Yeah I did it that way there because I totally agree with you - apply should be immutable. I'd support a PR changing apply to work this way. I don't think we need to pull in an immutable library to do it. Just making a shallow copy on the way down to the modified object should be enough. I'm not sure if inserted data should be cloned in or just moved in. I think if apply is immutable, taking a reference should be fine. (It should be one or the other.) In the tests the best way to verify this is to recursively call Object.freeze() on all the input data before calling apply. That way if anything is accidentally mutated, the tests will fail. |
FWIW I managed to solve this by monkey patching import produce from 'immer';
const originalApply = json0.type.apply;
json0.type.apply = (snapshot, op) =>
produce(snapshot, draftSnapshot => {
originalApply(draftSnapshot, op);
}); |
It would be really great if the library could apply ops such that instead of mutating the original data, a new object is created (with shared structure with the previous data object).
Related:
The text was updated successfully, but these errors were encountered: