Fork me on GitHub
#fulcro
<
2017-09-04
>
tony.kay00:09:48

I just tried it in the devcards…works perfectly 🙂

tony.kay00:09:12

I should add sanity checking for the form fields. Unfortunately that is a little thougher since you can define arbitrary form field types via your own functions, so there is no easy way to detect what the field name is reliably

tony.kay00:09:47

I guess I could just make it a restriction: if you want to use custom field types with defsc, then the field spec constructor functions must take the field name as the first argument.

tony.kay00:09:07

I think all of the built-in ones already match that shape

tony.kay00:09:59

25% reduction in boilerplate, by my use of wc.

tony.kay00:09:11

the sanity checking is a real win, though

wilkerlucio00:09:30

@tony.kay honestly, I think this macro is too magical... I like using the raw om defui. and I would be careful about having the properties at the definition, I remember the old om got away from having as parameters for a reason (forcing people to call om/props, if I remember right was a problem on callbacks or something). one question: how is the ident being infered, I see the table for the namespace part, are you assuming the db field would be always :db/id?

tony.kay00:09:55

There is an optional :id option (the defult is :db/id)

tony.kay00:09:30

This just emits a defui, so it cannot have the issues you speak of.

tony.kay00:09:40

It just morphs the argument list into a let

tony.kay00:09:51

it is calling om/props, om/get-computed, and om/children in that let.

wilkerlucio00:09:16

about the forms, I tried to use this yesterday, but I though it was too much... did you though about using specs to run the validations instead of having a separated multi-method system?

tony.kay00:09:03

a validation is a predicate…doesn’t seem like an ideal match for specs

tony.kay00:09:27

not like it has structure…it’s typically something scalar

wilkerlucio00:09:32

how come? you define the keys, and define a s/keys for the composition, looks very matchy to me

tony.kay00:09:46

oh, you mean full form validation?

tony.kay00:09:08

Then it is too weak. Write me a spec that checks that :password and :password-again match in a map

wilkerlucio00:09:17

I mean, we have field names already, why have to create a new key->predicate map?

wilkerlucio00:09:19

(s/and (s/keys :req [:user/password :user/password-confirmation])
  (fn [{:user/keys [password password-confirmation]}]
    (= password password-confirmation))

tony.kay00:09:58

Right, assuming they are inthe same map…that’s right. I was thinking about them as separate fields.

wilkerlucio00:09:21

when we are editing a form, we are basically editing a map, so we just need to validate that map

wilkerlucio00:09:35

it's much easier and simple if you keep then in 1 map dont you think?

tony.kay00:09:36

so, I’m game for a contribution that allows the use of spec for validation. But,you have to solve a couple ofproblems: 1. Matching the validation problem up with a form field 2. Nesting forms and validation

wilkerlucio00:09:01

spec is great for nested, the (s/explain-data) can give you the exact paths for the problems

tony.kay00:09:51

So, other than your obvious extreme preferece for spec validations, what else did you think? 😛

tony.kay00:09:29

BTW, partly spec wasn’t even in cljs when I wrote forms initially

tony.kay00:09:39

and it still technically isn’t stable or released

wilkerlucio00:09:19

I kind feel weird to declare parts of the input (view-wise) properties on that extra protocol... it might be a preference think, but I like better when I'm just provided a set of small tools that I can work with, and with forms it seems that extra layer requires too much and I don't see that much gain (trying to say: with smaller helper functions the same can be done without so much contract). I would like to see more just tools that can improve on the already existing structures, but helpers like defsc seems to drive users away from the more full featured approach, I just not sure if the sugar gain pays of

tony.kay00:09:27

So, when provided with those small set of tool, you end up writing generalizations. The added form support and defsc are just an added layer of abstraction on top of the base tools. No one says you have to use them.

tony.kay00:09:29

In the forms case, it generalizes and standardizes a pattern that a lot of people are goign to write over and over. It gives devs a framework to repeat instead of invent. With defsc, the added sanity checking prevents common errors, and becomes easier to read because of the reduction in boilerplate.

wilkerlucio00:09:05

I would just recommend going easy, maybe leaving the sugars to teach later on the docs and things, it's kind easy for us that know it already, but for who is comming those extra layers can add more confusion on how things work IMO

tony.kay00:09:33

I agree there. I’m not adding defsc to getting started 🙂

tony.kay00:09:31

And forms are actually kind of hard to get right. You have the “unedited” dirty state to track separately from the edited state, submission deltas, etc etc.

tony.kay00:09:39

writing all that from scratch is quite a bit of work.

wilkerlucio00:09:05

but hey, I want ot congrat on how complete the devguide is getting, it grew so much in the last weeks, you are rocking on it 🙂

tony.kay00:09:34

Thanks…I toyed with splitting it. IMO it is too large 🙂

wilkerlucio00:09:52

one thing about forms, you could try to leverage the keywords more

tony.kay00:09:01

that’s why I added the reference guide. I was thinking a lot of the details should go in something like a ref guide instead

wilkerlucio00:09:11

even if not just for specs, but the keywords have value by thenselves as unique idenfiers

tony.kay00:09:28

leverage kws more?

wilkerlucio00:09:42

you could use then to choosing form fields for example, instead of doing at the component level

tony.kay00:09:48

oh, you mean :person/age implies a kind of validation and type

wilkerlucio00:09:37

it's always good to be able to override, but most of times they work the same

tony.kay00:09:16

So, you’re implying that we derive form fields from the query?

wilkerlucio00:09:48

I think validations are better here, because the form is really about the layout, validation is more easy to share

tony.kay00:09:32

So, I agree with you that spec is probably the better way to go with form validations. I’m a little concerned about the namespaced keyword collision possibilities and how to handle them.

tony.kay00:09:33

and from a practical perspective, the whole-form validation would have to be declared somewhere…hooking it all up isn’t clear to me at the moment

tony.kay00:09:17

Rembmer that we have some global function that generally understands form support. When triggered, it has to figure out what validations are needed, run them, and mark the form field data on that particular bit of state.

tony.kay00:09:50

I guess we could add a special entry to specify the spec for the form in the IForm form-spec.

static IForm
(form-spec [(f/id-field :db/id) (f/validation-spec ::my-form) ...])

wilkerlucio00:09:07

it can be easier than that

wilkerlucio00:09:15

let me show you one code example

wilkerlucio00:09:48

(s/def :person/name string?)
(s/def :address/street string?)
(s/def :address/new (s/keys :req [:address/street]))
(s/def :address/list (s/coll-of :address/new))

(s/explain-data (s/keys :req [:person/name :address/list])
                {:person/name 123
                 :address/list [{:address/street "ok"}
                                {:address/street true}]})

wilkerlucio00:09:00

that outputs:

wilkerlucio00:09:03

#:clojure.spec.alpha{:problems ({:path [:person/name],
                                 :pred clojure.core/string?,
                                 :val 123,
                                 :via [:person/name],
                                 :in [:person/name]}
                                {:path [:address/list :address/street],
                                 :pred clojure.core/string?,
                                 :val true,
                                 :via [:address/list :address/new :address/street],
                                 :in [:address/list 1 :address/street]}),
                     :spec #object[clojure.spec.alpha$map_spec_impl$reify__483
                                   0x691dd42
                                   "[email protected]"],
                     :value {:person/name 123, :address/list [#:address{:street "ok"} #:address{:street true}]}}

wilkerlucio00:09:25

if you look at the :problems, you can see all the info you need to infer the field

wilkerlucio00:09:36

so you might just need to have 1 spec for the full form

wilkerlucio00:09:49

and by running the explain-data you can get the information to fill in

tony.kay00:09:25

Yes, I already know/understand that.

tony.kay00:09:36

I think we’re at diff points in the convo 🙂

tony.kay01:09:08

I can clearly see how to get spec errors to individual form fields

wilkerlucio01:09:10

ah, sorry, I think I got now

wilkerlucio01:09:27

that f/validation-spec is the indicator for the form

wilkerlucio01:09:36

sorry, read wrong before XD

tony.kay01:09:48

that is the join point so you can do what you’re talking about

tony.kay01:09:54

but there are still problems to solve

tony.kay01:09:57

well, perhaps not…I think that does get us validation. Once the markers are in place (which you’d have to translate to the graph db paths) the existing stuff should work, since rendering is separate from validating.

tony.kay01:09:25

Oh, actually, validations that are not directly associated with a single field are a problem

tony.kay01:09:47

That example earlier…passwords don’t match

wilkerlucio01:09:30

what misses there is how to relate that error with the fields, right?

tony.kay01:09:52

global kinds of relational validation

tony.kay01:09:06

field 1 needs to be smaller than field 2

tony.kay01:09:59

The rendering is based upon a marker on a field. “If this field is invalid, show this message”

tony.kay01:09:38

how does the problem path come back for the password mismatch?

wilkerlucio01:09:38

yeah, that's tricky

wilkerlucio01:09:02

because you can only return a false to handle the error, not sure how to provide that information part

tony.kay01:09:14

I mean, the existing on-form-change could be leveraged still to deal with that. The mechanism will always be needed anyway

tony.kay01:09:30

not only for validation, but for morphing the form as the user progresses

tony.kay01:09:02

e.g. “Do you like animals?“. Answer yes and the form changes to include a “Pick your favorite”

tony.kay01:09:59

@wilkerlucio what do you mean return false?

wilkerlucio01:09:11

I mean you can't provide extra error information, the spec validations are just simple predicates

tony.kay01:09:54

maybe we don’t have a full-form (clojure) spec at all…just some way of indicating that specs should be used against namespaced props if available.

tony.kay01:09:41

That gets you a kind of automatic form field validation system with the spec registry

tony.kay01:09:54

and full-form relations just continue to leverage on-form-change

tony.kay01:09:09

that’s also bw compatible with the multimethod validations

tony.kay01:09:46

Maybe just an option when calling the validation triggers…`:use-specs true`

tony.kay01:09:50

hm…that would be a lot of breaking API changes. Declaring it on the form itself during form init is better.

wilkerlucio01:09:28

it's complicated...

tony.kay01:09:47

better appreciation for forms now?

wilkerlucio01:09:53

hehehe, we will see, but for now I prefer to stay simple, just a couple helper functions here and there, maybe I didn't got complicated forms as you may have

tony.kay01:09:43

The features I felt were necessary: keeping the old state vs the edited one. Being able to revert a form. Being able to submit a delta. Being able to nest forms and have them submit as a delta of the graph.

tony.kay01:09:08

extensible validation, rendering, and form field types

tony.kay01:09:40

Now that spec is more commonly available, I agree that it is better for field-level validation.

tony.kay01:09:40

Oh, and the ability to do global form change monitoring that can do global validations and form state changes based on inputs.

tony.kay01:09:27

All of those things can be constructed by hand for a form. Obviously. The dirty state management is the most important feature, IMO

tony.kay01:09:03

@wilkerlucio and I would like your feedback once you actually to try defsc…particularly the sanity checking. We make mistakes all the time with defui, and it has already caught me screwing up a couple of times just playing with it. I’m not all that worried about the boilerplate, but catching errors like “you destructured something from props that you didn’t query for” is pretty darn nice.

wilkerlucio01:09:13

ok, I'll give it a try

wilkerlucio01:09:36

what about calling :query instead of :props?

tony.kay01:09:53

:props and :children make query

wilkerlucio01:09:33

I find that odd, because in Om we call it query

tony.kay01:09:25

Sure, I understand.

wilkerlucio01:09:21

I have one suggestion on the table field (this I would like be called ident, haha, sorry asking so much)

tony.kay01:09:23

defsc is not meant to replace all defui, just the common case of plain props and children

wilkerlucio01:09:50

but the recommendation on table is: if user provides a vector, you can use the second item to be key to be fetched from the props

tony.kay01:09:51

and ident is a table + id…I only want the table

tony.kay01:09:02

ah, that’s not bad

wilkerlucio01:09:02

eg: {:ident [:user/by-id :user/id]}

tony.kay01:09:26

at that point, :ident would be fine

wilkerlucio01:09:48

just to keep the names and avoid extra mind translations 🙂

tony.kay01:09:04

ok, I like that suggestion

wilkerlucio01:09:34

and since we are there, would be nice to accept a function there too

wilkerlucio01:09:14

because there are more complicated cases, like this one (real case):

wilkerlucio01:09:16

(ident [_ props]
    (let [[owner name] (str/split (::g.repository/name-with-owner props) "/")]
      [:github.repository/by-owner-and-name [owner name]]))

tony.kay01:09:06

so, I’m trying to walk the line with you of “too much” vs “too little”. If there are too many options, it becomes more difficult to understand. Too few options and it become too weak.

wilkerlucio01:09:29

yeah, I like the vector case because I think is the most common

wilkerlucio01:09:35

and the function one is the most flexible

wilkerlucio01:09:05

supporting both is a good cover range IMO

tony.kay01:09:22

yep, just more to read in a documentation string 🙂

wilkerlucio01:09:49

tradeoffs 😛

tony.kay01:09:03

The intention (for the moment) is to replace 60-80% of defuis

wilkerlucio01:09:33

is the css stuff supported there?

tony.kay01:09:34

so that we can slowly expand the API with experience to guide us

tony.kay01:09:45

nope, not yet

wilkerlucio01:09:32

this is my current snippet for fulcro components:

wilkerlucio01:09:34

(om/defui ^:once $COMP$
  static fulcro/InitialAppState
  (initial-state [_ _] {})
  
  static om/IQuery
  (query [_] [:$ENTITY$/id])

  static om/Ident
  (ident [_ props] [:$ENTITY$/by-id (:$ENTITY$/id props)])

  static css/CSS
  (local-rules [_] [])
  (include-children [_] [])

  Object
  (render [this]
    (let [{:keys []} (om/props this)
          css (css/get-classnames $COMP$)]
      (dom/div nil))))
      
(def $COMP_LOWER$ (om/factory $COMP$))

tony.kay01:09:23

Sure. So you’re using co-located CSS on everything?

tony.kay01:09:40

The problem there is that’s an external lib 😕

wilkerlucio01:09:01

would be too hard to make the config extensible?

wilkerlucio01:09:06

maybe another multi-method

tony.kay01:09:27

are you making fun, or serious ?

wilkerlucio01:09:55

maybe, that might make me more willing to use it if I can extend it, hehe

tony.kay01:09:57

yeah, it is possible to do that, but remember that you’re working with a macro here, so load-time issues turn multimethods into a nightmare

wilkerlucio01:09:09

yeah, that is this to consider

tony.kay01:09:15

macros run at compile time

tony.kay01:09:26

and then theres the “which compile time”

wilkerlucio01:09:33

I think on this case the multi-method might live on clojuer side

tony.kay01:09:40

it would have to

wilkerlucio01:09:21

but maybe is not that bad, if we can throw some data to the extension points like a pipeline, and then you get the combined thing and make into the final result

wilkerlucio01:09:00

bikeshedding, maybe XD

tony.kay01:09:09

So, I can say the last time I tried to use multimethod to modify a macro’s behavior, it did not go well

tony.kay01:09:59

I mean, it sounds like a cool idea…because that would make defsc fully extensible…like with mix-ins

tony.kay01:09:16

something in the back of my mind is nagging me with “hot code reload”…

tony.kay01:09:40

Ah, clojure isn’t hot code reloaded…if you make a multimethod add-on to your defsc, you have to restart figwheel to try it

tony.kay01:09:50

that was the main pain

tony.kay01:09:04

every edit: restart figwheel

wilkerlucio01:09:27

urgh... at least this extension should not be something people do very often (I hope)

tony.kay01:09:45

So, an alternative is this:

(defsc Component 
  [this props computed children]
  static css/CSS
  (local-rules [_] [])
  (include-children [_] [])
  { options map }
  body)

tony.kay01:09:15

allow the “extra” protocols to just be declared first. (or after) the options map.

tony.kay01:09:28

ugly, though

tony.kay01:09:01

bleh…that makes it extensible, is easy to code into the macro, but looks like shit-on-toast

wilkerlucio01:09:38

humm, how do you figure where the protocol implemention ends on that?

tony.kay01:09:48

body can only be one form

tony.kay01:09:00

oh, no, it can be any number

tony.kay01:09:06

hm…yeah, options map second

wilkerlucio01:09:20

you can enforce the body to be one

tony.kay01:09:34

yeah, but sometimes you want to shove debug statments in, etc.

tony.kay01:09:45

it’s nice to support something that acts like a function

wilkerlucio01:09:02

(do) is always there anyway 🙂

tony.kay01:09:28

I still don’t like how it looks…but I’m not liking any of the extensible options I can think of

wilkerlucio01:09:11

I was going to ask if you ever had a non-static protocol

wilkerlucio01:09:15

but then I remembered I actually did XD

wilkerlucio01:09:48

oh, I though I did, but wasn't, still a static call here

tony.kay01:09:59

They’re supported…but I’m in agreement: I wasn’t necessarily interested in the fully general case yet, just a common one that could save some headaches and boilerplate

tony.kay01:09:37

I have not been using co-located CSS, so the way I wrote defsc covers over 90% of my usages.

tony.kay01:09:59

maybe even all of them

tony.kay01:09:09

oh, not lifecycle cases

tony.kay01:09:11

so not all of them

tony.kay01:09:18

but rather than expanding the options for expansion, I was considering just letting you do the defui thing in the middle like that…I just don’t like how it looks. I like the idea of some kind of chain-of-responsibility add-on, but I don’t like the dev flow for it.

tony.kay02:09:46

this doesn’t seem better, other than the structure is cleaner:

(defsc Component 
  [this props computed children]
  { :props [:a]
    :protocols (static css/CSS
                 (local-rules [_] []) 
                 (include-children [_] [])) }
  body)

wilkerlucio02:09:53

maybe call it just :protocols

tony.kay02:09:39

why are whitespaces in Slack variable width???

wilkerlucio02:09:03

the editor is not monospaced font I guess

wilkerlucio02:09:17

so it's not WYSIWYG in that sense 😛

tony.kay02:09:54

now that I put it up there, I do like that much better

tony.kay02:09:49

Given that CSS is a formal extension lib and they are just symbols, I’m tempted to make CSS a first-class citizen of the macro, add :protocols for everything else (including lifecycle on Object).

tony.kay02:09:09

it doesn’t create a hard dep, and you’d only use it if you use the CSS lib (and require it somewhere to load it)

wilkerlucio02:09:37

I would prefer to send vector instead of a list to :protocols, I guess you can just avoid enforcing the list type and any will work as same (probably just a [email protected] somewhere anyway)

tony.kay02:09:56

exactly, either should work

wilkerlucio02:09:01

and you can even leverage the children and automatically fill-in the include-children

wilkerlucio02:09:18

probably better to have both as complementary (concat or something)

tony.kay02:09:56

I think I’ll start with just support for protocols. The CSS has some twists that I’m not sure make it much advantage to augment. The children cannot automatically be reliable, since you might have CSS children that are stateless

wilkerlucio02:09:35

yeah, that's why I suggest as complementary, so if it's not part of your query, you can add on a :css-children key

tony.kay02:09:44

although :children could be extended to auto-detect…does it have a query? does it have CSS?

wilkerlucio02:09:58

that sounds interesting too

tony.kay02:09:00

then you’d just list them all

tony.kay02:09:35

but all your components would have to be cljc then 😞

tony.kay02:09:45

which would break for those not doing so

tony.kay02:09:58

to detect at macro expansion

tony.kay02:09:07

I’d have to have the components avail on the clj side

wilkerlucio02:09:11

oh, good point

tony.kay02:09:22

although…all of these are methods

tony.kay02:09:34

they can run code at runtime…I’m already doing that to get initial state right

tony.kay02:09:06

I could compile their bodies to include the definition of children

tony.kay02:09:18

I’ll start with :protocols, it doesn’t hurt anything, and doesn’t really complicate anything either.

tony.kay02:09:40

prob a 5-min change

tony.kay02:09:47

@wilkerlucio got a suggested clojure spec for that 🙂 ?

wilkerlucio02:09:15

nope, all good I guess 🙂

wilkerlucio02:09:35

but lately I've being very inclined to use more namespaced keywords everywhere

wilkerlucio02:09:50

you could use some specs to validate the input for the defsc macro

tony.kay02:09:07

I am already doing that

tony.kay02:09:26

I was just thinking my spec chops are not quite up to a spec for what we just described in :protocols

wilkerlucio02:09:11

just declare it as a (s/coll-of any?) for now

tony.kay02:09:25

a sequence of one or more of an optional static symbol, followed by a namespaced symbol, followed by one or more method bodies…

tony.kay02:09:15

I’m going to need to parse it…might as well do it…I aleady wrote the extension…it was trivial

tony.kay02:09:35

but handling Object seems very desirable. Makes it fully a replacement for defui

wilkerlucio02:09:48

(s/cat :protocol-name symbol? :protocol-methods (s/+ ::protocol-method))

wilkerlucio02:09:04

it really feels like Clojure should provide specs for protocols

tony.kay02:09:27

yeah, but I’m happy with “is it a list” for that bit

tony.kay02:09:04

can you do s/? inside of s/cat?

tony.kay02:09:02

(s/cat :static (s/? #(= 'static %)) ...)

tony.kay02:09:47

prob need a nested s/def

tony.kay02:09:03

I’ll play with it, perhaps it isn’t that bad

wilkerlucio02:09:50

yes you can do s/? on cat 😉

wilkerlucio02:09:44

(s/cat :static (s/? #{'static}) :protocol-name symbol? :protocol-methods (s/+ ::protocol-method))

tony.kay02:09:41

wrap that in s/*?

wilkerlucio02:09:08

yup, might need some tweeks, you can use (s/spec) to break on some situations

wilkerlucio02:09:24

but I didn't used regexp specs much, but looks a good place for it

wilkerlucio02:09:41

I think s/* is more apropriated on this case than coll-of

wilkerlucio02:09:30

and coll-of wouldn't work, because they are in flat list (concated)

tony.kay02:09:04

(s/def ::static #(= 'static %))
  (s/def ::protocol-methods (s/+ list?))
  (s/def ::protocols (s/* (s/cat :static (s/? ::static) :protocol symbol? :method ::protocol-methods)))

  (s/conform ::protocols '(static A (x []) (y []) B (f [])))

tony.kay02:09:07

seems to work

tony.kay02:09:19

=>
[{:static static, :protocol A, :method [(x []) (y [])]}
 {:protocol B, :method [(f [])]}]

tony.kay02:09:08

A little group-by on protocol, and that makes it pretty easy to get to Object

wilkerlucio02:09:34

I was playing here too:

tony.kay02:09:36

oh, and sort by symbol for the type-A in us all

wilkerlucio02:09:36

(s/def ::exp (s/cat :static (s/? #{'static})
                      :name keyword?
                      :numbers (s/+ int?)))

  (s/conform (s/cat :protocols (s/* ::exp)) [:a 2 3
                                             'static :b 3])

wilkerlucio02:09:55

=> {:protocols [{:name :a, :numbers [2 3]} {:static static, :name :b, :numbers [3]}]}

tony.kay02:09:34

OK, gotta walk the dogs, they’ve been bored for hours. Thanks for the help!

wilkerlucio02:09:42

I would breakt your organization just a little bit different:

wilkerlucio02:09:09

(s/def ::static #{'static})) ; <- shorter, and generator friendly
  (s/def ::protocol-method list?) ; <- singular here
  (s/def ::protocols (s/* (s/cat :static (s/? ::static) :protocol symbol? :methods (s/+ ::protocol-method))))

  (s/conform ::protocols '(static A (x []) (y []) B (f [])))

tony.kay02:09:13

the first does…not sure about the singular bit

wilkerlucio02:09:38

then you can make the protcol-method more complicated if you want

wilkerlucio02:09:45

this leave the many decision to later

tony.kay02:09:50

ah, ok, yeah

wilkerlucio02:09:04

also, make the ::protocols cleaner to read (you see the regex entirely there)

tony.kay02:09:39

great. Thanks again 🙂

wilkerlucio02:09:46

no problem, good walk on 😉

tony.kay03:09:14

@wilkerlucio OK, :protocols added and on clojars with beta10 SNAPSHOT.

tony.kay03:09:34

oh…the ident param, that was the other thing…I should do that 🙂

tony.kay04:09:10

pushed again. No longer uses :table and :id, but instead uses :ident. At present, does not yet support a function.

tony.kay04:09:45

So, that covers all defui cases, with the following exceptions: - Complex queries (beyond regular props and regular joins) - Idents that need to run real code against props to figure out the ID The latter is easily fixable. The former might be. I am thinking about using :query and allowing any query. I can extract the props and children from such a query, which can still allow sanity checking. So, I guess consider defsc to still be unstable in API. Any additional feedback is welcome.

tony.kay06:09:31

@wilkerlucio CSS support added. props/children changed to :query.

tony.kay06:09:20

I cannot make ident accept a function without breaking sanity checks. I need to know what the ID prop and table are called, and arbitrary code execution in a macro to try to eval that is not possible. So, I think defui is how you get that kind of ident.

tony.kay06:09:13

Technically, you could still use defsc, not specify :ident, but instead put it in :protocols. That would disable some error checking, but would make it work.

tony.kay06:09:11

I’ve tested that scenario, and it works fine.

tony.kay06:09:58

So, I think that makes defsc almost completely general, a bit more opaque, but also a lot safer to use: - Cannot forget the static on main protocols (Ident, IQuery, InitialAppState, IForm, and CSS) - Prop destructuring is verified against query - Initial state is verified against query The one case that it still won’t handle is a union query.

mitchelkuijpers12:09:45

Really loving that defsc support css, would still find it cool to make it extensible with own keyword somehow the case with protocols looks pretty ugly imho

roklenarcic14:09:06

is there a way to specify refresh keywords to a post-mutation?

cjmurphy14:09:26

@roklenarcic: You can refresh from the transact! or the load(which calls transact! for you). In the latter there is :refresh that you can set. You can't refresh (also known as 'follow on read') from within a mutation.

roklenarcic14:09:28

Hm well the load has the refresh set, but it seems to apply refresh after load, but before the post-mutation

roklenarcic14:09:14

it probably applies refresh parameter to mutation that does the load, but not to post-mutation

cjmurphy14:09:21

Just looking at the load documentation: "`refresh` - A vector of keywords that will cause component re-renders after the final load/mutations."

cjmurphy14:09:41

That does explicitly say 'post mutation' unfortunately. But I would assume that the refresh comes after the post-mutation - as post-mutation instruction is bundled up together in the load call.

cjmurphy14:09:33

Are you sure your refresh is working? I often find when in trouble that refreshing using an ident always works.

roklenarcic15:09:29

If I run the same load twice it works

roklenarcic15:09:59

the second time refreshes the other component

roklenarcic15:09:24

"But I would assume that the refresh comes after the post-mutation - as post-mutation instruction is bundled up together in the load call" -> I'm not sure that assumption is correct. Seems to me that post-mutation is a whole another mutation.

cjmurphy15:09:33

Okay, yes I was just guessing based on what I would want to see.

roklenarcic15:09:39

🙂 I was disappointed that it didn't work the way I wanted it to, as well 🙂

cjmurphy15:09:06

But if my assumption is wrong I can't see how you could refresh after a post mutation. So I'm sticking with it for now... 🙂

cjmurphy15:09:07

The transact! is done for you so you can't do a follow on read. Only makes sense that the :refresh is all about doing the follow on read that you can't.

roklenarcic15:09:40

If you wanted you could skip using the refresh vector in load if post-mutation is present and then use the refresh vector when transact! is called for post-mutation

cjmurphy15:09:51

But the post-mutation is not a query - it is a mutation. You need a query to put the follow-on read. The query being the thing that transact! takes.

roklenarcic15:09:28

right, so you transact! ['(post-mutation-symbol post-mutation-params) ~refresh-vector]

cjmurphy15:09:04

Yes - 'parse expression' is a better term than query I guess: '[(do/this!) (do/that!) :read/this :read/that]

cjmurphy15:09:46

If you are doing a transact! rather than a load then :refresh doesn't come up.

roklenarcic15:09:28

I'm doing a transact! that contains a load-action which has :refresh :post-mutation

cjmurphy15:09:48

Right - that's the one you need to use if you want to load but you are already in a mutation.

cjmurphy15:09:28

That's a good point you have there - you should expect the refresh to work even though you are in a mutation. So I was wrong what I said before about not being able to specify a refresh/follow-on from within a mutation. You ought to be able to have the refresh work for you.

cjmurphy15:09:55

I do have one case of what you are doing in code that I have. It uses an ident and presumably it works. (And also presumably the :refresh [ident] is actually needed).

roklenarcic15:09:44

I think the post-mutation refreshes the component itself

roklenarcic15:09:15

the thing is that my post-mutation modifies the side-bar, which doesn't get rerendered

roklenarcic15:09:40

unless I click the thing again, in which case the :refresh key in load action refreshes it

roklenarcic15:09:59

so the key in my refresh vector is a correct one

cjmurphy15:09:00

Well follow on reads are actually the exception. this is passed into a transact! so the component you did the transact! in is automatically refreshed - there's no need for follow-on malarkey in the usual case of where the event is happening at that component and all components below refresh (automatically).

cjmurphy15:09:49

Usually you make sure you are doing transact! high enough in the tree (ie towards the root, the top) that refreshing is just automatic.

cjmurphy15:09:46

In an extreme case you could do all your transacts! from the root component and you would be guaranteed to have no refresh problems.

roklenarcic15:09:50

I guess I could move the first transact higher

roklenarcic15:09:54

see what happens

roklenarcic16:09:35

btw has anyone tried to use load-field-action with :params specified?

tony.kay16:09:51

@roklenarcic Regarding load-field with params. Om Next has a bug that will break that. It is fixed but unreleased.

tony.kay16:09:34

Regarding :refresh. It is a vector of keywords that get added to the load mutation transact. The intention is for that to indicate what should be updated when the load completes, including after the post mutation. Big caveat: You must name a keyword whose actual data value has changed or it will still short-circuit the update. Now, if that still isn’t getting you a refresh, then it should be considered a bug. Technically post mutations are not run in transact! because they are run as part of the internal network processing. So, instead the keywords you list on refresh are added to the render queue directly.

tony.kay16:09:13

@mitchelkuijpers Try making a macro that is extensible and use it from clojurescript/figwheel/hot reload. It is miserable. I think :protocols is the best option, and since you shouldn’t need it much, I’m not sure it matters in the large.

roklenarcic16:09:57

I named the keyword that is the toplevel keyword for the whole screen. I'll keep looking

tony.kay16:09:39

You should not name an arbitrary keyword. Name a keyword for specific data that actually changed.

tony.kay16:09:15

though I would assume that a join keyword for the screen in question should be sufficient.

tony.kay16:09:50

post mutations are run just above that on line 410

tony.kay16:09:20

So, if what you are doing is not working, then either there is a bug that I don’t see, or you’re sending something incorrectly to :refresh

roklenarcic16:09:33

you were right

roklenarcic16:09:14

I had a bug, where I accidentally embedded :refresh element too deep

tony.kay16:09:26

welcome 🙂

tony.kay16:09:47

glad it wasn’t a bug in fulcro…those always feel like anti-progress, especially when in code that is that critical to functionality

roklenarcic16:09:09

FWIW one thing I couldn't get to work though, was load-field-action where the field keyword was :parent and the component had a recursive query [:id :name {:parent '...}]. denormalize* threw an error ... is not Sequable when it tried to process my return {[person-by-id 1] {:parent {:id 2 :name "parent" :parent nil}}}. It's possible I was doing something wrong again, but I didn't see any recursive query samples in the documentation, so I don't know how many people tried to run recursive queries with load-field-action yet.

roklenarcic16:09:41

I've moved away to another solution already

tony.kay16:09:27

Your return value looks off to me, but the query looks fine

tony.kay16:09:44

you asked for id, name, and then a recursive parent

tony.kay16:09:05

so, that would be a bug in Om Next if it is broken…here are the things I see possibly wrong that could be typos: - :person/by-id is missing a colon…prob a typo here - Returning :parent nil is possibly wrong…try eliding :parent nil altogether - You’re missing :id and :name in the top-level map, but that shouldn’t crash it

roklenarcic16:09:10

since it was load field, the actual query to remote was {:parent '...} only

tony.kay16:09:44

hm….no, that doesn’t sound right

tony.kay16:09:10

oh, you said load-field on :parent?

tony.kay16:09:13

no, you cannot do that 🙂

roklenarcic16:09:38

cannot load field on recursive queries?

tony.kay16:09:59

the '... part isn’t sufficient information for anything to work

tony.kay16:09:34

how deep is your real recursion need?

tony.kay16:09:38

If you need to lazily load each level, I’m afraid you’d have to code three different defui, one for each level. You could use shared functions to provide impl

roklenarcic16:09:46

I got a feeling recursion doesn't mesh well with om model of denormalized database, the explosion of data can be immense

roklenarcic16:09:26

also circular idents

tony.kay16:09:34

the recursion is fine…it works pretty well. There are some things you have to be careful about. In this case, the problem is you’ve narrowed the query to the point of being informationless 🙂

tony.kay16:09:50

circular idents are handled fine

tony.kay16:09:56

though you have to be careful with rendering them

tony.kay16:09:08

infinite loops are detected and stopped, just like Datomic pull

tony.kay16:09:12

To render a loop correctly, you need to use computed props to track depth or refresh gets wiggy

tony.kay16:09:25

for example: person and spouse. Definite circular loop there

roklenarcic16:09:24

I believe I saw an example with :friend somewhere

tony.kay16:09:25

Say you render A with spouse B, but it is the same code with a recursive query. No problem on initial render; however, if you refresh B with a transact, the context will start at B and you’ll render a new A, ending up with A B A on the UI

tony.kay16:09:42

then refresh that last A, and you’ll get another B

tony.kay16:09:45

and so forth

tony.kay16:09:50

the fix is to track depth with computed

tony.kay16:09:14

so when you refresh B the first time, you know you’re already 2 deep and can short-circuit the extra depth render

tony.kay16:09:29

queries are relative, and that makes recursive queries a little tricky

roklenarcic16:09:57

hm, but how does the props get generated when denormalized without going infinite

tony.kay16:09:18

the algorithms watches for “idents already seen” as it goes

roklenarcic16:09:31

does it use mutable JS maps and cross reference

tony.kay16:09:34

if it detects a loop, it returns the ident instead of following it

tony.kay16:09:47

no mutation…just accumulation during recursion

tony.kay16:09:46

(fn process-query [query data idents-seen] ... (process-query subquery data (conj idents-seen x))) kind of thing

roklenarcic16:09:09

you have to watch out for unresolved idents when parsing props then

tony.kay16:09:04

there are probably a number of “bad data” cases that could cause problems. With an infinite number of ways of screwing up an arbitrary data structure, I’m sure you can cause bugs with malformed data 🙂

tony.kay16:09:17

An ident that points to nowhere is almost certainly a bug in your code. normalization will always put stuff in a table when it makes an ident, even it the ID comes back nil

tony.kay16:09:15

but yes, If you screw up an ident that you create and point it to nowhere, then you could possibly see unexpected behavior. I’m not sure that results in anything horrible…probably just empty query results.

roklenarcic16:09:38

kinda like the load markers, basically cause an empty list at that point in the tree

tony.kay17:09:32

load markers don’t cause an empty list (though they may displace a list while loading is in progress)…returning an empty list does 😉

tony.kay17:09:38

their intention is to let you render a loading view in place of said item or list…and you turn them off when that isn’t what you want to do

mitchelkuijpers17:09:38

That's true @tony.kay and can always create a macro if needed

roklenarcic17:09:25

is there some way to access what was loaded in post-mutation?

tony.kay17:09:00

what do you mean? You have the entire app state, and you issued the load. If you need more info from the call site, pass it as a post mutation parameter

roklenarcic17:09:36

That's what I do now, I pass the location where the loaded thing will be as a parameter

roklenarcic17:09:00

I load something then move it into a list in post-mutation

tony.kay17:09:32

you mean like you’re loading an item, and are adding it to an existing list?

roklenarcic17:09:24

I move the ident in post-mutation and prepend it to an existing list

tony.kay17:09:35

sure. You know about integrate-ident?

roklenarcic17:09:47

I call it in post-mutation

tony.kay17:09:10

then yes, unless you make a mutation that is site-specific (where the incoming item location would be well known already), then you have to pass it as a parameter

tony.kay17:09:25

So, some thoughts: loads are targeted at root by default, and you :target them…but normalization is moving your item into a table.

tony.kay17:09:28

nvm…that doesn’t help. you’re needing to know something like “where did the transact run?“, right?

tony.kay17:09:32

which is :ref in a normal mutation. This came up over the weekend, too. I’ve been resistant to adding :ref to post mutations because you’re pretty far removed from transact and the component, but it is technically feasible to pass it through…though it is a rather large and invasive change because of the length of the chain from transact, through loads (and async callbacks), back to the post mutation.

roklenarcic18:09:24

hm I defined a mutation which has a (conditional) load-action which has the same mutation as a post-mutation. The recursion doesn't seem to work

roklenarcic18:09:00

should this work or is this forbidden?

gardnervickers19:09:29

@tony.kay We have several spots where we supply ref through our fallback handlers and post-mutation-params to get around not having ref. It would be a welcome change to have as much of the transact env intact as possible in both fallback handlers and post-mutation‘s.

wilkerlucio19:09:25

@gardnervickers I agree with you, I have the same problem in a lot of places. to be honest I would love to see a more full-featured enviroment in the post-mutation, as a dev I like to have most information possible available, I find the limited environment on post-mutations restrictive

tony.kay21:09:30

@roklenarcic should work fine. It isn’t recursion (though you could cause yourself an infinite loop). Make sure you are using the same (namespaced) symbol.

tony.kay21:09:06

@gardnervickers @wilkerlucio So, I’m getting that. :component is out, but any raw data would be fine. And reconciler is even possible.

gardnervickers21:09:58

:component and :ref would help us a lot.

tony.kay21:09:27

hm…:component can’t go in app state

tony.kay21:09:39

I could try placing it as metadata on the load marker…

tony.kay21:09:34

So, why do you want :component? You have :refresh and an arbitrary data mutation…so you hsould be able to change anything on the UI you need…or am I missing something?

gardnervickers21:09:00

Really what we want is the query and ident from :component

gardnervickers21:09:09

So we can use db->tree/`tree->db`

gardnervickers21:09:19

But that’s not critical

tony.kay21:09:21

ah, ok, that is sane 🙂

tony.kay21:09:15

I can put anything at all from the load request into the env. The load marker is available and it has all of that. But there are two sets: the ident/query of the component that ran the load (which may or may not be present depending on how the load was triggered) and the keyword/ident + query of the load itself.

tony.kay21:09:25

seems like making :ref and :reconciler be just like normal transactions (the triggering component’s), and perhaps drop the load marker bits into env under separate keys…perhaps as :load-request

tony.kay21:09:48

I may start on that later tonight, so if there is any more feedback around it, please leave it here

roklenarcic23:09:17

Yeah that "recursive" call should work fine, but the second load stays in :fulcro/ready-to-load