Fork me on GitHub
#untangled
<
2016-05-20
>
ethangracer00:05:52

hm, ok. I’ll try it again in the morning, might just have been a glitch on my side — tried going back and forth a few times and thought I could reproduce it. some sleep and a night away from the machine might help 🤓

therabidbanana00:05:16

There's definitely something bad going on here, just not sure what - checking development mode again to make sure I'm not crazy, but it seems to work there

currentoor02:05:34

@ethangracer: just in case it helps we were able to consistently see the post-mutations not fire when we had advanced compilation on, and post-mutations consistently worked when we turned off the advanced compilation

currentoor03:05:30

ok in our app post-mutations work for optimizations none, whitespace, and simple. only advanced seems to break it

ethangracer03:05:37

@currentoor @therabidbanana thanks for the help troubleshooting. I thought we werent using advanced compilation on our dev build but I'll double check tomirriw. Any theories on why? That's just bizarre

currentoor03:05:20

well it could be that we have a different error than you and it gets exposed by the same recent code?

currentoor04:05:43

i’m verifying wether we can get post-mutations (for existing stuff so no tempids) on 0.4.8 with advanced compilation

currentoor04:05:15

nevermind i get the same error on 0.4.8

currentoor04:05:31

must be something wrong on our side as well

currentoor04:05:44

well for us the fix was changing (om/ident table/Widget widget) to [:widget/by-id (:db/id widget)] inside a mutation

currentoor04:05:44

the fact that this only happens with advanced compilation suggests it might be a bug in om/ident

currentoor04:05:06

we probably shouldn’t use om/ident anyway...

currentoor04:05:37

anyway, the latest snapshot appears to handle post-mutations, with tempids, fine for us (though as @therabidbanana said we only have one simple usecase for them)

ethangracer15:05:18

@currentoor: you should use om/get-ident instead, makes it a tad easier to get the ident

ethangracer15:05:13

ok @currentoor @therabidbanana @tony.kay confirmed the bug — has to do with post-mutations when parallel loading is true. see lines 58 and 87 in impl.data-fetch. notice on line 87 that items-to-load, which is a vector of data states, is being put inside another vector. On line 58, item, which is a singular data-state, is put in one vector (missing that second level of nesting).

ethangracer15:05:01

working on a patch now

ethangracer15:05:36

@therabidbanana would it be ok to re-write impl.data-fetch:47 as:

(action resp args)
… in which case we might be able to get rid of the whole action-with-args function? It looks like args is always supposed to be a vector of data states that is passed directly to the loaded-callback. not understanding why an apply is needed

ethangracer15:05:05

if not, it’s simple enough to wrap item on line 58 in another vector — just not as clean of a solution as I’d like

currentoor16:05:12

@ethangracer: thanks for the tip! And good job on figuring it out.

therabidbanana16:05:35

My only reasoning for using action-with-args was to allow existing requests in the queue that don't define callback-args to be called with the correct argument-arity. I wasn't sure if there would be other places where requests get pushed on that need to-load to take a single argument.

therabidbanana16:05:39

It's javascript under the hood, so I think maybe arity doesn't matter?

therabidbanana16:05:15

I didn't want to assume that all to-load callbacks would be okay with receiving a new second args argument though

tony.kay16:05:58

so, have you pushed a fix @ethangracer ?

tony.kay16:05:50

OK, so I did a patch that simplified the code quite a bit. testing now, but it is up on clojars as well

ethangracer16:05:01

@tony.kay: confirmed working on insight — good to go, thanks

tony.kay16:05:15

thanks...still doing checks myself

ethangracer16:05:07

@therabidbanana and thank you for finding the tempid remap issue in the first place because that solved another bug for us 😄

tony.kay16:05:29

cookbook for parallel and sequential loading works with it too

tony.kay16:05:37

ok. SNAPSHOT looks to be fixed.

tony.kay16:05:44

pushed to clojars and github

therabidbanana16:05:34

Glad to hear it was helpful, even if I broke a bunch of stuff. 🙂

tony.kay16:05:59

ah, we just need to standardize a set of post integration tests for network layer stuff

therabidbanana16:05:32

We have CI builds that take screenshots of all of our devcards. I wonder if you could set up something similar to just load each cookbook and screenshot the initial page to get some idea that the various stuff still works

tony.kay16:05:37

You guys getting checkouts-like stuff going so you can verify against a real app would be good too

tony.kay16:05:24

we also need behavioral tests on more of this layer...it is one of the critical spots that has no tests...

therabidbanana16:05:13

I think we found we can just symlink the untangled client source directory into our source dir and get builds complete with hot code reloading, which should be helpful testing new commits in our app moving forward

tony.kay17:05:23

So, I've updated todomvc to latest versions on develop. I had to do some project file tweaks t oget advanced optimization working on production build...running it now and it looks perfect. Single 195k download.

tony.kay17:05:37

love the closure love

tony.kay18:05:03

Just released 0.4.8 of server. On Clojars

tony.kay18:05:27

minor changes (a little better logging, support for more http-kit options in config)

tony.kay18:05:53

Untangled client 0.4.10 released to clojars

tony.kay19:05:16

Untangled datomic 0.4.9 released to clojars