Fork me on GitHub
#om
<
2016-09-08
>
selfsame03:09:46

on om (1.0.0-alpha44) and getting Uncaught Error: Can't pop empty vector when transacting the reconciler w/ re-read idents used by components with recursive queries : https://github.com/omcljs/om/blob/master/src/main/om/next.cljs#L280

selfsame03:09:57

per earlier discussion, is this from not finding an IQuery path to root? (possible I have messed up my data-paths in props, defui tree has IQuery from root)

selfsame05:09:02

root query is [{:views {:outliner {[:roots _] [:uid {:nodes ...}]}}}] , error happens before recur at {:om-path [:views 3 :roots 0 :nodes 1]}

anmonteiro07:09:49

@selfsame if you could provide a minimal repro and possibly open an issue so that it doesn’t get forgotten

anmonteiro07:09:01

happy to look at it when I have some time

peeja15:09:44

I'm a little confused about union queries. Is it up to the implementer to decide what they mean? For instance, in the Queries With Unions example, the union query switches on the :type of each item in :dashboard/items. Is :type entirely made up for the purposes of the tutorial? https://github.com/omcljs/om/wiki/Queries-With-Unions

peeja15:09:25

And is it therefore meant to be well-known as part of this particular app's data model?

peeja15:09:30

(It's often unclear to me when Om expects me to give something meaning, and when in giving something meaning I'm abusing a feature of Om.)

peeja15:09:50

For instance, in Compassus, the root component's query includes ::route and a union query on ::route-data which contains queries of every component in the routing map. That works because the Compassus parser knows what that union query means: it only uses the entry from the union query that matches the current value of ::route.

peeja15:09:45

That is, which branch of the union is used depends on the value of a different key. That feels like a completely different use of union queries to me. Is it just as valid?

selfsame15:09:18

it dispatches on the first key of the idents, expecting a vec like [[:foo 1][:bar 2]] , those don't have to have to point at anything technically, but usually there is data backing up the semantic

selfsame15:09:28

The wiki example uses unions for a mixed collection, but they let you route at the top level via a simple transact

peeja16:09:33

@selfsame Ah, interesting. Are you familiar with Compassus? Does this break that semantic and abuse unions, or does this still fit? https://github.com/compassus/compassus/blob/c30481e7f1061182429f0b68c2bb46148ba745d2/src/main/compassus/core.cljc#L32 route->query here is a map of each keyword that identifies a route to the query of the component which render that route. The value of ::route is whichever of those keywords is the current route. The parser's read only returns a value for ::route-data that matches the query in route->query that matches the current route keyword. https://github.com/compassus/compassus/blob/c30481e7f1061182429f0b68c2bb46148ba745d2/src/main/compassus/core.cljc#L143-L147

selfsame16:09:55

not familiar but looks like it's just a wrapper to get user queries into the root

peeja16:09:06

I'm not sure I follow. It's exactly that, but it should still follow the intended semantics for queries, shouldn't it?

selfsame16:09:14

yes just a normal union, same as a hard coded [{:view {:main [:a] :options [:b]}}]

peeja16:09:58

So, what idents do :main and :options match in that example?

selfsame16:09:00

something like {:view [[:main 0]]} as an app state

peeja16:09:31

Ah, yes, so this is getting at something I keep failing to understand about Compassus and other routing examples. There's usually a key in the app state for each page, and that key contains that page's data.

anmonteiro16:09:48

@peeja right but dispatching on a specific key doesn’t mean you need to have it in the app state

anmonteiro16:09:14

(at least in compassus)

anmonteiro16:09:38

nothing is stopping you from (get state :foo) in the parser read for :index

anmonteiro16:09:01

as long as the semantics are consistent

peeja16:09:07

Well, that's what I'm pushing on, then. Is it an abuse of the union construct in queries to do that? Or are unions supposed to be able to do things like that?

anmonteiro16:09:52

@peeja I don’t think there are very well-defined “Dos & Don’ts” or best practices at this point

peeja16:09:15

I think that's what I'm having trouble with, then

peeja16:09:34

I'm trying to figure out the "Om Next way" of doing things, and there often isn't one 🙂

selfsame16:09:29

unions open up a lot of design possibilities that don't depend on readers set-query, or params

peeja16:09:55

@anmonteiro Incidentally, what made you choose the union query strategy over the subquery strategy for Compassus?

anmonteiro16:09:27

@peeja well mostly driven by the fact that subquery is normally OK for the bounded case but I wanted to support an indefinite number of routes not known before the fact

anmonteiro16:09:53

(also don’t really want to be dealing with React refs)

peeja16:09:04

Why is subquery more bounded than unions?

peeja16:09:27

Couldn't you build a subquery version out of the same route map that Compassus currently uses?

anmonteiro16:09:55

probably, but could potentially mean additional complexity

peeja16:09:09

Also, as I understand it, subquery will compose the page component's query to the root even if it's updated with set-query!, while the union query version is always based on the page's initial query. Is that not a problem?

selfsame16:09:08

i'll admit I have had on and off problems with union query paths

mjhamrick18:09:48

I tried searching through the channel and didn't see anything. Is a child allowed to delete itself through an om/transact, or am I thinking about this the wrong way. I'm trying to write a todo app with datascript and can't figure out what query I should give to the mutate.

peeja18:09:59

@mjhamrick That should be fine, something like (item/delete {:id 6})

peeja18:09:21

Oh, I see, that may not re-read correctly, if it's transacting on the item component

mjhamrick18:09:46

Is there a better way to change the responsibility? Right now I have at item component that has a delete button on it. When it's pressed, it sends an om transaction to delete the item from the database. Is there a way to force a re render?

peeja18:09:19

The old-Om way to do that would be to have the parent transact on its cursor, and I think shifting the responsibility to the parent makes sense here too. You can do that the same way: by passing the child a function to call to delete the item.

peeja18:09:29

You'll want to pass in that function as a computed prop: https://github.com/omcljs/om/wiki/Documentation-(om.next)#computed

peeja18:09:47

And, look, the example there even includes a :delete handler. So that must be right. 🙂

mjhamrick18:09:11

Oh great. Thanks very much.

peeja18:09:02

Waaaaaaaait a minute. If I get-query a component, do I see the latest subqueries from subcomponents automatically, merged in by their paths?

peeja18:09:49

That is, if I set-query! a subcomponent, does that change the result of its parent's get-query?

peeja18:09:23

I thought it would only do that if I defined the parent's query in terms of subquery, but it appears to be doing it even without that.

anmonteiro19:09:11

@peeja: it does that if you query the instance, not the class

peeja19:09:27

That's awesome

peeja19:09:47

That makes what Compassus does make a lot more sense to me

anmonteiro19:09:17

That was the point of my dynamic queries patch a while back :-)

peeja19:09:59

I've been trying to understand how you're supposed to make everything compose to the root dynamically without subquery everywhere. But this makes sense now: it only needs to compose to the root statically, and then the dynamic query will be updated automatically.

mjhamrick19:09:02

I switched over to competed props being passed down and it's working as intended now. Thanks again, @peeja

peeja19:09:14

Sweet! :thumbsup:

anmonteiro19:09:20

@peeja yes that’s right

anmonteiro19:09:43

I also wonder what makes you want to set-query! so bad

anmonteiro19:09:51

or if you’re just trying to grasp the concepts for the sake of it

peeja19:09:21

@anmonteiro I'm grappling with deeper routing params (or what I would normally thing of as routing params). So, /blog-posts/2/comment/5 needs to query for the right blog post and the right comment, and when I click "Next Comment" I want to query for comment #6.

peeja19:09:54

set-query! isn't necessarily the right way to do that, I'm also trying to grasp the concept. 🙂

anmonteiro19:09:12

I normally prefer to handle top level routing via union queries and params via app-state

peeja19:09:53

So, how do you change the query based on the app state?

peeja19:09:13

Use om/params in calculating the query?

anmonteiro19:09:18

well maybe I’m not getting your needs exactly but I haven’t needed to change the query, that’s what I’m saying

peeja19:09:08

I'm saying that I need to query for comment #5, and then after the user takes an action I need to query for comment #6 instead

peeja19:09:39

I can store "currently selected comment" as a key in the state, but I'm not sure how to affect the query

anmonteiro19:09:06

right but why would you need to change the query if the comment is the same component, thus needs the same keys?

peeja19:09:20

…because I need a different comment?

peeja19:09:37

I want different data. Doesn't that mean I need a different query?

anmonteiro19:09:52

you want the same data

peeja19:09:54

I want [:comment/by-id 6]

anmonteiro19:09:00

I mean, you want the same attributes

anmonteiro19:09:15

but for a different ID

anmonteiro19:09:28

your query doesn’t need to have the ident

peeja19:09:35

Someone's does, doesn't it?

anmonteiro19:09:39

that’s what I’m saying should go in the app state

peeja19:09:51

:lightbulb:

anmonteiro19:09:07

you want [:db/id :comment/author :comment/content]

peeja19:09:10

Make the value an ident, then query through it

anmonteiro19:09:13

for every comment

anmonteiro19:09:39

you want :curent-route {:comment-id 6 } in your app-state

anmonteiro19:09:58

not necessarily in that structure, choose whatever fits your app’s needs

peeja19:09:15

Where :current-route is a key named for the route in question, or is a single key that holds the route data regardless of the page you're on? Or do both work?

anmonteiro19:09:36

@peeja I was thinking the latter

peeja19:09:16

Does that work if it's {:comment-id 6}? Wouldn't I need to have a full ident there?

peeja19:09:27

I guess it depends on what the parser does, yeah?

jasonjckn19:09:44

what does this error mean No queries exist for component path

jasonjckn19:09:05

Uncaught #error {:message "No queries exist for component path (admin.ui.root/Root admin.ui.tabunion/TabUnion admin.ui.search.core/SearchResults) or data path [[:search :tab]]",

anmonteiro19:09:04

@peeja normally it would be something more general

anmonteiro19:09:17

like {:route-params {:id 6}} or whatever

anmonteiro19:09:38

and if your :current-route would be :comments, or comment/by-id you can generate an ident from that

iwankaramazow19:09:36

@jasonjckn : an invalid query, there's probably a problem with the way you compose the query fragments to the root

jasonjckn19:09:08

@iwankaramazow if I called get-query on root, that would reveal the problem?

jasonjckn19:09:27

the error occurs when I transact! on SearchResults

jasonjckn19:09:29

[:ui/react-key
 {:loading-bar [[:ui/loading-data _]]}
 :env-vars
 :login-profile
 [:nav-bar :global]
 {:current-tab
  {:view-listing [:which-tab [:env-vars _] :listing-id],
   :guest-pending-reviews [:which-tab [:env-vars _]],
   :invite-user [:which-tab [:env-vars _]],
   :entitlements
   [:which-tab
    :sso-id
    :modal
    :ent-details
    :ent-profile
    :show-add-ent-section
    :all-active],
   :host-pending-reviews [:which-tab [:env-vars _]],
   :search
   [:which-tab
    :page-size
    :from
    :query
    :metro
    :es-sort-fields
    :sort-ascending?
    :hits
    :es-type
    :time-range
    :time-field
    :listing-status
    :bkg-status],
   :view-booking [:which-tab [:env-vars _] :booking-id],
   :TXJ [:which-tab [:env-vars _]],
   :new-collections [:which-tab [:env-vars _]],
   :reports [:which-tab [:env-vars _]],
   :edit-listing [:which-tab [:env-vars _] :listing-id],
   :edit-profile [:which-tab [:env-vars _] :sso-id],
   :create-booking [:which-tab [:env-vars _]],
   :space-transfer [:which-tab [:env-vars _]]}}]

iwankaramazow19:09:44

@jasonjckn not sure if I'm right, but I think the path Om maintains will be broken if you're query doesn't compose correctly. This only shows if you transact.

jasonjckn19:09:14

my root query looks okay to me

iwankaramazow19:09:42

It's difficult to say were you query is wrong without debugging your app

jasonjckn20:09:06

oh I see the issue

iwankaramazow20:09:07

you should probably remove everything from your app to produce the minimal root query which gives that error

jasonjckn20:09:17

I think it's because I call transact! SearchResults

jasonjckn20:09:22

but it's part of a union

jasonjckn20:09:29

so the om/Ident is implemented on TabUnion

jasonjckn20:09:32

but not on SearchResults

jasonjckn20:09:39

so the ref is on TabUnion , not SearchResults

jasonjckn20:09:47

and I expect a ref in my mutation for SearchResults

jasonjckn20:09:16

that's kind of awkward that there's no 'ref' argument on any of the Tab's in the tab union

iwankaramazow20:09:48

with 'ref' you mean ident?

jasonjckn20:09:15

when I transact! on SearchResults, the mutation handler expects 'ref' found in 'env' to have a value non nil

iwankaramazow20:09:00

If you add the necessary refs, does that solve the problem?

jasonjckn20:09:32

i don't think you're suppose to implement om/Ident for the subcomponents of a union

jasonjckn20:09:48

(defui ^:once TabUnion
  ;; InitialAppState can only initialize one of these (it is a to-one relation). See app.core for the trick to initialize other tabs
  ; IMPORTANT NOTE: We're using the state of **A** specific child. This is because a union controlling component has no state
  ; of it's own.
  static InitialAppState
  (initial-state [clz params] (initial-state MainTab nil))
  static om/IQuery
  (query [this] {:main (om/get-query MainTab) :settings (om/get-query SettingsTab)})

jasonjckn20:09:03

MainTab and SettingsTab don't have om/Ident implemented

iwankaramazow20:09:38

And what if you implement an ident on a subcomponent of a union?

selfsame20:09:34

jasonjckn : ah yes having this exact issue today 🙂

iwankaramazow20:09:14

@selfsame did you find a solution?

jasonjckn20:09:38

@iwankaramazow so I put an ident on the subcomponent of the union, and so far so good

selfsame20:09:40

still trying to understand a minimal repro

jasonjckn20:09:44

I thought I tried this a while ago, and ran into some issue

jasonjckn20:09:58

it was on an older version of om.next so maybe it's been fixed

jasonjckn20:09:36

any idea if this is proper usage of om.next? because technically TabUnion 's ident evaluates to [:search :tab] and so does SearchResultsTab

jasonjckn20:09:46

so there's a 'duplicate ident' but it's only because i'm using a union

selfsame20:09:55

I use idents on subcomp unions too, works when they get focused updates with :pathopt, but the om.next/path gets lost on the unions on the initial render, and now is causing errors in om .44

jasonjckn20:09:54

my initial render seems to work i'm on [org.omcljs/om "1.0.0-alpha41"]

selfsame20:09:21

"Uncaught Error: Can't pop empty vector" happening for recursive sub union queries in om.next/get-indexed-query

peeja20:09:44

@anmonteiro How would you suggest transacting the route data into the app state? compassus/set-route! doesn't take additional data. Should I be calling that and an additional om/transact! from my browser history implementation? Feels weird to do two transactions at once when they could be two mutations in one transaction.

anmonteiro20:09:30

@peeja are you using Compassus?

peeja20:09:58

Sort of. I'm seeing if it fits, and also playing with a toy app.

peeja20:09:32

Every time I think it's not what what we need, it turns out I'm wrong about what we need. 😜

anmonteiro20:09:36

this was here a few days ago

anmonteiro20:09:45

my answer to that was

anmonteiro20:09:12

I also don’t agree that we should be doing 2 transactions, so I suggested that we open an issue in Compassus

peeja20:09:29

Awesome, then I'm on the right track :thumbsup:

anmonteiro20:09:32

set-route! could eventually take an additional argument that would be e.g a mutation to run

anmonteiro20:09:58

and instead of performing 2 transactions we’d run 1 transaction with the 2 mutations

peeja20:09:47

Yeah, that sounds great

peeja20:09:43

The other way to go would be to support "route data" explicitly (but not have opinions about how it's structured or used)

peeja20:09:23

So, have Compassus accept the data, use its mutation to store the data, and then have it hand the data back to your component

peeja20:09:04

Oh, I guess you don't want that, because you (the app developer) want to be able to query for the data, so you want to own the key it lives under

anmonteiro20:09:12

@peeja right, unless you come up with very convincing arguments, I would not go that way

peeja20:09:28

No, I've talked myself out of it. 🙂

selfsame21:09:00

re: union query data paths think I'm just reading them wrong

selfsame21:09:39

(om/db->tree query @state @state) does :om-path [[:union-join-key idx] :ect ] continued down the recursive subcomps, guess that's fine? (edit edit: ooh think this is where you (get-in @state (get-in @state [:union idx])) )