keechma 2017-08-10

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

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

yes, so there are two parts

1. form-type

2. form-id

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

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

there is a convenience form mount controller that can automatically mount forms https://github.com/keechma/keechma-toolbox/blob/master/src/cljs/keechma/toolbox/forms/mount_controller.cljs

but you can also manually mount forms if needed

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

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.

Yeah, I saw that.

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.

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

well this is crazy

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

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

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

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

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

(defn partial ([f] f))

it’s a mistery 🙂

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

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

@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

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

so arity2 not 3

it seems that cljs 1.9.456 introduced this behavior

I’ll try to figure out what did it

👍 1

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

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

@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

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

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

Ah, ok I'll push the fix out

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

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

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

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

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

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

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

So I think that all pipeline based code is safe

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

Yeah, all caused by the same issue

My first hunch was that its multi arity related

But it was wrong

So it's a bug in clojurescript?

I guess

But I couldn't find the commit that caused it

I've asked on #clojurescript channel

Hopefully someone will answer

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

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

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

I like how the current API feels

👍 1

Wonder why partially implemented protocols aren't supported

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

yeah, that’s what I was expecting too

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

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

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

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

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

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.

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 😆

@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

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

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

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