Clojurians Log v2

Still looking at making this css protocol server-side 🙂 (a bit slow since this is on my free time) Do I read this PR correctly in that it allows extending untangled defui? https://github.com/untangled-web/untangled-client/pull/35 If this is the case, I have a few question about untangled if you don't mind: - does it suport sse completely now? - is there anything that would make untangled unsuited for a "live" dashboard? ie. a dashboard where re-rendering is not triggered by user input, but rathter by data being updated (by a streaming query for instance)?

I'm looking at untangled-ui for a reference for generic components. Like a multiselect or a calendar. I see the calendar actually has I query, which I found odd, because my first instinct was that generic components would just provide hooks for their actions.

So that all their props would be determined by some transformation of their parents data

So a multiselect would get a list of options (e.g.: [{:label :id :selected?}])

and some callbacks via om/computed

:on-select

and :on-select woud be a function which takes a new list

[{:label "a" :id 1 :selected? false} {:label "b" :id 2 :selected? true}]

and when "a" is selected on-select gets a new vector

where [{:label "a" :id 1 :selected? true} {:label "b" :id 2 :selected? true}]

So this doesn't really require any new data in the app db

or at least no data whose name (:keyword) is predefined in the query of the generic component

But I'm sure there's a reason for the opposite approach, so I'd like to be enlightened 🙂

@nha Yes, we have an extensible defui. Not documented yet. Considered experimental (API may improve). It has not been tested server-side. Untangled is very well suited for server push. We even have a websockets networking add-on library.

@urbank On Untangled UI: Some components have an active nature, and instead of using component local state (which would make them not appear in the support viewer), they are instead first-class Om components. They come with construction functions and mutations. In cases where they give you data, they will have callbacks. The APIs in untangled-ui should be considered unstable and improving. Not an official release yet. Don't consider any particular thing to be the prototypical example code.

But in general, any active component will have app state, query, ident, mutations, etc.

The exceptions will be things like the image crop tool, which needs local state for performance

@tony.kay Right, but there's also a middle ground between Om components with queries, idents and mutations, and those with local state, right? Namely those that do not compose into a query, but just get some props from a parent that takes care of deciding which part of the db this component should affect. Like the multiselect I described above. Or should such a multiselect also have a query?

Hm. "should"

that is a can of worms

1. Do you want it to show up in the support viewer so you can see the user's interaction?

Just read "should" as "is there a list of reasons" 🙂

The other consideration is that local state ends up hiding things you might care about. Take a menu. If you click on a menu, it should open. What should happen if you click elsewhere? Should it disappear? How do you hook that up?

but clicking elsewhere is not the menu's concern...so, what, we install event handlers on the doc that try to deal with closing the menus that are open

ok, now do you do that in menu's code? How do you know it isn't going to infect something else with badness now?

If menu is an Om component with state, then mutations are used to open/close them. The menu calls mutations when it sees interactions directly with itself.

Then, on your top-level div of the app, you can easily install an onClick that transacts a (close-all-menus) mutation. Now everything is still reasonable (you don't have to know implementation detail or internals of menu). You only need to know the name of a mutation

and can even choose what actions close all menus.

Then again, the menu installing event handlers on the doc has worked for us for years. YMMV

but think about all that complexity of event handling, component lifecycle, making sure 3 menus don't install 3 event handlers (or maybe you make the close idempotent so you don't care). Making sure the callbacks are not triggered if the menus are somehow gone. etc etc.

Ask me: bleh. I'd rather have a few mutations I can reason about, and actually visibly SEE the installation of the event handler on my root

cause I typed it, not because some component infected my DOM with it

To me, closing menus on a global click is a global concern (not private to menu)

Right, and I agree with you on all that. I'd also prefer the global mutations. However, does every component that doesn't directly affect the app-db need to have local state to be useful? In the example of the multiselect, it doesn't have any local state, and it doesn't directly have entries in the app-db

It's just nested inside some component which has some concept of selecting things

that component passes some transformation of its props to the multiselect

Yes, I'm on the fence on those as well.

They don't really need local state or their own query. Just props.

I would prefer they be pure render, no local state: (ui-multiselect {:options [:a :b :c] :selected #{:a} :onChange (fn ...)})

that way the parent manages the state

and can do things like deny combinations of selections...and we don't end up with hidden state in the UI

that is out of sync from app state

Something like Calendar: I'd rather have the query, etc. There is a lot of complex stuff to manage, and if there is a bug in Calendar you'd like to see it in app support viewer

Right, that's my intuition about the multiselect as well. I suppose if the calendar were made in the same way, you'd still see it in the support viewer, right? Just indirectly? Isn't that just a copy of the application for the support that gets sent the list of sequence of mutation that happened on the user's side?

Yeah, but you'd have a much larger data set to manhandle

the point of composable queries is to...compose...why push that off on your parent?

(defui Parent 
   (query [this] [I HAVE TO KNOW ABOUT CALENDAR HERE NO MATTER WHAT])

so what if it is a join or a set of props?

I'd rather think "Oh, I ask for calendar's props here". InitiailAppState or mutations make it simple/easy to put the calendar's initial state in the db

Hm, right. I hadn't considered it from the 'I HAVE TO KNOW ABOUT CALENDAR HERE NO MATTER WHAT' perspective... good point

yeah, sorry for the caps. Not sure how to bold in code 😉

not bothered at all 🙂

the parent always needs to know about immediate children. You want that reasoning to be abstract. Composable components/queries/initial state/rendering work out fine

We're used to composing this way...it's just we're used to the information hiding stateful component model. The Om model keeps the composition and abstraction, but throws away the information hiding and statefulness

You're allowed to see what is in Calendar, but you're allowed/encouraged to think of it in the abstract

Yeah, I'm a huge fan of that model because it tends to turn out that I need some information that I've hidden

In the case of the multiselect though, I have a feeling (and I need to verify it), that it does work best as stateless and without representation in the db, because the list you're selecting can be something other than a list of values.

For example, if I have a bunch of 'Instruction entities' in my db that each have some field that holds the day to which a particular instruction pertains.

The component can still be a stateful component, just not THAT state. But I do agree that perhaps you'd rather transform your data on the fly and pass it as props as opposed to transform it into a diff form in the db.

Yeah, basically. That's a very concise way of putting it!

I was going to go an a whole ramble, but that sums it up 🙂

Tracking down what I think to be a bug in Untangled. I just wanted to check my thinking. Should a df/load on a component using the component’s ident replace the entire components db table entry with a load marker, or just put a load marker under :ui/fetch-state?

@tony.kay Well, I've gotta run. Thanks for the discussion!

@gardnervickers replaces the component.

Thanks

the fetch state has to be in the query, or you won't see it

you can turn off that behavior with a load option...I think :load-marker

So the parent would be in charge of lazily-loaded then, so as not to be passing the munged state to ident?

yes

if the component isn't loaded, you should not try to render it 🙂

Yup makes sense. I’ll put up a PR in a few minutes.

a PR?

df/load doesn’t behave like that unless you specify a :target.

From the doc string I take it that using an ident with df/load should make it ignore a :target.

Hm. Not sure ignoring target is necessary

the object should normalize to the ident location, but there could be a good reason for plopping the ident into the graph somewhere.

and avoiding needing a post mutation for that op

https://github.com/untangled-web/untangled-client/blob/develop/src/untangled/client/impl/data_fetch.cljc#L309-L311 Don’t we need a case here to place the data-ident when a data-field is not specified?

It works (loading by ident) for df/load-field but not for df/load

sorry....brb

I agree that putting the ident elsewhere in the graph would be very useful too. :target or no :target, we should still be replacing the data in the ident’s table with a load marker.

Sure!

OK, some useful info just to make sure we're on the same page.

I think (if I remember right), that a load marker has this structure {:ui/fetch-state { nested data }}

in an early version we just merged this with your entity

but the to-many case made that not work, cause you cannot merge a map with a vector

we toyed with metadata, but Om messes with the metadata, and we didn't want to take the chance on collisions

so, since we were not able to mark the vectors while also keeping them around, we decided replacing them was the right thing.

The load markers are simply a convenience. Nothing says you can't implement them yourself (do a mutation to mark your load, then use a post mutation to remove the marker)

so, if you want to leave the data in place you can accomplish it, but only you know how

now, that said, when targeting an ident, you ARE targeting a map

but that map may or may not already be present, so even if we could merge, you still would not want ident being called on it

in case it wasn't there yet

Yup, sorry I don’t mean to get focused on the lack of ident there.

no, I was just talking through some of the logic so you understand.

Ahh gotcha thanks

now let me answer your question 😉

I believe you are right

well, perhaps no

The query should cause normalization, so the table should get filled no matter what

the data-path is about additional locations for idents

https://github.com/untangled-web/untangled-client/blob/develop/src/untangled/client/impl/data_fetch.cljc#L364

might be wrong

if the key is an ident

I guess this is where I’m confused how the else the load marker is placed in the table entry for an ident https://github.com/untangled-web/untangled-client/blob/develop/src/untangled/client/impl/data_fetch.cljc#L46

oh, no, an ident dissoc would just crash

Oh, ok, I see

I think you are right there...the data path of an item targeted at an ident is just the ident.

Yeah, I don't see how that could hurt anything, and it definitely sounds like the bug you're seeing.

Thanks for your help, PR #61 works for me now.

cool

@gardnervickers got a failing test

and that is definitely in the wrong order

that would break load-field

cond short-circuits, so the field case is now unreachable

I went ahead and patched it and added a new test. Try 0.8.0-SNAPSHOT

on clojars

I'm testing for regressions using cookbook recipes

Ah ok thanks

@tony.kay Was that pushed to github? Not seeing

just did, sorry

was pushed to clojars though

Does that logic not conflict with this? https://github.com/untangled-web/untangled-client/blob/develop/src/untangled/client/data_fetch.cljc#L68-L69

I'm not seeing why

keyword is just a keyword

oh...shoot. Is the ident always set?

hm. no it isn't, but I'm not sure we've fixed anything.

I was referring to the part “A custom target will be ignored"

If we put the check for ident at the end of the cond, then setting a custom target ignores the ident.

sorry, I'm in the middle of a bunch of other stuff. This is not a trivial fix, and I probably should not have even committed a patch without better analysis.

No worries

load is one of the more complicated areas, any change will need to be regression tested against all full-stack cookbook recipes.

I've verified that my change does not break the load markers recipe, but it does not enable a load marker on a ident-based load

It could be that detecting the ident and putting in the params map at :ident in load will work, but I'll have to comb through it

the internals of this need refactoring 😕

probably deserves separate functions for each use-case, so that each one is clear. They're all tangled up at the moment, which is a bit ironic 😉

Heh

No rush, thanks for looking at it. We’ll go back to just using the :target then.

probably better. I'm thinking it prob does not make sense to put a marker in a db table...because of the possible confusion about ident

for example what if you queried the table in a UI component and tried to render all of the things...you would not expect a loading marker, nor would you want to mess with the logic

the markers make more sense in fields in the graph

A data-load for an entity is a pretty app-wide thing though right?

hm. could be argued that way, true

might want to see loading in all the rendered spots.

It’s more I would rather not have stale data in other spots

Stale data isn’t a good word for it actually

hm. nothing will be stale

oh you mean the marker only appearing in one location

if you're rendering it more that once on same screen

in that case you could do your own marker. Just a boolean on your object. set it to true, trigger load, include post mutation that sets it to false

easily a reusable pattern/helper function of your own

:ui/is-loading?, (defmutation loading-done ... (assoc-in state-map (conj ident :is-loading?) false), (load ... {:post-mutation 'loading-done :post-mutation-params {:obj ident}})

then just implement your loading UI via that boolean

that's possibly a better general pattern for refresh

I don't think "dumb" load markers should ever appear in tables, because of the ident generation issue

I just pushed a sample to the develop branch in cookbook:

https://github.com/untangled-web/untangled-cookbook/blob/develop/recipes/lazy-loading-visual-indicators/src/client/app/ui.cljs#L26

@gardnervickers ^^^

Oh neat thanks

of course, I'd recommend composing that stuff into a single mutation, so it looks like a clean transact...I should refactor the example 🙂

pushed a slightly cleaner bump on that

With using the post mutation there, is it possible to trigger a re-render on the component that originally called the mutation?

I find myself needing to change the AST for a remote from CLJS land - anybody have a good example of that? I feel like I've set it up correctly, but it's not working

(In certain situations, I want to tack on an additional parameter to a mutation before it is sent to the server)

Actually, nevermind - I did it correctly - it's just my server-side mutation is not responding the way I thought it should.