Fork me on GitHub
#untangled
<
2016-10-25
>
mitchelkuijpers09:10:33

Whohoo return values for mutations (will be very useful for certain situations)

tony.kay19:10:21

Hey everyone trying to use latest cljs: @wilkerlucio just sent me a PR that fixes Untangled to work with the latest Om/Cljs. I have applied it to develop branch, and pushed it to clojars as part of 0.6.0-SNAPSHOT

wilkerlucio19:10:36

@tony.kay just fyi, I just tested compiling with the latest snapshot and it's working again, thanks for the release 🙂

tony.kay19:10:42

thanks for figuring out the fix

tony.kay19:10:00

I'll probably cut an official release soon

tony.kay19:10:25

hoping to get the forms stuff worked out as well this week. Too many things going on

tony.kay19:10:33

I'm really excited about the forms addition.

wilkerlucio19:10:33

nice, exciting times

tony.kay19:10:16

I also need to get some cookbook recipes and docs around the new additions

tony.kay19:10:26

if anyone is looking for stuff to do 🙂

wilkerlucio20:10:24

@tony.kay what features you are looking for help on documenting?

tony.kay20:10:31

The stuff in the changelog

tony.kay20:10:38

@wilkerlucio like the new return value support

tony.kay20:10:48

want to put it in the devguide

tony.kay20:10:05

probably using a client-simulated server, which I've done in other files.

tony.kay20:10:31

post-mutation parameters can probably just go where post mutations are talked about

tony.kay20:10:20

Make sure the new load function is in data fetch, and note the deprecated load-data.

tony.kay20:10:50

those are the main updates that are needed

tony.kay20:10:00

Then, of course, I want to start a section on forms when I get that done

tony.kay20:10:09

I would also like a section covering the i18n story

tony.kay20:10:13

and one for the support viewer

tony.kay20:10:36

both of those last two need cookbook examples too

wilkerlucio20:10:48

@tony.kay ok, I'm still trying to catch up with all the new goodies, I would like to experiment a bit with the server-return, then I can help with some docs, do you have an example use case for the server return?

tony.kay20:10:14

Yeah, um....where did I put that 😉

tony.kay20:10:18

@wilkerlucio See the discussion above with jasonjckn (11:11am ysterday)

tony.kay20:10:32

it has pointers to the exact lines of example code in my template

tony.kay20:10:57

and a discussion of how/why. I made one change at the end of that discussion, so make sure you follow that 🙂

tony.kay20:10:17

I hope it didn't throw off the hyperlinks

tony.kay20:10:04

My coding guidelines around it are to make your return handler a multimethod, and co-locate the return handler with the mutation.

tony.kay20:10:16

that way you can reason about them together

tony.kay20:10:11

If I had control of the Om parser internals I probably would have made it an additional key on mutation methods (:value, :action, :return-action)

wilkerlucio21:10:04

@tony.kay just to check if I'm getting it right, the :return is something you can get back from a server mutation, and then it will be dispatched into a post-mutation to be handled on the client, is that correct?

jasonjckn21:10:28

@tony.kay perhaps the mutation results should trigger a mutation ? it seems like the untangled api is asymmetric, e.g. on mutation failure (tx/fallback) => trigger mutation, but on mutation success (mutation result) implement some global handler

jasonjckn21:10:21

in that bucket we also have post-mutations

tony.kay21:10:03

@jasonjckn the problem is we only have the symbol that is already an Om mutation

tony.kay21:10:13

and made the original net request

tony.kay21:10:19

we need a separate entry point

jasonjckn21:10:30

e.g. here's symmetry: post-mutation: a mutation triggered to operate on recently changed local state mutation result: a mutation triggered to operate on recently interim state returned fallback: a mutation triggered to operator on error state

tony.kay21:10:43

yes, but fallback is for things like server exceptions

tony.kay21:10:51

return values are for...well, anything

tony.kay21:10:09

and each mutation may want a specific handler for its return value

jasonjckn21:10:59

yah good points, oh well, i guess it's the best design right now, in theory, you could add (tx/mutation-result :action 'foo/bar)

jasonjckn21:10:06

but i don't know if that improves anything

jasonjckn21:10:46

i guess i can feed mutation results into another multi method

tony.kay21:10:48

yeah, just makes it noisy

tony.kay21:10:20

if you co-locate the defmethods of mutate and handle-return, then the code is all right together

tony.kay21:10:28

that was my comment earlier about guidelines

jasonjckn21:10:32

I suppose the 'real' API win would be the following:

(defmethod mutate 'foo/bar [..]
  {:value {:keys []}
   :action (fn [] ..)
   :result-action (fn [] ..)})

jasonjckn21:10:36

is that workable?

tony.kay21:10:44

that was my earlier comment

tony.kay21:10:52

see above about 1 hr ago

jasonjckn21:10:16

If I had control of the Om parser internals I probably would have made it an additional key on mutation methods (:value, :action, :return-action) ah yes

jasonjckn21:10:22

didn't read that till now

jasonjckn21:10:33

so we're on the same page, too bad that's not doable

jasonjckn21:10:36

but multi method should be fine

tony.kay21:10:40

well, we can fork Om 😉

jasonjckn21:10:08

om.next.next

jasonjckn21:10:35

maybe it's worth opening a githbu issue about this

jasonjckn21:10:43

you won't be the only one to try and wrap om.next in a framework

tony.kay21:10:53

I can already tell you the answer

jasonjckn21:10:54

yes to see if they can make it more pluggable

tony.kay21:10:56

The answer is to customize your own merge. Hm.

tony.kay22:10:05

my brain is itching

jasonjckn22:10:07

in clojure philosophy data is extensible, e.g. {:untangled/mutation-result ... :action ... :value ...}

tony.kay22:10:15

which means there may be a way to hack that in after all

jasonjckn22:10:23

interesting 😄

tony.kay22:10:40

actually, I think there is

tony.kay22:10:14

OK, well, thanks for pushing on that. I see how to do it now

tony.kay22:10:31

@wilkerlucio might want to hold off on docs, I think we can do without the extra mmethod

jasonjckn22:10:51

i love it when we our conversations lead to these refinements 🙂

tony.kay22:10:08

it's also why some of this stuff is in a SNAPSHOT

tony.kay22:10:15

play with it

jasonjckn22:10:33

also may be worth doing :fallback-action in the same place if that makes sense, you could keep both apis for backwards compatibility.

tony.kay22:10:14

fallback is needed for exceptions. Don't know what went wrong, so they're not as easy to target

tony.kay22:10:27

they are the fallback for everything in the tx, not just an item

tony.kay22:10:38

if you want targeted, use return vals

tony.kay22:10:58

:result-action or :return-action?

jasonjckn22:10:51

so the arguments are are [env k result-without-tempids] ?

tony.kay22:10:55

hm. This might not be the best idea, for other reasons. I have no AST or anything. So, if you write a mutation that wants to play with that stuff globally (e.g. to rewrite the params of a remote) it might crash

tony.kay22:10:09

in other words all I have for env is state

tony.kay22:10:27

I could mark it in env

tony.kay22:10:33

(:is-return? env)

jasonjckn22:10:02

that seems fine

jasonjckn22:10:30

in theory you could pass the right env, you just don't know how to do build it up yet

jasonjckn22:10:35

leave it for another PR

jasonjckn22:10:06

it's just not easily available to you through om.next, the env still definitely makes sense in mutation results

jasonjckn22:10:34

-that- might be something to open a PR about in the future

tony.kay22:10:53

I'm not sure I can't get to it...I'll just have to walk through the implementation

jasonjckn22:10:03

for now is-return? is good enough

tony.kay22:10:05

but a marker seems good nevertheless.

jasonjckn22:10:06

i'd leave that for another day

jasonjckn22:10:20

honestly nobody will probably notice

jasonjckn22:10:53

can you put the right :ref in there though?

jasonjckn22:10:24

i like :remote-result fwiw

jasonjckn22:10:47

or possibly (:remote (assoc ast :action-result ...) :action ...)

tony.kay23:10:19

@jasonjckn erm, :result-action?

tony.kay23:10:02

it's not a result, it's an action

jasonjckn23:10:15

it's all okay but that name doesn't really highly it's the remote's action, not the local action

tony.kay23:10:30

bleh...naming

jasonjckn23:10:30

there's really 2 different actions, easy to conflate

tony.kay23:10:44

:remote-result-action

jasonjckn23:10:50

yes there's no confusion there

jasonjckn23:10:10

(:remote (assoc ast :action-result ...) :action ...) is more in line with om.next

jasonjckn23:10:16

but not as 'easy'

tony.kay23:10:28

yeah, I kinda hate that

jasonjckn23:10:39

yah i wish om.next did something else there

jasonjckn23:10:50

for situations where you have remote params

jasonjckn23:10:07

:remote-result ?

tony.kay23:10:46

I'm sorta liking how it is currently implemented, partially because the env isn't the same (since we're no longer parsing a query)

tony.kay23:10:02

I think conflating them is going to cause all manner of confusing bugs

tony.kay23:10:28

but I do like the symmetry of this new approach

jasonjckn23:10:54

the env is the same no? it's just we're not sure how to generate it right now

tony.kay23:10:13

Let me dig in the plumbing for a min

tony.kay23:10:50

here's the problem: our hack location is in Om merge

tony.kay23:10:58

and Om does not pass the original env of the request to me

tony.kay23:10:09

I can get to app state because I made it

tony.kay23:10:19

so I can close over the atom in the merge routines

tony.kay23:10:43

All I get from Om is the target and source values

tony.kay23:10:41

and you want to be called after all of the merging is done...so this is the phase to run in

tony.kay23:10:05

um...I can probably hack it through with metadata

tony.kay23:10:27

but to get env absolutely correct I'd really have to hack into the parser

jasonjckn23:10:41

is there some conversation you can have with @anmonteiro about this

tony.kay23:10:42

and re-parse the original complete tx while dispatching to this

jasonjckn23:10:49

some new extension to om to make env available

tony.kay23:10:09

no, this is something Om is leaving up to the implementor for good reasons, I think

jasonjckn23:10:16

changing the parser is no big deal IMO

jasonjckn23:10:22

that's how om.next has meant to be used, every tutorial is like that

tony.kay23:10:38

I'm kinda leaning towards my current solution again. I don't think this is something ppl are going to need a lot, and co-locating a clear mmethod next to your mutation really is pretty clean

tony.kay23:10:54

no, you're wrong there.

tony.kay23:10:59

we use the term wrong

tony.kay23:10:11

our multimethods are really emitters

tony.kay23:10:19

the parser is the thing that makes an AST

tony.kay23:10:25

and dispatches to our emitters

tony.kay23:10:41

I'm saying we'd have to hack the thing that makes the AST and dispatches

tony.kay23:10:46

which is not something you ever do

jasonjckn23:10:51

yah that's no good

tony.kay23:10:07

I can see a hack that would work, but it is uuuuugly

tony.kay23:10:33

mmm, not sure it would work either

jasonjckn23:10:28

does it make it easier if it's {:remote (assoc ast :action-result (fn [] ..) :action } this seems like the right place for it, does this not give you some flexibility in the remote implementation to call the action-result handler when the remote action finishes?

tony.kay23:10:33

not seeing how that could work

tony.kay23:10:59

and then I've got a mess of logic to even pull it out

tony.kay23:10:07

in a whole different part of the plumbing

tony.kay23:10:53

the problem continues to be the correct env, which I cannot generate

tony.kay23:10:14

but your server code can pass back anything you might need

tony.kay23:10:19

which is the intention

tony.kay23:10:33

e.g. you can add things in your params on the mutation that you want reflected back

tony.kay23:10:08

yeah, I'm sticking with how I've implemented it. It's just too fragile and complected to add this into mutate, even though on the surface it looks like a good idea

jasonjckn23:10:34

untangled implements this remote right? so in the implementation, here's a shitty one, but you maintain a registry of mutation name->{action-result function , env, etc} then when merge comes back (or maybe there's some callback in remote) you use that registry, obviously this sucks, does it work? i'm not that familiar with om, but i feel like if it's doesn't already support this 'callback when remote mutation comes back' this seems like a logical flow of it's design

tony.kay23:10:51

It seriously makes an internal mess.

tony.kay23:10:00

for very little benefit

jasonjckn23:10:04

sure I follow

jasonjckn23:10:22

anyways if you ever think of some small change to om.next that would allow this to be internal niceness, definitely post an issue

tony.kay23:10:28

not remotely simple to accomplish or maintain

jasonjckn23:10:49

i don't know the specifics of om, but just thinking about it generally it seems to flow at least in my mind, so maybe there's some small change they can make

jasonjckn23:10:41

whether that's env available in reconciler pluggable merge, or env available in remote mutation ajax cb, etc, etc

tony.kay23:10:40

env is generated during the parse, not any of those other phases

tony.kay23:10:08

the whole constructions of invocations of parse has to do with the front-side of transactions, not the back-side

jasonjckn23:10:05

the results of parsing, e.g. the AST are needed both before remotes are contacted, and also after they're contacted, to for example normalize the returned data

jasonjckn23:10:26

so from that abstract view it would follow env should be available after remote returns with action-result

jasonjckn23:10:08

i understand the specific om.next api right now doesn't allow for that, but maybe there's a small change worth suggesting

tony.kay23:10:35

hm. There's actually a trick we could leverage that is already supported

tony.kay23:10:52

hadn't thought of that...

jasonjckn23:10:04

😄 that sounds promising

tony.kay23:10:26

so, when you invoke the parser, you say (p q) or (p q :remote)

tony.kay23:10:32

the :remote is convention

tony.kay23:10:40

multiple remotes just use diff keywords

tony.kay23:10:13

(p q :result-action) would act like a remote parse

tony.kay23:10:22

oh, but it would expect the value to be a boolean or ast

tony.kay23:10:37

not a thunk

jasonjckn23:10:46

well you could do the (assoc ast thing

jasonjckn23:10:29

are you the first person to use this for non-remote things?

tony.kay23:10:40

I don't think you should, actually

jasonjckn23:10:13

yah good brainstorming, but i think we're back to where we were before

tony.kay23:10:12

{:result-action (callback body)}

tony.kay23:10:25

where callback is a macro that emits an ast hack

jasonjckn23:10:51

are you the first person to use :remote for non remote things?

tony.kay23:10:12

I don't know of anyone even thinking along these lines, no

adambrosio23:10:27

wouldnt callback need env or ast?

tony.kay23:10:44

yes, but I'd call my own parser on the original tx

adambrosio23:10:57

ah so it doesnt need to be passed

tony.kay23:10:13

you'd just use it from the param list of your mutate

tony.kay23:10:19

which I'd be invoking again

tony.kay23:10:24

close over it

tony.kay23:10:46

it's really hacky to get a small amount of syntactic sugar

jasonjckn23:10:09

talk to @anmonteiro at some point, I don't see why there isn't a small change here in om.next

tony.kay23:10:26

well, perhaps we'll find future creative things to do that need these ideas. I'm going to stick wit hthe current impl

tony.kay23:10:20

It really isn't a small change. The ability to plug-n-play your own database, networking, and everything else eliminates being able to do it well, I think

jasonjckn23:10:31

it's very clojury to support {:action ... :remote ... :untangled/action-result } sort of thing and there's no reason why env shouldn't be available, it's needed to merge&normalize, so why not for :untangled/action-result

tony.kay23:10:42

it is not needed

tony.kay23:10:45

the query is needed

tony.kay23:10:54

and you can modify the query before you call merge

tony.kay23:10:17

actually merge doesn't even use the query

tony.kay23:10:20

normalize does

tony.kay23:10:30

and that certainly isn't a place to be calling callbacks

tony.kay23:10:42

you haven't even put the data in the db yet

tony.kay23:10:13

Q -> net -> server -> resp -> om callback -> normalize -> merge -> result-callbacks

jasonjckn23:10:24

ok, and you don't like on-success mutation? even though you have post-mutation and error mutation

tony.kay23:10:57

post-mutation was experimental, and I don't think it should remain, to be honest

tony.kay23:10:05

what error mutation?

tony.kay23:10:20

but that just is a hook to trigger mutations

tony.kay23:10:49

on-success is what I just added 🙂

tony.kay23:10:06

and it needs to trigger on the original mutation symbol

jasonjckn23:10:44

alright well looks like you chose the best available option then

tony.kay23:10:04

For now. It is almost certainly possible to make our desired thing work

tony.kay23:10:18

and it will be an easy port from the current implementation

tony.kay23:10:42

so, let's call this v1 of return value handling and move on

adambrosio23:10:50

jokingly just write a defmutation macro that looks like what you want but outputs 2 defmethods

tony.kay23:10:09

Actually, I kind of like that best so far

jasonjckn23:10:32

yah i mean i'll probably just do that for the team here if that's the api lol

jasonjckn23:10:36

i do want the logic co-located

tony.kay23:10:05

but how often will you acutally need it? I've written a lot of Om code so far without needing it at all

tony.kay23:10:21

I can see the basic need, but it seems like something you rarely should use

jasonjckn23:10:23

not often, but the other guys keep asking about it

tony.kay23:10:35

yeah, but part of that is a desire to do things "the old way"

tony.kay23:10:41

and should be discouraged 🙂

jasonjckn23:10:45

for sure, i've had the conversation a number of times now

jasonjckn23:10:50

yah definitely

tony.kay23:10:23

optimistic updates -> tx and fallbacks on the (very rare) failures is soooo much better for the end user

tony.kay23:10:31

and the developer

jasonjckn23:10:31

it's not just with developers though, it's also with UI people

jasonjckn23:10:36

(the conversation) 😞

tony.kay23:10:41

The UI ppl should not even be aware of a remote

jasonjckn23:10:54

their little spinners and stuff

tony.kay23:10:56

except for lazy loading

tony.kay23:10:07

yeah, but that is loading, not mutation

jasonjckn23:10:31

well action buttons with spinners

jasonjckn23:10:49

"applying action... " "action applied.." sort of UI feedback

jasonjckn23:10:53

anyways i'm on the same page as you