Fork me on GitHub
#fulcro
<
2017-11-30
>
tony.kay03:11:52

OK everyone. Last change to comment on the 2.0 name updates…I’m doing them for the next few hours. So far the list is: - Move defsc, InitialAppState, and related functions to primitives - Rename fulcro.client.core to fulcro.client I’ll ping out more if I do any others.

tony.kay03:11:39

I think I might move the other helpers related to merging and generating state to primitives as well

tony.kay03:11:53

so, all state management stuff would end up there

tony.kay04:11:39

Man I’m glad I have tests

mitchelkuijpers08:11:42

Oh I love that you move InitialAppState also to primitives ^^

mitchelkuijpers09:11:04

Ahh fulcro-inspect doesn't work with fulcro 2.0 😢

tony.kay09:11:57

So, while doing the renaming and heavy testing, I found a critical bug in merge…working on it, but it may take me a bit

mitchelkuijpers09:11:09

We upgraded our next project btw, we still need om for devcards?

tony.kay09:11:13

Wilker is working on inspect for 2.0 @mitchelkuijpers

mitchelkuijpers09:11:23

Yeah, no worries

tony.kay09:11:33

no you don’t need om for devcards

mitchelkuijpers09:11:47

We got a sablono error otherwise

tony.kay09:11:47

use defcard-fulcro out of the fulcro.client.cards ns

tony.kay09:11:50

it’s been there for a while

tony.kay09:11:15

can’t help you with sablano…if it has a hard dep then that is not so good

tony.kay09:11:26

I would try dependency exclusions

mitchelkuijpers09:11:39

Yeah it does, we excluded it everywhre but it nees om.next.server

mitchelkuijpers09:11:10

And we use defcard-fulcro ^^

tony.kay09:11:50

why would it need that?

tony.kay09:11:20

strange. the author of the lib needs to fix it…they’re pulling in Om into people’s non-om projects.

mitchelkuijpers09:11:53

Oh they piggieback on om to provide Server side rendering with sablone

tony.kay10:11:25

So, once you’ve ported cleanly, it’ll be ok for Om to be around…just bloat

tony.kay10:11:56

assuming they are not assuming you’re using Om yourself.

mitchelkuijpers10:11:25

I think it's not a big problem, we just added it as a dev dependency so we will get compile errors when we compile with the production profile ^^

mitchelkuijpers10:11:41

But it is kinda lame that sablono has a hard dep on om

tony.kay10:11:55

so, the bug I discovered causes a render with new state, but with a query based on the old props. This might actually be the path-opt bug in Om Next rediscovered…never could explain what was going on. I’ve made path-opt always the default and thought I was getting around the bug by a rework of the internals…but the exact line of code from Om Next that was path-opt is causing the problem. I’ve identified exactly what it is, but I’m still tracing why it happens. Basically, it seems that on return from remotes (on merge), the props used to calculate the ident of the component to refresh are stale. If tempid remapping happened, this leads it to use a query against the database that has an ident join with a tempid that finds no data. This leads to a refresh of your component with no props, which causes some components to throw an exception, which gets eaten by the internals (I’ve fixed this last bit which dramatically improves the debuggability). It’s almost 3am…I’m done for now 😕

tony.kay10:11:45

The workaround for now is to make sure your components can tolerate empty props.

tony.kay10:11:04

(this is just on 2.0, and 1.x with :path-opt on)

roklenarcic15:11:23

Hm, I still struggle with how to implement when I have multiple views of the same entity and views of the entity paired with some extra data. I have a massive proliferation of components that have same ident, very similar queries and renders

cjmurphy15:11:17

@roklenarcic If as you say the renders are similar then perhaps they could be made less 'static' - by making the same render output differently depending on what comes in via props and/or computed props. For instance in the websocket-demo some users are highlighted and others are not.

wilkerlucio17:11:17

@tony.kay about the data-load story, currently it seems we have a gap regarding data loading + init state, when we start combining elements that need :ui/... data + data load, we need to write some manual post-mutations to get the data from the server and add the initial state

wilkerlucio17:11:39

had you though about this case, you have ideas to handle this situation?

tony.kay17:11:59

@wilkerlucio Open an issue and we can have a discussion there. Too many things on my plate today 🙂

tony.kay17:11:21

I figured out the path-opt bug! It’s an easy fix, and would fix it for Om Next, too.

wilkerlucio17:11:29

sure, no problem, I'm just curious if you ever got to think about this

wilkerlucio17:11:42

I did a somewhat solution here yesterday

wilkerlucio17:11:55

so if that aligns we can discuss if it's worth pulling in

wilkerlucio17:11:20

what I have done is an algorithm that you give a ref and a class

wilkerlucio17:11:32

and it goes traversing the data initializing everything (that is related)

tony.kay17:11:11

but there should be only one, right? It’s a ref

wilkerlucio17:11:38

but it looks at the query, find the related refs that are present, and recurivelly goes initializing the dependencies

wilkerlucio17:11:51

(defn init-state
  "Starting from an ident and query, scan the DB initializing the components. This should be used to initialize data
  loaded from the network with fulcro fetch, this will recursively traverse using query information and merge the
  initial state with the current data (data load from the server takes priority)."
  ([state x ident]
   (let [initial  (fulcro/get-initial-state x nil)
         children (-> x om/get-query om/query->ast :children)
         data     (om/db->tree (om/get-query x) (get-in state ident) state)]
     (reduce
       (fn [s {:keys [type component key]}]
         (if (and (= :join type) component)
           (let [value (get-in state (conj ident key))]
             (cond
               (om-ident? value)
               (init-state s component value)

               (vector? value)
               (reduce
                 (fn [s ident]
                   (if (om-ident? ident)
                     (init-state s component ident)
                     s))
                 s
                 value)

               :else
               s))
           s))
       (merge-entity state x (merge initial data))
       children))))

wilkerlucio17:11:50

it does a merge, but without overriding any attribute taht's already there (just fills whatever is missing compared to the result of initial-state)

tony.kay17:11:34

yeah…issue…no brain power to process all that and the implicaations right now

tony.kay17:11:25

I added the forensics to the path-opt bug here, if anyone is interested: https://github.com/fulcrologic/fulcro/issues/81

tony.kay18:11:58

I’m going to have to loosen up the error checking on defsc due to dynamic query syntax

tony.kay18:11:48

and I’m dropping the fancy initial-state stuff…it’ll have to be a lambda

tony.kay19:11:00

that was confusing anyway 😜

tony.kay19:11:07

If anyone wants to play with the latest changes. I’ve pushed fulcro 2.0.0-SNAPSHOT6 with the renames to clojars. If you use spec, you’ll need alpha4-SNAPSHOT of that… I’m working on the refinements of defsc tonight, so that API will change slightly before alpha6 of fulcro is sent. This cleanup work is taking longer than expected…not doing a release today 😕

currentoor19:11:46

Any reason why the reconciler for a fulcro app would be nil?

currentoor19:11:59

a mounted app that is

grzm20:11:45

Looking through the "getting started" docs under figwheel (https://github.com/fulcrologic/fulcro/blob/develop/GettingStarted.adoc#running-figwheel), the Cider instructions don't mention calling (user/start-figwheel). Is this an oversight or should figwheel start automatically upon successful completion of the cider-jack-in-clojurescript command? It doesn't start automatically in my experience.

currentoor16:12:42

@U0EHU1800 I use cider with fulcro, i usually do cider-jack-in-clojure, then call (start-figwheel), since i’m already in the user namespace

currentoor16:12:06

start-figwheel will give you a browser connected cljs repl

grzm20:12:32

@U09FEH8GN thanks for confirming. I suppose I should supply a PR to update the docs