Fork me on GitHub
#untangled
<
2017-03-03
>
nha00:03:27

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)?

urbank13:03:28

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.

urbank13:03:10

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

urbank13:03:04

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

urbank13:03:21

and some callbacks via om/computed

urbank13:03:24

:on-select

urbank13:03:40

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

urbank13:03:23

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

urbank13:03:52

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

urbank13:03:24

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

urbank13:03:39

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

urbank13:03:28

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

urbank13:03:22

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

tony.kay16:03:43

@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.

tony.kay16:03:44

@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.

tony.kay16:03:14

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

tony.kay16:03:35

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

urbank16:03:34

@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?

tony.kay16:03:11

Hm. "should"

tony.kay16:03:18

that is a can of worms

tony.kay16:03:07

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

urbank16:03:11

Just read "should" as "is there a list of reasons" ๐Ÿ™‚

tony.kay17:03:38

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?

tony.kay17:03:26

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

tony.kay17:03:57

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?

tony.kay17:03:46

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.

tony.kay17:03:41

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

tony.kay17:03:52

and can even choose what actions close all menus.

tony.kay17:03:22

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

tony.kay17:03:40

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.

tony.kay17:03:08

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

tony.kay17:03:22

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

tony.kay17:03:21

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

urbank17:03:36

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

urbank17:03:02

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

urbank17:03:28

that component passes some transformation of its props to the multiselect

tony.kay17:03:31

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

tony.kay17:03:43

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

tony.kay17:03:29

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

tony.kay17:03:20

that way the parent manages the state

tony.kay17:03:37

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

tony.kay17:03:44

that is out of sync from app state

tony.kay17:03:34

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

urbank17:03:59

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?

tony.kay17:03:26

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

tony.kay17:03:47

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

tony.kay17:03:20

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

tony.kay17:03:48

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

tony.kay17:03:40

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

urbank17:03:30

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

tony.kay17:03:44

yeah, sorry for the caps. Not sure how to bold in code ๐Ÿ˜‰

urbank17:03:56

not bothered at all ๐Ÿ™‚

tony.kay17:03:27

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

tony.kay17:03:11

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

tony.kay17:03:53

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

urbank17:03:20

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

urbank17:03:24

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.

urbank17:03:21

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.

tony.kay17:03:49

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.

urbank17:03:33

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

urbank17:03:10

I was going to go an a whole ramble, but that sums it up ๐Ÿ™‚

gardnervickers17:03:14

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?

urbank17:03:49

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

tony.kay17:03:21

@gardnervickers replaces the component.

tony.kay17:03:36

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

tony.kay17:03:01

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

gardnervickers17:03:04

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

tony.kay17:03:28

if the component isn't loaded, you should not try to render it ๐Ÿ™‚

gardnervickers17:03:41

Yup makes sense. Iโ€™ll put up a PR in a few minutes.

gardnervickers17:03:22

df/load doesnโ€™t behave like that unless you specify a :target.

gardnervickers17:03:49

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

tony.kay17:03:12

Hm. Not sure ignoring target is necessary

tony.kay17:03:40

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

tony.kay17:03:55

and avoiding needing a post mutation for that op

gardnervickers17:03:42

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?

gardnervickers17:03:51

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

tony.kay17:03:35

sorry....brb

gardnervickers17:03:28

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.

tony.kay18:03:19

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

tony.kay18:03:45

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

tony.kay18:03:06

in an early version we just merged this with your entity

tony.kay18:03:23

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

tony.kay18:03:47

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

tony.kay18:03:21

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

tony.kay18:03:45

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)

tony.kay18:03:08

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

tony.kay18:03:27

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

tony.kay18:03:57

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

tony.kay18:03:04

in case it wasn't there yet

gardnervickers18:03:21

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

tony.kay18:03:44

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

gardnervickers18:03:50

Ahh gotcha thanks

tony.kay18:03:05

now let me answer your question ๐Ÿ˜‰

tony.kay18:03:57

I believe you are right

tony.kay18:03:25

well, perhaps no

tony.kay18:03:42

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

tony.kay18:03:09

the data-path is about additional locations for idents

tony.kay18:03:58

might be wrong

tony.kay18:03:07

if the key is an ident

gardnervickers18:03:13

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

tony.kay18:03:16

oh, no, an ident dissoc would just crash

tony.kay18:03:05

Oh, ok, I see

tony.kay18:03:25

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

tony.kay18:03:33

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

gardnervickers18:03:39

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

tony.kay19:03:55

and that is definitely in the wrong order

tony.kay19:03:17

that would break load-field

tony.kay19:03:34

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

tony.kay19:03:38

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

tony.kay19:03:26

I'm testing for regressions using cookbook recipes

gardnervickers19:03:05

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

tony.kay19:03:38

just did, sorry

tony.kay19:03:43

was pushed to clojars though

tony.kay19:03:06

I'm not seeing why

tony.kay19:03:24

keyword is just a keyword

tony.kay19:03:35

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

tony.kay19:03:10

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

gardnervickers19:03:56

I was referring to the part โ€œA custom target will be ignored"

gardnervickers19:03:26

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

tony.kay19:03:08

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.

tony.kay19:03:02

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

tony.kay19:03:32

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

tony.kay19:03:28

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

tony.kay19:03:41

the internals of this need refactoring ๐Ÿ˜•

tony.kay19:03:38

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 ๐Ÿ˜‰

gardnervickers19:03:51

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

tony.kay19:03:00

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

tony.kay19:03:27

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

tony.kay19:03:35

the markers make more sense in fields in the graph

gardnervickers19:03:45

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

tony.kay19:03:13

hm. could be argued that way, true

tony.kay19:03:29

might want to see loading in all the rendered spots.

gardnervickers19:03:07

Itโ€™s more I would rather not have stale data in other spots

gardnervickers19:03:23

Stale data isnโ€™t a good word for it actually

tony.kay19:03:59

hm. nothing will be stale

tony.kay19:03:18

oh you mean the marker only appearing in one location

tony.kay19:03:26

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

tony.kay19:03:25

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

tony.kay19:03:45

easily a reusable pattern/helper function of your own

tony.kay19:03:19

: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}})

tony.kay19:03:31

then just implement your loading UI via that boolean

tony.kay19:03:36

that's possibly a better general pattern for refresh

tony.kay20:03:26

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

tony.kay20:03:34

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

tony.kay20:03:21

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

tony.kay20:03:54

pushed a slightly cleaner bump on that

gardnervickers21:03:21

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

therabidbanana23:03:54

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

therabidbanana23:03:37

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

therabidbanana23:03:16

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