Fork me on GitHub
#fulcro
<
2018-02-12
>
Drew Verlee03:02:07

One thing i was interested in was the idea of modeling using state-charts. I’m curious if this idea has been ported in anyway to fulcro and in also, if in general, people think its a good idea.

myguidingstar07:02:48

@wilkerlucio How do I use pathom along with fulcro.server/defmutation? fulcro.server/parser use server-read which has different signature from that of pathom parser.

pithyless08:02:50

@wilkerlucio don’t have a small reproduce case at the moment, but my preliminary notes: 1. render-map and render-data are being called a lot, which is hitting the GC hard. 2. The above is happening on each transaction, even if inspect is not visible. This seems like an easy win fix: don’t do all the render-map work when inspect is not visible. 3. In the GC work, I’m seeing lots of cons/seqs; if I’ve got 8k single key-val items, that’s calling render-map 8k times and render-data 16k times (key and value). Maybe some of those inner loops should be rewritten with more eagerness?

pithyless08:02:22

Here’s what I’m seeing during a single transaction:

nzjoel08:02:37

Hi. I'm just learning about fulcro and have been through the YouTube series and read some of the book. One thing I'm wondering is if/how it is possible to represent aggregate queries. Especially how to do a count of entities i.e. how many items are in a shopping cart. I can do this by getting a list of all IDs but is there a way to just retrieve a count? Thanks

claudiu09:02:41

@nzjoel for aggregate I sometimes just do that in the parent-component and just pass it thought computed. For count I just add the count in the db, and have a prop ui/count (on load you have post-mutation). Add item to basket mutation will also do a count and add the result in the appropriate place in the app-state.

cjmurphy12:02:28

@drewverlee Using state charts is appealing to me too. But after reading http://blog.cognitect.com/blog/2017/5/22/restate-your-ui-using-state-machines-to-simplify-user-interface-development I decided I might try something a little less formal. I haven't tried it yet but my idea is to have a data-structures of state transitions, just like in the article. Then each component that warrants it would have :current-state as part of the props so rendering can be done according to whatever :current-state is. Then perhaps have a mutation that gets the keyword of the transition that is to be performed, and changes :current-state, which of course will be automatically picked up by the component.

Drew Verlee18:02:55

Thanks a lot for posting this. Ill take a look!

wilkerlucio13:02:35

@myguidingstar in the end, the defmutation is a wrapper that adds to a mutation multi-method of fulcro, the fulcro.client.mutations/mutate, you will want to replace the full parser and use the mutation from fulcro, eg: (def parser (p/parser {:mutate fulcro.client.mutations/mutate, ::p/plugins [...]}))

myguidingstar13:02:35

actually I'm asking about fulcro.server, but I guess it's the same

wilkerlucio13:02:16

yeah, maybe it's a different method name (check with the server defmutation implementation, not sure if it is the same), but once you have the multimethod name, just use it 🙂

myguidingstar13:02:18

it's fulcro.server/server-mutate

myguidingstar13:02:29

actually I've never seen (p/parser {:mutate λ}) anywhere in pathom's doc

wilkerlucio13:02:39

@myguidingstar you are right, it's missing and I wasn't realising it, thanks, I'll be adding it soon

wilkerlucio13:02:45

@pithyless thanks for the details, I just pushed a new version that might fix it, can you please try the 2.0.0-alpha6-SNAPSHOT?

pithyless14:02:25

@wilkerlucio does the newest clojars version map to the develop GH branch? I’ve got a local version checked out to profile/test that I can check later.

wilkerlucio14:02:41

@pithyless just pushed on avoid-render-when-inspect-is-inactive

tony.kay15:02:33

@nzjoel There are a few ways to do aggregate things. At the moment you can add to the query engine, but it is not very convenient (you can, in fact, completely supply an alternate read engine). The overhead of doing that for just getting simple aggregates is a major source of pain, which is ironically part of the reason Fulcro exists. Om Next has you write the entire query engine for your application (which would allow you to add such features willy-nilly, but is a lot of work). Fulcro defaults you to a graph query engine, but it is lightweight, and does not attempt to provide these kinds of concerns out of the box because of the complexity and overhead they might add. We are considering ways of making it more extensible, but at the moment here are the options: 1. Write your own read handler. This is similar to writing the parsing for the server (see advanced parsing), and I don’t really recommend it. It’s way too much for most webapps. 2. (recommended for small cases) query for the items, and display a count as a derived value (as you indicated). This is recommended because it is lightweight, simple to understand, and easy to code in seconds. It has the distinct advantage that the UI is always up to date with respect to the data. 3. (recommended for large cases) Cache the count somewhere. I don’t recommend this unless it is a costly query or calculation (lots of items). A cached count goes out of date easily. That said, if you have a lot of them, then a mutation that runs at well-defined points (additions/removals) of the list makes for something you can easily query, update, and display. Note that you can query for a complete table just by using the table name in a link query [ [:things/by-id '_] ] (which comes back at :table/by-id in props). Then it’s as easy as asking for the count of values from the map. Of course, you may need to filter the table, etc. The down-side of this is that your aggregation logic ends up in the UI. In some cases that’s fine (like a count), but in other it is less ideal (business logic filtering). In the latter case, I’d definitely opt for a mutation that maintains the aggregate value in app state, and is refreshed at well-defined state transitions.

tony.kay15:02:55

One final note: You may be thinking “Well, if it just had aggregate queries, I wouldn’t have to cache the stupid thing”. I would like to circumvent that thinking with this fact: I can’t write an aggregate to be any faster than you can. If it is a lot of items, it will slow down the UI, and you’ll end up caching it anyway. That is ultimately at the core of the decision of “don’t bother” with aggregates in the default query engine. If it is a small number (shopping cart), the query-for-all-and-calculate is so little code as to be a non-issue…why complicate the internals of Fulcro for it? If it is a large number, you’re going to need to cache it to get fast UI updates…so again, why complicate the internals? What I hope to provide soon is a way for you to easily dispatch on a given query value during the query processing to optionally calculate the value as part of the query result. That would allow us to plug in things on-the-fly, and allow you to extend the query engine for any abstract desire without having the overhead of writing the whole thing, nor complicating Fulcro with a bunch of custom query syntax and concerns.

pithyless15:02:17

@wilkerlucio - so the fix partially works. 1. No lag when loading new app w/o inspect. 2. Enable inspect; lots of lag (but this would be expected, fix does not address this) 3. Disable inspect (via Ctrl-F); Still see lag and see DataViewer/render being called. So I think something is not resetting visible? correctly. 4. DataViewer/render is called twice after each transaction; do you know why? Removing this would also help with performance.

wilkerlucio16:02:29

@pithyless the DataViewer is used in many places, it's no surprise if that's called many times after a tx, but the item 3 is a bit surprising, with the change, if you can't see it, it should mean it's not been rendered, so that looks odd

pithyless16:02:15

@wilkerlucio - you’re right, visible? is indeed false after 3, but I’m still seeing calls (via println debugging) to DataView/render afterwards.

pithyless16:02:53

so, I don’t think it has anything to do with GlobalInspector at that point; it must still be mounted and reacting to tx changes.

pithyless16:02:31

yeah, so I can confirm that after 3, GlobalInspector/render is not being called, but DataView/render is still being called

wilkerlucio17:02:31

@pithyless can you please try to add a breakpoint on your logs and track from where the DataView/render is coming from?

pithyless17:02:55

Yeah, I’m going to try to debug this soon. First guess - it looks like every time I open/close inspect, the number of times DataView/render gets called per transaction increases by 2. Is there any reason why unmount would not be working correctly?

wilkerlucio17:02:45

I can't see that on my trials here, your setup might have something different from mine, let's keep on debugging

wilkerlucio17:02:45

@pithyless and thanks for taking the time to debug and provide info on that, this is the type of real world scenarios that are hard to figure upfront

tony.kay17:02:45

@wilkerlucio and thank you for working so hard on such a useful tool!

pithyless17:02:13

Yeah, seriously. fulcro-inspect is a game-changer 🙂

pithyless17:02:00

I’m seeing this in my firefox debugger:

;; in (reconcile! [this remote] ...
;; calling:
(optimal-render this components-to-refresh render-root)

;; which calls:
(when (mounted? c)
 ...
 (update-component! c next-props)

;; which does:
(force-update c)

;; which ends up calling
DataViewer/render

;; (without going through GlobalInspector)

wilkerlucio17:02:17

do you know if firefox debugger can track async stacks?

wilkerlucio17:02:24

I know chrome can, and it's helpful on those times

pithyless17:02:00

give me a few and I’ll just grab the newest Canary; I recently ditched it for FF Quantum 😛

wilkerlucio17:02:19

hehehe, yeah, it had a lot of good performance promises

pithyless17:02:33

@wilkerlucio - what am I looking for? I have one async stack - the requestAnimationFrame. It’s just reacting to the Atom/notify_watches (since my transaction swaps! the client db, after which it invokes reconciler/reconcile! which leads to the optimal-render etc.

pithyless17:02:53

I’m trying to figure out why the thing would still be mounted

wilkerlucio17:02:52

@pithyless this is something you can try, inside of the render of DataView, log this: (js/console.log "rendering data viewer" (js/ReactDOM.findDOMNode this))

wilkerlucio17:02:09

this way you can see the dom node on the console, and use that to try to find it on screen

pithyless17:02:41

So, I can confirm Chrome still thinks the node exists

pithyless17:02:06

(although it can’t really point me to it on the screen; heh)

wilkerlucio17:02:48

had you tried to right click on it and "Reveal in elements panel"?

wilkerlucio17:02:56

that's not very helpful, hehehe 😛

pithyless17:02:09

KeyListener/componentWillUnmount does not look like it’s getting called. (But componentDidMount does get called once every time I open inspect)

wilkerlucio17:02:25

humm, that's a smell

wilkerlucio17:02:37

I suspect it can be related to the IFrame component

wilkerlucio18:02:12

not really, the iframe goes below that... :thinking_face:

wilkerlucio18:02:12

just so I get to know, what react version are you using? and which build tool (figwheel or shadow?)

pithyless18:02:52

"react": "15.6.2",
        "shadow-cljs": "2.0.147",

pithyless18:02:40

FYI; looks like KeyListener willUnmount doesn’t get called, but willUpdate, didUpdate, and didMount are logging

wilkerlucio18:02:22

the log you see every mount comes from the key-listener inside of MultiInspector

wilkerlucio18:02:51

but it should be unmounting, if it isn't it might mean the other ones are not been properly unmounted as well

wilkerlucio18:02:03

and again I suspect on the iframe

wilkerlucio18:02:11

@pithyless I pushed a change to that same branch, also published a new snapshot, this should fix the iframe unmount, can you try that please? (with this change I see the unmount properly logged from the keylistener)

pithyless18:02:38

@wilkerlucio - I can confirm, iFrame unmount runs, as does KeyListener unmount; BUT DataView does not unmount and DV/render still gets called. 😄

pithyless18:02:28

don’t worry, at this rate we’ll find/fix all React/Fulcro bugs 😛

pithyless18:02:03

BTW, feel free to go async at any time; fixing this is not mission critical for me; I’ll just slowly putter along through the code while finishing up work-work.

wilkerlucio18:02:00

that's ok, if I need to go async I'll just be gone 🙂, would be nice if I could reproduce it here too, maybe if you just dump your data and send me?

nzjoel19:02:59

@claudiu @tony.kay thanks for your responses. At this stage it is just something I was wondering about and looking into but hopefully soon I will have some concrete cases to deal with and actually try out the different approaches you mentioned! So far I am really liking what I see with Fulcro. I really enjoyed the youtube series BTW, thanks!

pithyless20:02:03

@wilkerlucio I’ve made a repo for the unmount bug; see if you can reproduce on your machine. https://github.com/pithyless/fulcro-inspect-perf-bug#bug-1