Fork me on GitHub
#fulcro
<
2017-12-15
>
levitanong05:12:09

Hi all, is there any known gotcha for passing computed props to routers?

claudiu10:12:25

@levitanong havent tried, but curious what is the use case ?

levitanong10:12:37

@claudiu passing a google map instance down. Not doing that anymore, but still curious. When I was trying to do it, the computed props were nil.

tony.kay13:12:05

pre-written routers don’t pass computed props through…they are two levels deep. It would be easy to do so, I just had not thought of it @levitanong.

timovanderkamp15:12:23

Hey, I'm currently working with fulcro forms and im trying to figure out what the way to go is with handling ui changes in a subform. it seems to be a bit of an overkill to rerender the whole form when for example typing in a subform.

timovanderkamp15:12:24

The issue that I'm currently facing is that i want to call commit-to-entity with the root-form when making changes somewhere in nested subforms

tony.kay15:12:12

@timovanderkamp rendering is so optimized that you should never worry about that unless you see an actual perceptable problem.

tony.kay15:12:59

not much “rendering” happens…just the changes. VDOM diff also is faster once in React production mode….so, should be fast

timovanderkamp15:12:07

It seems to be a problem in Safari

tony.kay15:12:22

Ah,ok. So, why commit on keystroke?

tony.kay15:12:39

shouldn’t commit be at the very end?

timovanderkamp15:12:38

oh im sorry, i mean form/set-field

tony.kay15:12:57

Hm, so, that’s just a prop update. That should be fast.

timovanderkamp15:12:17

I am passing the root-form as computed to the sub-form and then call commit-to-entity with that root-form

tony.kay15:12:26

Are you using unions (routers) to make sure stuff that isn’t on the screen isn’t getting queried?

timovanderkamp15:12:09

So if only the sub-form is being rerendered, the root-form wont contain the changes that have been made

tony.kay15:12:56

ok, so some conceptual clarifications. The root form and subform each track their own changes through the normal entity props. Forms cache the original (from build form) state that was in your entities.

tony.kay15:12:01

everything else is basically diff

tony.kay15:12:06

or validation

tony.kay15:12:27

so, the root form doesn’t have anything to do with “containing” changes of subforms

tony.kay15:12:05

they are separate things…subforms (the concept) are so that the top-level form’s validation knows what other forms need to be checked when you ask globally “is this form valid?”

timovanderkamp15:12:44

Do you have to do commit-to-entity twice then if you create a new subform and make changes to it afterwards. One for the add-relation operation and one for the entity-update of the subform?

tony.kay15:12:18

Have you read the stuff on writing your own commit?

tony.kay15:12:29

short answer: no

timovanderkamp15:12:14

No I haven't read that yet

tony.kay15:12:22

diff is smart enough to figure out all of the changes at any point. The lifecycle is here: http://fulcro.fulcrologic.com/guide.html#!/fulcro_devguide.O02_Forms_State_Lifecycle

tony.kay15:12:50

If you have a set of forms declared, then running a diff (which is what commit does) will recurse into the subforms.

tony.kay15:12:06

The form is “dirty” if the root or any children have changes or tempids

tony.kay15:12:15

root is relative

tony.kay15:12:31

(so if you commit a subform, it doesn’t walk backwards)

tony.kay15:12:47

The forms support is really quite basic. It’s just a cache of the original state (from when you initialized forms) until commit (when the state is re-cached). Commit is just a diff of the two. Validation is just…well, validation on a specific current value.

tony.kay15:12:24

The “current value” is always what is in your entity. So, the diff is between what you told it to start with, and what it is now. It’s up to you to decide when that is out of sync with respect to a server. For example, if using websockets, things get a lot more complicated, but you might also want to do optimistic concurrency where you carry versions along with entities to prevent overwrites on the server. The minimum delta support of forms is a way around that (last write wins is typically OK as long as a minimal delta of real changes is what is sent)

tony.kay15:12:12

But none of that has to do with UI rendering speed….unless you’re doing more work than is needed. Most speed problems are due to over-querying in the UI….e.g. not using defrouter to split up your screens.

timovanderkamp15:12:14

What i'm currently doing is having a form which contains a list of subforms, if i add another subform - the root-form will rerender which is obvious. But whenever i update that subform i want to rerender just the subform, when saving this subform i want to call commit-to-entity on the root-form (which i obtain by prim/computed in the subform) and expect it to figure out all the changes that happened just like you said >diff is smart enough to figure out all of the changes at any point.

timovanderkamp15:12:54

But because i only rerendered the subform the only diff it figured out was the add-relation of the new subform. And not the changes made to the subform afterwards

tony.kay15:12:08

use a callback

tony.kay15:12:19

depending on how you did computed, you probably closed over the old state

tony.kay15:12:45

or better, put the commit button on the root so you don’t even need a callback

tony.kay15:12:36

so, there is one other thing

tony.kay15:12:51

unless the root form refreshes from Fulcro, it’s props will be stale

timovanderkamp15:12:06

I'm using a callback, but probably not the right way now i think of it.

tony.kay15:12:18

yeah, that’s your problem

timovanderkamp15:12:31

ah i think i found my problem

tony.kay15:12:40

on field blur you need to force a refresh of the parent form, so it’s props are updated

timovanderkamp15:12:19

Indeed, or i can pass the subform to the callback, and compose it with the old state

tony.kay15:12:43

yeah, the rendering refresh of the subform is updating its props, but the parent form has a cached version of those

timovanderkamp15:12:03

Thanks tony for helping me out

tony.kay15:12:08

so, you do need to “checkpoint” it up somehow…sure!

timovanderkamp15:12:53

Works smoothly now

tony.kay16:12:52

So, note to general users. The defsc messages are helpful, but don’t word wrap in Figwheel’s heads-up display. I’ve submitted a PR to figwheel, but until that is merged you can add this to your HTML file to get better error support when you make mistakes in defsc:

<style>
            #figwheel-heads-up-content-area pre { white-space: pre-wrap; }
</style>

tony.kay16:12:17

I’ve added that to the HTML files in fulcro’s lein template for 2.x and released it on clojars

tony.kay17:12:38

Fulcro 2.0.0-RC1 on clojars

wilkerlucio17:12:28

@tony.kay I'm here on my journey for a generic error system implementation, I'm having just some difficulty to incorporate network errors on the process, I want ask if you have some advice on that, let me explain:

wilkerlucio17:12:40

I'm using the ptransact! to handle pre and post actions to mutations, the pre-action is to set some information on the entity to tell that a mutation is happening (kind like load markers), then I call the actual operation, and then I have a finalize operation, that checks if the result of the operation was success or error, and change the data according to it

wilkerlucio17:12:54

looks like this:

wilkerlucio17:12:58

(fp/ptransact! this `[(start-mutation {})
                          ~(list mutation params)
                          (finish-mutation ~{:ok-mutation    ok-mutation
                                             :error-mutation error-mutation
                                             :input          params})])

wilkerlucio17:12:23

this works fine for errors that come from the graph, done there

wilkerlucio17:12:33

what I'm trying to handle now is the network errors on the same process

wilkerlucio17:12:10

the problem is that I can't figure a way to handle the network error and get the context where the error happened

wilkerlucio17:12:18

I was thinking about some options:

tony.kay17:12:36

network error…not server error, right?

wilkerlucio17:12:38

1. try to use tx-fallback: don't work because tx-fallbacks don't work with ptransact! aparently

wilkerlucio17:12:52

yeah, network error, like a 500, or server down

tony.kay17:12:13

so fallbacks should work with ptransact…if they don’t then that is a bug 😕

wilkerlucio17:12:16

2. the global network error handler: don't work because at that point I have no idea what mutation or ref caused the problem

wilkerlucio17:12:24

3. checking for network error on my finish-mutation: not reliable, the only think I figure could be used for that is the global :fulcro/server-error, the problem is that I might have multiple operations from multiple componetns running at same time, and that only shows what happened last

tony.kay17:12:54

you have multiple remotes?

tony.kay17:12:20

so, once a network error happens, your tx is over. It’s all queued together

tony.kay17:12:31

the thing is, with network error you cannot know how much of tx ran, and how much didn’t

tony.kay17:12:39

distributed systems problem in general

wilkerlucio17:12:49

that's ok, I consider this a whole process, the problem is between ptransacts

tony.kay17:12:01

between steps of ptransact you mean?

wilkerlucio17:12:15

wait, I'm getting confuse, what solution we talking about, 3?

tony.kay17:12:29

not talking solution yet…trying to establish problem statement 🙂

wilkerlucio17:12:52

ok, so, I'm doing a "localized" error handling

wilkerlucio18:12:04

all error state goes into the ref that started the mutation process

wilkerlucio18:12:27

for cases where the network gets there (and a problem happens inside of it), the normal flow works fine

wilkerlucio18:12:44

the issue is when network failures occur, because I can't find how to associate that error with the ref

tony.kay18:12:05

OK, so one thing is this: if the network is down, that is a global problem, not a local one.

wilkerlucio18:12:13

I guess tx-fallback seems the correct solution here, but its not working, I can double check, but I didn't triggered the handler last time I checked

wilkerlucio18:12:27

yeah, but the opreation started from somewhere, and I would like to report it there

wilkerlucio18:12:01

because that's where the user is looking at

tony.kay18:12:19

so, fallbacks probably are broken in ptransact because the transform fails to understand them….actually, if you intersperse them in front of the remote they should affect, they might work ok.

tony.kay18:12:49

For network errors though, I would suggest an overlay over the entire app letting them know the problem. That way you don’t pepper your entire app with fallback handlers.

wilkerlucio18:12:56

I should try it more, I only did a quick one on that and fought they weren't supported

wilkerlucio18:12:04

but since you say they should, I wanna try harder

tony.kay18:12:35

So, the way ptransact works is it changes your tx. If there are non-remote calls (which fallback will look like), then it clusters those together until it finds one that is remote:

tony.kay18:12:16

[(local) (local) (remote) (local) (remote)] becomes: 1. [(local) (local) (remote)] wait…. [(local) (remote)]

tony.kay18:12:41

the fallbacks have to be on the tx they should handle, but they don’t look remote…so if you put them after, they’ll get mis-grouped

tony.kay18:12:22

I do consider that a “bug” of a sort…not sure how those should be grouped for ptransact…should they all get copied to all?

tony.kay18:12:23

[(l) (r1) (f1) (r2) (f2)] => [(l) (r1) (f1) (f2)], [(r2) (f1) (f2)]

tony.kay18:12:44

that’s what I do for follow-on reads…copy them through

wilkerlucio18:12:40

yeah, seems like fallback needs to be a special case if we want to support it

wilkerlucio18:12:51

to be clustered with the remote instead of after

tony.kay18:12:06

so, putting them before should work now, and we could just document it that way

tony.kay18:12:30

grouping them all and including them on all is also easy

wilkerlucio18:12:34

you mean like:

wilkerlucio18:12:36

(fp/ptransact! this `[(start-mutation {})
                          (fulcro.client.data-fetch/fallback {:action mutation-network-error})
                          ~(list mutation params)
                          (finish-mutation ~{:ok-mutation    ok-mutation
                                             :error-mutation error-mutation
                                             :input          params})])

tony.kay18:12:37

splitting them up is too confusing

wilkerlucio18:12:52

humm, doesn't seem to work here

tony.kay18:12:35

look at the tx that ran first…did it have start, fallback, mutation?

wilkerlucio18:12:37

I got this log [fulcro.client] Fallback triggered, but no fallbacks were defined.

wilkerlucio18:12:10

what you mean it have start, fallback, mutation?

tony.kay18:12:20

oh…no, I just looked at the source

tony.kay18:12:26

fallback is considered a remote op

tony.kay18:12:32

so splitting is just broken

tony.kay18:12:39

fallback is ending up on a tx by itself

tony.kay18:12:31

So, which grouping option sounds right to you?

tony.kay18:12:42

all in each, or prefixed fallbacks

wilkerlucio18:12:58

can you give me examples?

tony.kay18:12:04

I did, above

tony.kay18:12:22

[(l) (r1) (f1) (r2) (f2)] => [(l) (r1) (f1) (f2)], [(r2) (f1) (f2)]

tony.kay18:12:27

is all in each

tony.kay18:12:34

r means remote, f means fallback

tony.kay18:12:37

l means local

tony.kay18:12:29

the prefix interpretation of that would be: [(l) (r1)], [(f1) (r2)] f2 lost

wilkerlucio18:12:06

[(l) (r1) (f1) (r2) (f2)] => [(l) (r1) (f1)], [(r2) (f2)]

tony.kay18:12:47

so, I tend to prefer “copy all in each”

wilkerlucio18:12:14

doens't sound weird that you replay all error handlers on all? I have no strong opinion on this, but I found it a bit counter-intuitive

tony.kay18:12:19

the user/reader/developer coming after you should not have to read through all the mutations to figure out local semantics

tony.kay18:12:07

as a reader of code, [(a) (b) (fallback) (c) (d) (fallback)] is ambiguous

wilkerlucio18:12:15

I can't actually think on a case where I would want more than one network error handler per transaction, so, thinking on that the copy to all seems to make sense

tony.kay18:12:27

if only (b) is remote, it has one meaning, but if others are remote, it has a different one

tony.kay18:12:48

I can argue the other way 😜

wilkerlucio18:12:58

I'm agreeing with your approach 😛

tony.kay18:12:18

in example above: two different remotes…`b` and c are remote, but you want to handle the errors from the remote for b differently than c

tony.kay18:12:53

This is a lib decision that has long-term implications…I’d rather play devil’s advocate a bit 🙂

tony.kay18:12:37

1. implementation-wise it’s a pain to split that way. But that is me just feeling lazy 😜 2. Network errors are more of a global concern..the app cannot work when they are happening, but with multiple remotes perhaps you have a pretty complex micro-services thing where some things can just work while others fail.

tony.kay18:12:02

3. Fallbacks are very fine-grained, and I don’t like them for that reason. The lead to lots of extra code…some on each tx. Seems very tedious. Errors should happen rarely, and peppering a lot more code around for something that rarely happens (and isn’t very recoverable when it does) sucks.

wilkerlucio18:12:54

the fallback way I'm doing is something I'm only writing once

wilkerlucio18:12:20

I'm kind creating a system here where I can choose if I want the operations to be optimistic or pessimistic

wilkerlucio18:12:27

(per operation)

tony.kay18:12:57

Fallbacks have their uses…don’t get me wrong on (3)…I just don’t like using them as a defensive tool all over the place.

tony.kay18:12:26

also, like you said: wrap the logic in something else, and you never have to look at them

wilkerlucio18:12:49

I see too many errors happening here, so I can't ignore, deploy is constant, and breakages happen more often than they should...

wilkerlucio18:12:34

but I have to let the system users know it's currently broken, that's the whole point, but I'm not closed to the idea of reporting those globally, it's just that if I can report in the place where the user is already looking, I think that's a better UI

tony.kay18:12:18

ok, so logic is technically going to be this:

tony.kay18:12:54

In a given ptransact!: 1. take up until the first remote call. 2. Out of the rest, take as long as it is a fallback 3. 1+2 is next tx 4. Defer rest, (running is back through step 1)

tony.kay18:12:25

So, [(l) (r) (l) (f)] will not work.

tony.kay18:12:39

but [(l) (r) (f) (l)] is fine

tony.kay18:12:12

I guess (2) could be to reorder fallbacks automatically to their closest remote

tony.kay18:12:20

so both would be ok…

tony.kay18:12:39

just makes the alg messier

wilkerlucio18:12:40

I like that, this is how I was thinking when you suggested the clustering fix

wilkerlucio18:12:11

because you look at the f and now it's related to the remote just before it

tony.kay18:12:59

OK, so you feel like the sort-before-split is desirable?

tony.kay18:12:43

1. Sort tx so that fallbacks cluster just to the right of the closest remote 2+ is like 1+ above

wilkerlucio18:12:42

humm, I guess this is more prone to bugs, not against it, but I feel like it's ok to leave as user responsibility to place it right

wilkerlucio18:12:24

I'm afraid that re-sorting might have unexpected consequences

tony.kay18:12:25

actually, I don’t have to sort so much as re-do step 1 on the remainder and pull fallbacks out of that

tony.kay18:12:07

ok, give me 30 mins or so…I can try that now

wilkerlucio18:12:33

you are the best, thanks man 🙂

tony.kay18:12:31

Both of these specs now pass, so it should be good (and all prior tests still pass):

"Clusters prefixed fallbacks with following remote op"
    (prim/pessimistic-transaction->transaction `[(fulcro.client.data-fetch/fallback {:x 1}) (r1 {:x 1})])
    => `[(fulcro.client.data-fetch/fallback {:x 1}) (r1 {:x 1})]

    "Clusters postfixed fallbacks with closest prior remote op"
    (prim/pessimistic-transaction->transaction `[(r1 {:x 1}) (L)
                                                 (tx/fallback {:x 1})
                                                 (L2) (r2)
                                                 (fulcro.client.data-fetch/fallback {:x 2})
                                                 (L2) (L)])
    => `[(r1 {:x 1}) (tx/fallback {:x 1})
         (fulcro.client.data-fetch/deferred-transaction
           {:remote :remote
            :tx     [(L) (L2) (r2) (fulcro.client.data-fetch/fallback {:x 2})
                     (fulcro.client.data-fetch/deferred-transaction {:remote :rest-remote
                                                                     :tx     [(L2) (L)]})]})]

tony.kay18:12:37

And how’s that for a time estimate? 25 mins out of 30? 😜

wilkerlucio19:12:16

very accurate 👏

wilkerlucio19:12:24

trying it now 🙂

adamvh19:12:51

@tony.kay i implemented a datepicker as a fulcro component. is that something you'd be interested in packaging with fulcro itself, or does it make more sense to release it as a separate library?

tony.kay19:12:39

@adamvh So, there may already be one packaged…I don’t remember. When it was Untangled, we had a lib called untangled-ui. I’m thinking of splitting some of the Fulcro lib off that way again. I don’t have time to maintain those bits, and they aren’t really part of the core mission. It would be nice to have such a spot for people to contribute into.

tony.kay19:12:16

I combined it all for ease of maintenance, but I don’t like the things like bootstrap mixed in…there’s no reason to give it preferential treatment (or busy my life with UI tweaking CSS kinds of things).

tony.kay19:12:28

that’s a longer answer than you were looking for, I know 🙂

wilkerlucio19:12:10

@tony.kay tx-fallback been called with success 🙂

tony.kay19:12:28

@adamvh Looks like the old one got lost…I didn’t want to maintain the CSS for it, and like I said, not looking to maintain a UI component library. Don’t have time. That’s why bootstrap support is half-baked. There is also an image clip tool that was meant to turn into an image upload/browsing library. Part of that got lost in the shuffle from Untangled as well.

tony.kay19:12:41

it is still there and shown in the advanced UI Dev Guide section

tony.kay19:12:22

@adamvh How interested are you in joining in on a fulcro library as a maintainer??? 😄 Want to be the guy for fulcro-ui?

tony.kay19:12:07

or you could package on your own…but the short answer is I don’t want to pull more UI components into Fulcro itself.

tony.kay19:12:15

quite the opposite

tony.kay19:12:51

but I like the idea of having UI components for off-the-shelf use. It’s why I did semantic-ui-wrappers as well

tony.kay19:12:09

they have controlled versions of a lot of things, so we don’t have to code/maintain our own

tony.kay19:12:23

bootstrap was an experiment that was kind of time consuming

wilkerlucio19:12:27

@tony.kay just one problem, no ref at the fallback env =/

tony.kay19:12:36

@wilkerlucio you and your refs 😜

wilkerlucio19:12:10

they are vital for the way I'm generalising, but we are very close, I think this is the last missing one

tony.kay19:12:27

I have a call at noon I have to prep for, so open an issue. Should not be that hard to add

wilkerlucio19:12:00

is there a way to send params to fallback? just to see if I can keep progressing until we get that fixed

tony.kay19:12:23

ANything you put in the params map should show up

tony.kay19:12:30

I think that’s how it works

tony.kay19:12:55

there are two special parameters: :action and :execute. The latter is used internally

tony.kay19:12:00

others should just pass through I think

wilkerlucio19:12:15

humm, doesn't seem like it, the param for the fallback is just the map with :error

tony.kay19:12:18

execute should be namespaced but isn’t for legacy reasons

wilkerlucio19:12:33

ah, so I need to send there

tony.kay19:12:42

oh…you’re right

tony.kay19:12:47

here is the place to fix it

tony.kay19:12:59

line 1626 of primitives or so

tony.kay19:12:31

but wait…that does look like it preserves them

tony.kay19:12:34

it just adds in error

wilkerlucio19:12:41

I was doing it wrong

tony.kay19:12:43

new-children (->> children (filter (fn [child] (contains? symbols-to-find (:dispatch-key child)))) (map (fn [ast] (update ast :params assoc :execute true :error resp))))

wilkerlucio19:12:47

I was adding to the original mutation params

wilkerlucio19:12:21

@tony.kay not sure if you wanna change that, but I can't find a way to replace this component with a defsc version:

wilkerlucio19:12:24

(fp/defui ^:once CSS
  static css/CSS
  (local-rules [_]
    (into [[:$pointer {:cursor "pointer"}]
           [:$center {:text-align "center"}]
           [:$right {:text-align "right"}]
           [:$flex {:flex 1}]

           [:* {:margin 0 :padding 0 :box-sizing "border-box"}]
           [:body {:font-family "'Open Sans', 'Helvetica Neue', Helvetica, Roboto, Arial, sans-serif"}]
           [:h2 {:font-size     uc/font-size-18
                 :margin-bottom uc/space-5}]
           [:a {:text-decoration "none"}]]
          (helpers/gen-all-spaces 0 5 8 10 16 20)))
  (include-children [_] components))

wilkerlucio19:12:03

because it pulls the included children from a var, and the regular css is computed, defsc doesn't accept that, and I think we don't have fn versions for the css, or do we? (I tried and got an arity error)

wilkerlucio20:12:33

@tony.kay I just noticed an error been logged because I'm adding a parameter to a load-field

wilkerlucio20:12:44

Error: You attempted to add parameters for #{:abrams.api/shard} to top-level key(s) of [({:>/credit-card-status-history [:card/id :ui/fetch-state :com.wsscode.pathom.core/errors {:card/status-history [:card.status-history/instant :card.status-history/user :card/status :card/status-detail]}]} {:abrams.api/shard "s0"})]

wilkerlucio20:12:12

and this my code: (fetch/load-field this :>/credit-card-status-history :params {:abrams.api/shard shard})

wilkerlucio20:12:29

this is totally legal, and I'm getting the correct response, can you clarify why this validation is there?

tony.kay23:12:50

NOTE: RC2 on clojars. Fixes a history bug and ptransact support for fallbacks.

tony.kay23:12:36

@wilkerlucio That error check is from the days of load-data.

tony.kay23:12:54

well, actually, it isn’t…the generic load is. See here is what you used to be able to do: run an arbitrary query, but you’d want to put params with the correct thing in that query…so it scans the query looking for the right AST node on which to place the params. The newer load code doesn’t let you do that (unless you use the low-level load mutation).

tony.kay23:12:03

but the check is still there…not sure why you triggered it

tony.kay23:12:53

As far as I can tell the actual load code is correct, as is your use of it 😜

tony.kay23:12:29

Ah…seems the field wasn’t set for some reason

tony.kay23:12:31

Also, NOTE, load-field now allows you to use a map or named params…I assume you’ll be pleased by the former.

tony.kay23:12:11

On your CSS. Read the error carefully. Arity problems mean it is supported, and you’re using the wrong number of args. this is closed over from the arg list, so the lambda form of it is :css (fn [] ...)

tony.kay23:12:32

This is in a comment in the docs, but I admit that could be better 🙂 http://fulcro.fulcrologic.com/guide.html#!/fulcro_devguide.M05_Defsc_In_Detail