om.core/update-refs

[verma]

Just trying to understand the refs update logic a little bit.

Our ref-changed? calls (from inside shouldUpdateComponent [1] seem to be always saying that the ref has changed while comparing an empty map with the value of the ref (fetched from the global app-state).

This was a little confusing for us since we are not really performing any state updates. Upon further inspection of the code, it seems like the update-refs is never really updating the "__om_refs" (the existing set of refs) with new values from the app-state [2].

seems to be doing something like this:

(into #{} (filter nil? (map ...))

Seems like its trying to only add nil values into the the set, which seems to indicate that if any refs haven't changed or have changed and have been correctly updated from the global app-state, never make it the the set of "__om_refs" there by rendering it empty in most cases. This causes our code around [1] to always see refs as "changed" (local ref value being empty and app-state saying that it has data) and causing shouldComponentUpdate to return true even when there were no state updates.

Is this the expected behavior? Not sure we're understanding this right :(.

[1] https://github.com/omcljs/om/blob/master/src/om/core.cljs#L322
[2] https://github.com/omcljs/om/blob/master/src/om/core.cljs#L259

[verma]

Tried changing it to

(into #{} (remove nil? (map ...)) ..

The updated refs we end up with are slightly different from the ref-cursors observe seems to add to the __om_refs vec. Which means, on the next update cycle, (contains? refs ref) fails again and we end up with two cursors in the __om_refs vec which kind of point to the same value inside the app state, which is probably not the intended effect.

Also, the (adapt ..) stuff returns something that doesn't really implements the IOmRef protocol. We end up with some errors with -remove-dep! is called on these instances.

Not sure if I am explaining this right, just trying to get as much information as possible posted here.

Will try to work more on this, and may be try to get a PR submitted if things seem to be going in the right direction here.