Fork me on GitHub
#keechma
<
2017-08-10
>
urbank10:08:30

@mihaelkonjevic In the new forms library, can only one form be active at a time for a given key in the map passed to keechma.toolbox.forms.controller/regiester ?

urbank10:08:32

the second part of the tuple is determined by the forms-params function, right?

mihaelkonjevic10:08:58

yes, so there are two parts

mihaelkonjevic10:08:15

so you can have multiple forms of the same type active at the same time

mihaelkonjevic10:08:46

form id is usually either :new or id of the entity you’re editing

mihaelkonjevic10:08:27

but you can also manually mount forms if needed

urbank10:08:10

Cool. The example apps are very useful for figuring out how it fits together

urbank11:08:42

Hm... so with the newest version of clojurescript keechma$controller$IController$execute$arity$3 is called in the compiled javascript for the pipeline controller. But with an older version (1.9.229) keechma$controller$IController$execute$arity$3 is never called, only defined in the keechma/controller.js file.

urbank11:08:15

Yeah, I saw that.

urbank11:08:43

But surely execute has to be called at some point in the working version, right? Can't find any call sites in the compiled javascript though.

urbank11:08:40

Hm... no that was a shot in the dark. It does get called, obviously. If I add console.log there it prints

mihaelkonjevic12:08:43

well this is crazy

mihaelkonjevic12:08:54

this form works (apply controller/execute [this :start params])

mihaelkonjevic12:08:31

this also (let [e (partial controller/execute this :start params)] (e))

mihaelkonjevic12:08:22

this one also works lol ((partial controller/execute) this :start params)

urbank13:08:19

Does this mean it's likely that this is a compiler bug ?

urbank14:08:19

But wait... partial with no arguments literally just returns the function!

urbank14:08:35

(defn partial ([f] f))

mihaelkonjevic14:08:13

it’s a mistery 🙂

mihaelkonjevic14:08:35

I’m downgrading cljs version by version atm to figure out which version introduced this

urbank14:08:59

Cool :thumbsup:

urbank14:08:21

So if partial with not arguments works so should just plain identity, right?

urbank14:08:26

@mihaelkonjevic So it seems that with the pipeline controller the partial thing works. However with the dataloader controller and the forms-mount-controller I get

urbank14:08:27

this$__$1.keechma$controller$IController$execute$arity$2 is not a function

urbank14:08:45

so arity2 not 3

mihaelkonjevic14:08:48

it seems that cljs 1.9.456 introduced this behavior

mihaelkonjevic14:08:02

I’ll try to figure out what did it

urbank14:08:20

Wonder why arity 2 doesn't behave the same. Adding an empty parameter to the dataloader controller's execute method and applying partial to execute works too

urbank18:08:54

@mihaelkonjevic hm, why does key-to-path strip namespace from the keyword

mihaelkonjevic18:08:58

@urbank it's using name fn which does it. Name is used to get string from keyword, but there was no intention to remove the ns too

urbank18:08:55

I see. I'm using namespaced keywords for most of the fields of various entities. For example, the 'on-change' helper from keechma.toolbox.forms.helpers/make-component-helpersuses the key-to-path function, so a form with a field like :person/name doesn't update

urbank18:08:48

Oh, if a pass it in as a vector, it should work I suppose

mihaelkonjevic18:08:18

Ah, ok I'll push the fix out

mihaelkonjevic18:08:57

I'm not sure what to do with the arity errors though

mihaelkonjevic18:08:27

I could fix them in keechma and toolbox by wrapping fns in identity

mihaelkonjevic18:08:25

It feels dirty but it would also allow new cljs version which has nice npm integration

urbank18:08:09

Does feel a bit dirty. But IMHO the benefit outweighs it 🙂

urbank18:08:24

however a custom controller would also need to do this, correct?

mihaelkonjevic18:08:43

Yes but I'd add it to readme. I'm. It sure how much would it affect people

mihaelkonjevic18:08:22

It only happens when you call on overriden protocol method from the overriden one

mihaelkonjevic18:08:46

So I think that all pipeline based code is safe

urbank18:08:22

Did you also see the arity 2 error for dataloader and mount-form controllers?

mihaelkonjevic18:08:49

Yeah, all caused by the same issue

mihaelkonjevic18:08:25

My first hunch was that its multi arity related

mihaelkonjevic18:08:29

But it was wrong

urbank18:08:37

So it's a bug in clojurescript?

mihaelkonjevic18:08:43

But I couldn't find the commit that caused it

mihaelkonjevic18:08:14

I've asked on #clojurescript channel

mihaelkonjevic18:08:23

Hopefully someone will answer

urbank18:08:56

Oh, cool, a minimal example. I'm sure someone will know what's going on.

mihaelkonjevic20:08:40

heh, I sure hope that the new behaviour is not the intended one, I’ll have to rewrite the controller if it is 🙂

urbank20:08:19

@mihaelkonjevic 🙁 Oh well. So are you thinking of a macro where it puts the default implementations for the unimplmented methods?

mihaelkonjevic20:08:45

I like how the current API feels

urbank20:08:48

Wonder why partially implemented protocols aren't supported

urbank20:08:51

Protocol is somewhat like typeclass in haskell, right? There default implementations of "methods" still work if you implement some of the "methods"

mihaelkonjevic20:08:29

yeah, that’s what I was expecting too

mihaelkonjevic20:08:06

one other option is to split methods, so there’s only one per protocol. We would have IControllerExecute, IControllerHandler etc, but that seems verbose

urbank20:08:53

I don't have the foresight to have an opinion on what would be better 🙂

mihaelkonjevic21:08:06

@urbank what do you think about multimethods? I think this might be the solution and the API would be similar

mihaelkonjevic21:08:41

they support default implementation, so most of the API would be similar

urbank21:08:49

@mihaelkonjevic Hm... right multimethods! Seems good to me. Perhaps it's a bit less "in one place" if that makes sense, but otherwise seems fine. Multimethods are supposed to be somewhat slower, but I suppose that shouldn't be an issue for this use-case. I'll think about it for a bit and tell you my thoughts for what they're worth. Can't think of much right now 🙂

mihaelkonjevic21:08:03

yeah, I understand what you mean with “less in one place”. But the most important (at least to me) part is the ability to have a default implementation so you don’t have to implement everything just to satisfy the form.

urbank21:08:09

Yeah, I think that's definitely much more important. "Less in one place" is mostly just aesthetics, and maybe it's because I got used to this. Plus, emacs (parinfer/edit not sure) indents my protocol implementations a bit weirdly anyway 😆

mihaelkonjevic22:08:42

@urbank keechma 0.3.0-SNAPSHOT-1 is out. Controllers are rewritten to use multimethods, updated to latest reagent, latest (stable) react (15.6) and latest ClojureScript

mihaelkonjevic22:08:19

it wasn’t that horrible. all controller changes are mechanical, function bodies are almost identical

mihaelkonjevic22:08:11

also, I’ve pushed my forked version of cljs-react-test (https://clojars.org/org.clojars.mihaelkonjevic/cljs-react-test) if you need it. It’s updated to latest react too

mihaelkonjevic22:08:31

I’ll update the toolbox lib tomorrow or over the weekend