Conditional nodes rendering clarification needed
Closed this issue · 32 comments
I've started to port my current app to freactive from reagent because I'm very satisfied of former core model in spite of alpha project state. Good play with polymer is real advantage over React-based solution, and I see much more goodies in freactive.
But I'm lost with conditional nodes rendering. The simplest and quite complete demonstration of my problem is case of using json-html to render debug panel:
(defn inspector [state]
[:paper-shadow {:z 1}
[:core-toolbar "Inspector"]
[:div (rx (edn->hiccup @state))]])
(dom/mount! debug-root (inspector state))
This snippet render everything right first time, but after state
is updated, only nodes representing keys which presented in initial version are updated, other nodes are ignored.
Is this a bug or my misunderstanding how things should work?
Okay let's see. Can you give me some examples of types of objects that would be represented in state? Also, maybe describe a little more fully what you're expecting to happen and what is happening.
Also, I did write some more about how invalidation works: https://github.com/aaronc/freactive#reactive-change-notifications-in-depth. Let me know if this helps clarify anything.
I want to achieve simple goal: reactively render content of freactive.core/atom, which is nested hash-map of dynamic structure. I have function edn->hiccup
which maps hash-map to hiccup-like vector structure. E. g. initial state is {:a 1}
, and it is updated to {:a 2 :b 3}
. (edn->hiccup {:a 1})
gives (roughly, real function draws beautiful table) [:div [:div "a" [:div 1]]]
, and (edn->hiccup {:a 2 :b 3})
gives [:div [:div "a" [:div 2]] [:div "b" [:div 3]]]
Using snippet from first post gives me <div><div>a<div>1</div></div></div>
at the beginning and <div><div>a<div>2</div></div></div>
after state update. So, inspector is re-rendered — but no new nodes are added.
Excuse me if my explanation is too obscure, say what is not clear, and I will try again.
I think what you are describing should work the way you expect it to - I will look into it.
One possible issue is that freactive currently does not have any default rendering for numbers defined. Maybe I should change this but I guess I was thinking that this should be locale specific so that it only understands strings (text nodes) or hiccup vectors (element nodes) like the real DOM. Does it work if you convert the numbers to strings first.
No, it doesn't. But I've started to compose minimal example project, and everything work as expected. M.b. it is bug in my application, or I need to expose more complex example. Thank you for your time, I will investigate further and write back when anything will become more clear.
Yes, you are right, I've tested stringifying numbers in wrong way, it is really because of numbers and booleans in place of text nodes. Strange that this failed silently in my app.
This should be easy to make freactive accept numbers and booleans, or there are reasons for which user should cast those types before passing hiccup to freactive?
Smth like
(defn- wrap-element-spec [elem-spec]
(cond
(string? elem-spec) (ElementSpec. elem-spec)
(or (number? elem-spec)
(= (type elem-spec) js/Boolean)) (ElementSpec. (str elem-spec))
:else elem-spec))
(defn- get-virtual-dom [x]
(when x
(cond
(dom-node? x)
(when-let [state (get-element-state x)]
(get-virtual-dom (.-element-spec state)))
(string? x) x
(vector? x) x
(or (number? x) (= (type x) js/Boolean)) (str x)
:default (-get-virtual-dom x))))
Yes, we could do something like that - we'd actually add extra clause to the cond
in get-virtual-dom
or extend IElementSpec
(protocol invocation is slower however).
Does hiccup support bool's as well? We should try to support what it supports. In the case of nil
values, one option is to insert an empty placeholder text node.
Yes, it does. Tested a moment ago [:div 1 true nil]
What does it do for nil
?
On Tue, Nov 18, 2014 at 2:46 PM, Ruslan Prokopchuk <notifications@github.com
wrote:
Yes, it does. Tested a moment ago [:div 1 true nil]
—
Reply to this email directly or view it on GitHub
#13 (comment).
result of above is <div>1true</div>
, i'm not sure what it does under the hood
I think I'll do this:
(extend-protocol IElementSpec
boolean
(-get-virtual-dom [x] (str x)))
(defn- get-virtual-dom [x]
(if x
(cond
(dom-node? x)
(when-let [state (get-element-state x)]
(get-virtual-dom (.-element-spec state)))
(string? x) x
(vector? x) x
(number? x) (str x)
:default (-get-virtual-dom x))
;; nil values treated as empty "placeholder" text nodes
""))
(= (type x) js/Boolean)
appears to be quite slow against non-boolean values...
I still have problems with conditional rendering. It succeeds in simple case, but in my application it does not work for somewhat reason.
When I write
(defn view [state]
(rx [:textarea (str @state)]))
textarea contents is automatically updated accordingly to state changes, but
(defn view [state]
(rx (if (:ws-ready? @state) [:textarea (str @state)] [:div "NOT READY"])))
hangs on NOT READY
when ws-ready?
is already set to true
. In minimal case both examples work fine, but I have problems in more complex environment, and I can't figure what is really interfering with rx
dependency registration.
Do you have any ideas where to look and how to debug this case? May be I should show you my code?
You will be laughing, but
(defn view [state]
(if (rx (:ws-ready? @state)) [:textarea (rx (str @state))] [:div "NOT READY"]))
works fine. I'm idiot.
Well, I wouldn't say you're an idiot ;)
I'm actually not totally sure myself why this doesn't work:
(defn view [state]
(rx (if (:ws-ready? @state) [:textarea (str @state)] [:div "NOT READY"])))
what happens if you try:
(defn view [state]
[:div (rx (if (:ws-ready? @state) [:textarea (str @state)] [:div "NOT READY"])))]
I've tried to wrap in :div
already, result is exactly the same.
Have you still been having issues with conditional rendering? I haven't been able to replicate an example like this one: (defn view [state] (rx (if (:ws-ready? @state) [:textarea (str @state)] [:div "NOT READY"])))
not re-rendering. Maybe I fixed something... - are you using git master or 0.1.0?
I'm using master and it is still does not work in my particular case — but works when I'm trying to find out minimal case ;-(
So, is it possible you could post a full gist of a failing case? It doesn't have to be your actual code just something similar enough that fails where it's unexpected.
I want to make sure this sort of unexpected behavior gets fixed.
It is not a problem to show real code, I'll open repo for public access for a while, it is kept private only because it is pre-pre-pre-alpha messy stuff ;-)
admin
function is main view which is mounted to dom and where bug happens.
Ok, I have a better sense now.
For your admin function, you need an rx
around the whole if block. (if (rx @ws-ready?) ...)
doesn't cause any data binding to occur - the rx
is in the test part of the if. You also want an rx
around each block that's returned from the deck-section and structures fn's. Like this:
(defn deck-section [structures id i]
(let [section (get-in @structures [id :sections i])]
(rx
[:div
[:paper-input {:label "Section name"
:value (:name section)
:on-change (fn [e] (swap! structures assoc-in [id :sections i :name] (.. e -target -value)))}]
[:paper-input {:label "Size"
:value (:size section)
:on-change (fn [e] (swap! structures assoc-in [id :sections i :size] (.. e -target -value)))}]])))`
In your match block, you'll probably want to surround the calls to deck-structure, etc. with [:div ...]
until directly nested bindings are supported.
Actually you probably want something more like this with cursors:
(defn deck-section [structures id i]
(let [section (fr/cursor structures [id :sections i])]
(rx
[:div
[:paper-input {:label "Section name"
:value (:name @section)
:on-change (fn [e] (swap! structures assoc-in [id :sections i :name] (.. e -target -value)))}]
[:paper-input {:label "Size"
:value (:size @section)
:on-change (fn [e] (swap! structures assoc-in [id :sections i :size] (.. e -target -value)))}]])))`
Then, (swap! structures assoc-in [id :sections i :name]
can be changed to (swap! section assoc :name ...)
. Cursors behave just like atoms in freactive.
So, I really wanted (rx (if
but it does not cause rerender, but (if (rx
does. I don't know why.
Thank you for recommendations for deck-structure(s)
, it is my playground now, I'm searching optimal combination of rx/cursor placement which will always rerender on data update (because sometimes it does not for somewhat reason) and will not cause memory leaks (because sometimes they happen)
So, if you don't deref
an rx
or let framework do it by binding somewhere, it should always evaluate to true in (if (rx
. See what the return value of (rx ...)
is in the REPL - should always be ReactiveExpression...
.
So memory leaks - we'll need to debug that... if bindings aren't disposed properly this could happen - I thought I had it taken care of, but maybe not.
Have you tried the debug-rx
macro anywhere?
It was a goog.History
magic which interfered with freactive
and broke admin
conditional update. I don't understand in which way exactly, but I've localized problem. I will use another mechanism for routing handling.
So memory leaks - we'll need to debug that... if bindings aren't disposed properly this could happen - I thought I had it taken care of, but maybe not. Have you tried the debug-rx macro anywhere?
https://gist.github.com/ul/ebc72eee5a4d915c8f83 — when i go to :deck-structures
page with code in this gist i got memory leak with debug-rx
output as in gist.
So would you say in terms of conditional rendering that freactive is now more or less behaving as you'd expect it?
If so, then we can move on to debugging memory leak issues... How are you noticing that this code is causing a memory leak? Are you observing it in the Chrome task manager or something like that?
Okay, I did a little profiling myself and did discover a significant memory leak caused by not detaching a node state from its parent state when it's disposed. Just pushed 772b238 to fix it. See if that helps at all with your memory leak issues. There may be other things, but this was definitely glaring.
Yes, thank you a lot, conditional rendering works fine, and memory leaks (which I observed by Chrome task manager, you've guessed) stopped. But items-view does not work for me (it does initial render, but I lost how to use it properly — how does it track source changes, or entire items-view form got called again on change, so every element rerendered in DOM and only gain is that it it not recalculated in VDOM?; inserting in source vectors will lead to many of redraws after inserting point up to end of vector?; why need of transact!, will it be better to workout transparent swap! on passed cursor? etc.). I've tried to write my own version of it, but it seems not so trivial to get it right, need to think more. May be some thing I mentioned in parenthesis are not necessary optimizations, I'm not sure about that.