Clarification about documentation

[ghost]

I want to ask clarification about documentation.

In pitfalls section you state:

That is, no object should appear twice in the tree, and there should be no circular references.

Can you explain what you mean? For example if I had state like

myState = {
items: [{itemID:1}, {itemID:2}, {itemID:3}]
filteredItems:[]
}

and then say filteredItemswould store all items from itemsexcept say items with ID 3, would that violate the sentence I cited from your docs? Because in that case say same objects (objects without ID 3) would be stored both under key itemsand under key filteredItems.

e.g if I computed next state as:

nextState = produce(myState, (draft)=>{
 draft.filteredItems= draft.items.filter(x=>x.itemID!=3)
})

[mweststrate]

Correct, that violates the rule indeed, as your state is no longer a directed acyclic tree. Simple rule of thumb: if JSON.parse(JSON.stringify(state)) produces not the same thing, your state is not a tree.

Since your items have id's already, just store the ID's in filteredItems. (And turn items into a key/value map for fast lookups).

That being said, storing filterItems in your state is imho an anti pattern; information that can be derived doesn't need to be stored in the state. It prevents bugs where you add for example an item to items but forget to update filteredItems as well

[ghost]

Simple rule of thumb: if JSON.parse(JSON.stringify(state)) produces not the same thing, your state is not a tree.

Can you explain what you mean here? JSON.parse/stringify correctly worked on my example above.

Thanks for response. Maybe it would be useful to add those explanations in the "Pitfalls" section too, as I think it is important and some may overlook/not understand.

[mweststrate]

@davidmal200 no, cloning will change the structure of your state in your example, simple demonstration:

const x = { id: 1, text: "hello" }
const items = { items: [x], filteredItems: [x] }
items.items[0].text = "hello2"
items.filteredItems[0].text
// 'hello2' (correct)

items2 = JSON.parse(JSON.stringify(items))
items2.items[0].text = "hello"
items2.filteredItems[0].text
// 'hello2' (incorrect)

You serialized a graph, but you got back a tree; and your filtered items are now inconsistent with your items. One of the core principles of Redux for example is that the state must be a tree as doing otherwise will break assumptions other tools make on your state, not just Immer, but also Redux or React devtools etc etc. I'm pretty sure there is quite some literature online covering why the state should be organized as tree, and what some best practices are around designing state trees.

[ghost]

@mweststrate Ah do you mean in items2 now the filteredItems doesn't have the reference to [x]? Although content wise both items2 and items are same right?

[mweststrate]

Yes, but semantics are different, first you got one object, now you got two. Checks for reference equality will fail in items2, etc

Op ma 24 sep. 2018 om 10:00 schreef davidmal200 notifications@github.com:

@mweststrate https://github.com/mweststrate Ah do you mean in items2 now the filteredItems doesn't have the reference to [x]? Although content wise both items2 and items are same right?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/mweststrate/immer/issues/201#issuecomment-423900226, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvGhOg2budOGdejp7WCYqe5z1FMxKBnks5ueJE4gaJpZM4W10uY .

[ghost]

Yes items2.items[0] !== items2.filteredItems[0] while items.items[0] == items.filteredItems[0]. You mean that right?

I would still suggest you add such examples in Pitfalls section as it would be more clear what is allowed and what not. There is also some restriction on dates I remember small code examples wouldn't hurt there either.

Thanks.

[mweststrate]

if a small PR could elaborate on it, that would be welcome, (ideally providing a good external link). But explaining these concepts are kinda outside the scope of immer, immer is tool you might pick because it is useful when opting for an immutable state tree architecture. But explaining why that is a good architecture or understanding the rules of that game are kinde outside of Immer, as typically you answered those first.

One uses Immer as result of the architecture choice (like Redux, or React-copy-on-write, bey etc). If using immer influences your state tree design, then you are using immer either for the wrong reason, or you didn't think the design through. In your case specific I would first take a second look at at how the design your state shape, as your current setup is quite unidiomatic. Googling for something like "designing immutable state tree" or "redux store structure best practice" or something like that will probably lead you to something useful that sheds some light on the background of immutable state trees.

[ghost]

ok, it was my suggestion just. adding explanations never hurts. For example I wasn't 100% sure what

no object should appear twice in the tree

meant-though I had suspicion as I elaborated.

So finally as you said if state fails JSON.parse/stringify test (e.g. produces objects with same content but different semantics say) I shouldn't use immer right? (If yes, then maybe docs should at least mention it?)

Ok thanks and good luck.

[mweststrate]

Yeah, as said, PR is welcome :-) if there is way of expressing this that would have made this more clear to you

Op ma 24 sep. 2018 om 10:26 schreef davidmal200 notifications@github.com:

ok, it was my suggestion just. adding explanations never hurts. For example I wasn't 100% sure what

no object should appear twice in the tree

meant-though I had suspicion as I elaborated.

So finally as you said if state fails JSON.parse/stringify test (e.g. produces objects with same content but different semantics say) I shouldn't use immer right? (If yes, then maybe docs should at least mention it?)

Ok thanks and good luck.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/mweststrate/immer/issues/201#issuecomment-423905252, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvGhIkh8tXeKRHBGqbL6C1ivsTIjVhGks5ueJcbgaJpZM4W10uY .

[ghost]

i probably won't have time for it and maybe I am not best person to do that PR.

I just advised there maybe many more beginner developers (esp. in functional stuff) trying to use immer and they might not be well aware of the stuff we discussed.

Ok that's it. Bye thanks.

[ghost]

@mweststrate Also

One of the core principles of Redux for example is that the state must be a tree as doing otherwise will break assumptions other tools make on your state

I don't see in the link you provided where Redux states that state can't be graph e.g. that two objects can be repeated in the state.

[mweststrate]

A tree is always a graph. But not all graphs are trees, like yours. As soon as there are two paths leading to the same leaf, it is no longer a tree. https://en.wikipedia.org/wiki/Tree_(graph_theory). The main reason that normalization is an important concept when working with Redux, is to make sure that the tree properties are preserved. As stated, you will have to either make sure your filteredItems are denormalized, or make sure that the filtered items are not part of the state in the first place (you could store for example the filter itself, instead of it's resulut)

Op ma 24 sep. 2018 om 12:08 schreef davidmal200 notifications@github.com:

@mweststrate https://github.com/mweststrate Also

One of the core principles of Redux for example is that the state must be a tree as doing otherwise will break assumptions other tools make on your state

I don't see in the link you provided where Redux states that tree can't be graph e.g. that two objects can be repeated in the tree.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/mweststrate/immer/issues/201#issuecomment-423928264, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvGhGkSgfmn25dXP0geJJX9wDULvDa2ks5ueK8hgaJpZM4W10uY .

[ghost]

@mweststrate I think we have bit muiscommunication.

I understand requirement of immer that state must be unidirectional tree.

However, what I said it is not a requirement in Redux for state to be a tree such that same object can't appear twice for example. e.g. see here: https://stackoverflow.com/questions/43657675/redux-what-is-the-point-of-normalizing-the-data#. It maybe recommendation but not a requirement.

[mweststrate]

I read the page as follows: the state must be a tree, that is why a lot of data is duplicated in the examples. Since this is a bad thing, stuff should be normalized. The normalization part is, as far as I can see the optional part, (there are cases where non-normalized trees can be more convenient, for example if compositional relationships are used).

The reducer model simply breaks when using a graph; try for example to write a reducer that updates the element with ID: 1 in the state you are proposing. You will notice that it will be very tricky to create a next state that is still a graph, especially if you want to keep couplinglow. but anyhow, you are free to structure your state whatever you like, but as far as it concerns immer: it doesn't guarantee anything when using graphs.

Op ma 24 sep. 2018 om 15:41 schreef davidmal200 notifications@github.com:

@mweststrate https://github.com/mweststrate I think we have bit muiscommunication.

I understand requirement of immer that state must be unidirectional tree.

However, what I said it is not a requirement in Redux for state to be a tree such that same object can't appear twice for example. e.g. see here: https://stackoverflow.com/questions/43657675/redux-what-is-the-point-of-normalizing-the-data#. It maybe recommendation but not a requirement.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/mweststrate/immer/issues/201#issuecomment-423978467, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvGhGDhjGKYltY2R-bMT0XKqN5LlN0Yks5ueOEagaJpZM4W10uY .

[ghost]

You will notice that it will be very tricky to create a next state that is still a graph

You mean this because now the reference to the element with ID1 will change right? But I was never interested in retaining structure of graph in state. I was just mentioning that redux doesn't restrict you from having two same objects in state.

but anyhow, you are free to structure your state whatever you like, but as far as it concerns immer: it doesn't guarantee anything when using graphs.

Yes I think this was my point there are no restrictions from redux, you can use a graph, however, for immer it must be a unidirectional tree.