I'm not sure if this is a issue with immer, which conceptually I love, or I'm using it wrong so I'd be interested to hear thoughts on this.
I'm trying to plug immer into a relatively small React/Redux project I have on the go. In reducers it all seems to work exactly as expected as the inputs are essentially a merge of the old state and an action payload.
However I then started looking at some of my selectors and thought it would be applicable there since the output from a selector is frequently a mutated version of a portion of the state, with lookups and/or references plugged in or one-time evaluations executed and saved. Again, that more or less seemed to work until I got to the part where a reference to a different part of the state tree was being attached to my new state. Then I got the following error,
Uncaught TypeError: Cannot assign to read only property '_id' of object '#<Object>'
In my case the objects are from a Mongo DB and so _id is the first field in the object.
To express this in more familiar terms, given an example of users and todos where each todo belongs to a user, my selector might be called getUserAndTodos and would return both the original user record and include a list of their todos taken from the general todo list in the global state, as well as maybe an 'allDone' flag to indicate whether that user has completed all their todos.
Given that I am essentially modifying (by addition) the user field from state, I thought this was a good place to use 'produce'. If 'produce' returns just the user record unmodified, then it's fine. If it additionally adds the 'allDone' field calculated from the users list of todos then it's also fine, but if I filter the todo list from state and add a list of the users todos to the user object I get the error above. Note, that if I turn off autofreeze then I don't see the error.
My guess is that because I am plugging in already frozen objects into a member of the 'new state' that somehow it's getting itself confused and trying to copy them over, rather than noticing it can just use a reference? More so because I am just referencing them and they aren't a direct argument to 'produce' itself. Or perhaps more like I've entirely misunderstood how this is supposed to work.
If necessary I can try and get a sandbox up that shows the error but I thought I would ask first in case I'm just doing it all wrong.
The partial stack trace of the error is below if it's of any use.
Thanks
```×
TypeError: Cannot assign to read only property '_id' of object '#
226 | base[key] = finalize(base[key]);
227 | }return freeze(base);
228 | }
229 | }
View compiled
finalize
node_modules/immer/dist/immer.js:226
223 | var proto = Object.getPrototypeOf(base);
224 | if (proto === null || proto === Object.prototype) {
225 | for (var key in base) {
226 | base[key] = finalize(base[key]);
227 | }return freeze(base);
228 | }
229 | }
View compiled
finalizeObject
node_modules/immer/dist/immer.js:237
234 | var copy = state.copy;
235 | var base = state.base;
236 | for (var prop in copy) {
237 | if (copy[prop] !== base[prop]) copy[prop] = finalize(copy[prop]);
238 | }
239 | return freeze(copy);
240 | }
View compiled
finalize
node_modules/immer/dist/immer.js:212
209 | if (state.finalized === true) return state.copy;
210 | state.finalized = true;
211 | if (Array.isArray(state.base)) return finalizeArray(state);
212 | return finalizeObject(state);
213 | } else return state.base;
214 | } else if (base !== null && (typeof base === "undefined" ? "undefined" : _typeof(base)) === "object") {
215 | // If finalize is called on an object that was not a proxy, it means that it is an object that was not there in the original
View compiled
produce
node_modules/immer/dist/immer.js:258
255 | //values either than undefined will trigger warning;
256 | !Object.is(maybeVoidReturn, undefined) && console.warn("Immer callback expects no return value. However " + (typeof maybeVoidReturn === "undefined" ? "undefined" : _typeof(maybeVoidReturn)) + " was returned");
257 | // and finalize the modified proxy
258 | var res = finalize(rootClone);
259 | // revoke all proxies
260 | revocableProxies.forEach(function (p) {
261 | return p.revoke();
View compiled
web2wire
All 13 comments
Please include something small and testable
mweststrate
on 15 Jan 2018
Will do. Do you know of an immer sandbox example I can fork as it's much easier than getting one working from scratch.
web2wire
on 15 Jan 2018
You can pick https://codesandbox.io/s/k557jo8jl7. But basically any sandbox
will work, just import from "immer" and let sandbox do its magic
Op ma 15 jan. 2018 om 15:30 schreef Philip Coombes <[email protected]
:
Will do. Do you know of an immer sandbox example I can fork as it's much
easier than getting one working from scratch.—
You are receiving this because you commented.Reply to this email directly, view it on GitHub
https://github.com/mweststrate/immer/issues/66#issuecomment-357698229,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhKw91VwvZni0vEo5cCeiRpvYZ-oCks5tK2D8gaJpZM4Rec_4
.
mweststrate
on 15 Jan 2018
Closing for now until reproduced.
For now it seems, given the description, that this is case of accidentally modifying the current state, which is exactly the thing freezing tries to detect and protect from.
mweststrate
on 16 Jan 2018
Reopened: This issue seems to occur when adding already immutable data to the tree. Will fix
mweststrate
on 16 Jan 2018
@web2wire version 0.8.1 might fix this, could you verify?
mweststrate
on 16 Jan 2018
Ah. good, at least in that I'm not imagining things. I am having trouble reproducing it in a sandbox, though I am gradually bringing it up to the complexity of my app itself.
web2wire
on 16 Jan 2018
Crossed over post. Yep, I'll check it shortly.
web2wire
on 16 Jan 2018
Yeah, not sure if it is the same issue (hard to deduct that from your description), but at least the symptoms are the same :)
mweststrate
on 16 Jan 2018
Looks good so far, no crashes yet, and it's normally immediate. Thanks for addressing this so quickly.
I hope you don't mind if I ask you a quick related question here, rather than raise a new issue, though please feel free to delete or move as appropriate.
Should you be able to mutate an array or object member of a 'produce'd object? This is the sandbox I was working on (https://codesandbox.io/s/n9vn1v50pp) and at the bottom I have various tests. The ones that are commented out cause errors, but the ones that remain don't. Is this the correct behaviour? I have a very vague recollection that I may have read that immer doesn't prevent this kind of secondary modification but I can't find anything in the docs and may be getting my wires crossed with something else.
web2wire
on 16 Jan 2018
Closing this issue as you are indeed raising new ones but the answer is kinda complicated:
- If autofreeze is disabled: you can modify the produced state
- If autofreeze is enabled: it depends (and one could make good arguments for both the current behavior and different semantics), objects that already existed in the state and were modified are frozen (so the objects that are proxied). But completely new objects that are introduced into the tree are left as is; freezing data trees deeply is an expensive process and currently the goal is more to prevent as best as we could, yet cheaply, against accidental mistakes, rather than guaranteeing that everything in the tree is frozen
mweststrate
on 17 Jan 2018
After putting this on paper, it made less sense then before :-P. Next version will automatically freeze newly added, non-proxied data as well
mweststrate
on 18 Jan 2018
Thanks for the answers. I look forward to the continued development of immer, it's a great little package!
web2wire
on 18 Jan 2018
Related issues
phryneas
·
5Comments
Knacktus
·
5Comments
psantos10
·
5Comments
cakoose
·
6Comments
toli444
·
6Comments
Most helpful comment
After putting this on paper, it made less sense then before :-P. Next version will automatically freeze newly added, non-proxied data as well