Fork me on GitHub
#om
<
2016-06-13
>
jimmy03:06:06

@iwankaramazow: I have updated your example with my actual use case. You can try output the state in :project/details reader, you can see that when the data is merged, the id is nil. https://gist.github.com/rhacker/fd1d60aa93e69263f8944901f26f277b @anmonteiro: it looks great, looking forward to try it out 😄

petterik07:06:22

@anmonteiro: Thanks for the post and lib! You're doing incredible work. :sports_medal: I find it interesting that you've not included "path navigation"/"nexted routing" in your library. We've rolled our own nested - quite hacky - routing with multiple nested set-query! calls, which has been broken since the dynamic query update (we're on alpha-32). Have you thought about what a "path navigation" routing solution might look like? A solution where each part of the path is passed to nested components? I.e. given route /project/1/person/2 the project component is passed 1 and the person component is passed 2.

petterik07:06:54

After seeing your library, I'm reconsidering our choice to go with path navigation and may end up using your library and app/component-state instead. More hammock time is needed.

iwankaramazow07:06:57

@nxqd: I found the problem

iwankaramazow07:06:53

When the remote result get's merged in, the key :project/detail will be scheduled for a re-render. The problem however is that we don't have the :id-param from the original read available. It only sees :project/detail. Solution: we need to store the selected id in the app-state.

jimmy07:06:16

yeah, it seems to be the only solution here. nice !

jimmy07:06:11

@iwankaramazow: btw, in your first example yesterday the one without wrapper, why does it work ? we still have id when being re-render later.

iwankaramazow07:06:33

Good question 😄

iwankaramazow07:06:44

No idea 😄

jimmy07:06:57

haha 😄

jimmy07:06:03

my first thought was

jimmy07:06:26

the params should be in the the root component but it's not

jimmy07:06:39

you can try output the result of state

jimmy07:06:24

I try to change the params of the root component but it still doesn't work xD

iwankaramazow07:06:23

Hmm there has to be something

iwankaramazow07:06:22

The response that gets merged in:

without wrapper {:project/detail {:description Lisp rocks!, :id 2}}
with wrapper {:active-props {:project/detail {:description Lisp rocks!, :id 2}}}

iwankaramazow07:06:47

I'm guessing, but I think in the first case the ident of :project/detail gets picked up. In the second case it doesn't. If you look at https://github.com/omcljs/om/blob/master/src/main/om/next.cljs#L1624, that sounds logic

jimmy08:06:34

I'm not sure I understand that does default-merge do anything with the params?

iwankaramazow08:06:18

This line {:keys (into [] (remove symbol?) (keys res)) picks the keys from the response, that needs to be scheduled for a re-render. The actual scheduling for a re-render happens https://github.com/omcljs/om/blob/master/src/main/om/next.cljs#L1640 My theory is that in the first case we get :project/detail as a key that gets scheduled, in the second case we get :active-props.

iwankaramazow08:06:42

Om Next figures out :project/detail is an ident with that id

iwankaramazow08:06:54

and it re-renders

iwankaramazow08:06:02

That's my theory at least 😄

jimmy08:06:06

true, let me try this thing. I have a theory that because it's gonna rerender :active-props it would only reserve the params for direct keys to be render :active-props, the rest it only use the default params

jimmy08:06:26

in our case, the id is nil when it's first mounted, let me change to something else

jimmy08:06:11

:query [{:projects [:id :title]} ({:project/detail [:id :title :description]} {:id 2})]
:query [{:projects [:id :title]} ({:project/detail [:id :title :description]} {:id "something else"})] ;; after merged, it uses default component params.

jimmy08:06:42

it's correct.They only care about the params of first keys, the rest they use the params the component first mounted. The output above is from the env variable, in :active-props read fn

iwankaramazow08:06:56

Another Om Next mystery solved 😄

iwankaramazow08:06:34

the question is, should it link to the updated params instead of the default params from the mounted component?

jimmy08:06:34

true, it's the question. Last time I read the source code, I see there is another class-path->query and such in om next. They do cache these things in indexer, and they might take things from there to construct this query as well ( not related to the question btw, just the source of these )

iwankaramazow08:06:23

we should probably present this to anmonteiro

jimmy08:06:49

sure we should 😄 thanks for helping though 😄

jimmy08:06:08

I encounter this one several times, not sure it's my problem of not understanding om next correctly

jimmy08:06:42

working with you clarify things a lot. let's put this on hold and get help from @anmonteiro when he's available

jimmy08:06:57

btw, can we get the indexer in read fn ?

jimmy08:06:04

I want to see if our value in there

iwankaramazow08:06:55

probably not, those are the keys available in the env (:query-root :path :pathopt :ast :state :parser :logger :shared :target :query)

yusup08:06:54

Hi , I am running a TodoMVC app (Om Next + Datascript). Everything works but code reloading. defui is giving undeclared variable warning in figwheel during code reload.

yusup08:06:49

has anyone encountered similar warning ?

jimmy08:06:08

@iwankaramazow: ah, I just get the indexer in the reconciler, I will post one here. You can check see that the params for each individual component is correct, but the params for components path is the one when it's first mounted. https://gist.githubusercontent.com/rhacker/9d7b11eba1b49d6a05eb6f19fdd8cdad/raw/9302aa60ce4f32f8722e0659b107cef15cd31720/indexer_output , raw is easier to search.

iwankaramazow08:06:03

the question is, how do we fix this 😄

jimmy08:06:57

i think we should ask Anmonteiro or Dnolen about this, hopefully figure out a way

iwankaramazow08:06:15

Indeed, when Anmonteiro's online I'll make a minimal case

hlolli09:06:07

I'm studying few months old code I found on github, it's doing remote mutation. And in the mutation function there it this to be found: :value [:app/title] :action ..etc... I get this error, probably saying excacly what is wrong

parser.cljc:221 Uncaught Error: Assert failed: app/update-title mutation :value must be nil or a map with structure {:keys [...]}
(or (nil? value) (map? value))

cjmurphy10:06:21

:value {:keys [:app/title]} might work better

jimmy10:06:52

@iwankaramazow: does your fake backend working with :tempids merging ? I try this one but it doesn't seem to work https://gist.github.com/rhacker/fd1d60aa93e69263f8944901f26f277b . But it works fine if I use normal backend server

cjmurphy10:06:52

The old code seems to be supplying the keys to :value directly. Perhaps that is the way Om Next was originally.

cjmurphy10:06:25

Hmm - not sure about that - because the :action part is more important, given that :value is just for documentation.

iwankaramazow10:06:04

@nxqd: didn't test it with tempids, I'll check it out this evening

hlolli11:06:43

@cjmurphy: thanks, I remember now a discussion about this recently. This old code var also using om.next parser in the backend. I learned still alot from this code, tough callback from :send used on the responsetext is still a black magic to me, but that is a discussion for another time.

jimmy11:06:24

@hlolli: just in case you are looking for a modern example of current om next. You can check the code from talks of Anmonteiro : https://github.com/anmonteiro/talks . I find it's quite useful in many cases.

hlolli11:06:48

yes! I've been researching his code from Berlin (which I attended) to the atoms. Very very helpful, last days I've been just exploring how the json post and responses look like. Since I need to query a database, and want to do so querying only what I need and send that across the wire. Next step is understanding the tempids, a lot of good stuff there.

cjmurphy11:06:26

@yusup: I've seen things like that in the past. I don't currently seem to have those warnings thou. I'm quite sure they are harmless. Perhaps a more recent version of figwheel would help. I'm using [figwheel-sidecar "0.5.0-6"].

anmonteiro11:06:25

@petterik: thanks for the kind words. re: path navigation, what I feel like routing is supposed to do from a high level is showing the right component at the top level. That’s what people had most trouble with. I feel like everything else is a different concern. In your example, /project/1/person/2 could e.g. route to the Project component and every other parameter can be handled by the app-state

anmonteiro11:06:00

I found that’s the most flexible way to be dealing with stuff like that, when using the bare bones approach that I described in my routing catalogue post

anmonteiro11:06:09

integration with routing libs like secretary and bidi is a first class thing too, and they already extract the URL params for you

anmonteiro11:06:43

for me it’s just a matter of keeping them in the app-state (which is where every other thing will be — hence the single source of truth opinion of Om Next)

anmonteiro11:06:36

using links (`'[:my-app/route-params _]`) is also a good way of accessing that information in your nested components

anmonteiro11:06:27

that said, I’m happy to listen to whatever problems people encounter, and revisit some of the assumptions that I made along the way

yusup11:06:09

@cjmurphy: I am using the latest version of figwheel. This warning brokes the flow of reloading. I have to fix this. I have previous errors caused by merging data(retrieved via remote) to DataScript . I think this error maybe the root cause of it . This variable not found warning happens once in two or three compilation/reloading.

anmonteiro11:06:18

@yusup: what does the warning say, specifically?

yusup11:06:08

111 :id-key :db/id 112 :migrate migrate-tempids 113 })) 114 115 (defui Todos ^--- Use of undeclared Var lispy.core/x43489 116 static om/IQueryParams 117 (params [this] 118 {:todo-item (om/get-query item/TodoItem)}) 119 120 static om/IQuery

yusup11:06:21

^--- Use of undeclared Var lispy.core/x43489

anmonteiro11:06:37

AFAIK that should only happen if you’re compiling with :advanced

yusup11:06:42

I am not compiling with :advanced.

yusup11:06:51

it is :none.

anmonteiro11:06:20

that’s really weird

anmonteiro12:06:53

@yusup: does the issue only appear on reload?

anmonteiro12:06:59

or is it there on initial compilation?

anmonteiro12:06:28

also which version of Om/Figwheel are you using?

cjmurphy12:06:28

And perhaps using Incognito Chrome browser and lein clean, lein deps would make sure nothing left lying around, just in case you haven't done that.

yusup12:06:01

I tried lein clean many many times. The result is the same. This warning only showes up on reload and it shows up once in two reload.

cjmurphy12:06:22

And the browser cache has been cleared out? (using Incognito means no cache so no need to clear it).

yusup12:06:17

[org.clojure/clojurescript "1.9.36"],[org.omcljs/om "1.0.0-alpha32" :exclusions [cljsjs/react]]

yusup12:06:44

I tried chrome incognito, the same. result.

yusup12:06:46

I think I have to fix previous error related to merging data into Datascript.

anmonteiro12:06:34

@yusup: yeah I can’t seem to reproduce that

anmonteiro12:06:54

anyways, you can use this figwheel option to load code with warnings, shouldn’t disrupt your workflow:

:load-warninged-code true

yusup12:06:49

Thanks . Let me try that out.

yusup12:06:17

works like magic. @anmonteiro Thanks again.

peeja16:06:53

If I want to pass "children" into a component factory, but I want those children to be identified by names (I've got a component with a few named slots that can be filled by arbitrary components), is the best way to pass those in as om/computed params?

anmonteiro16:06:49

@peeja: if the component implements IQuery, pass it as computed props

anmonteiro16:06:01

if it’s a reusable component that doesn’t implement IQuery, pass it as normal props

peeja16:06:24

Oh, right, that only matters for IQuery components. Sweet.

iwankaramazow17:06:16

@anmonteiro: While helping out @nxqd, I encountered a problem with a component's params. Minimal case: https://gist.github.com/IwanKaramazow/a4fa111baf657b6e39ada44a643d17b3 The idea is to have a list of projects. If you click on a list-item you get the detail. onClick of a list-item the params of a query get updated & a remote read happens. When the result gets merged back in, a re-read happens of the detail key, but the params are the params from when the component mounted. Not the actual params with the selected id from the selected list-item. If you want to reproduce it, click on a list item in the minimal case. It prints out the id param, it'll be the param from when the component's query mounted. Might be worth adding, the detail key sits under a join.

Joe R. Smith20:06:43

Why does this fail om/props’ assertion using om/component?, but om/component? returns true otherwise? ' static om/IQueryParams (params [this] (let [c (om/app-root state/reconciler)] (println "is component? " (om/component? c)) (println (om/props c))) {:app/route-data []})'

Joe R. Smith20:06:00

the first println prints “true”, the second fails the om/props’ :pre assertion

dnolen20:06:51

@solussd: that can happen on the first render

dnolen20:06:57

before there any instances at all

dnolen21:06:14

i.e. a static method call on application start

dnolen21:06:23

then this will be the class not an instance