Fork me on GitHub
#om
<
2016-02-26
>
dnolen00:02:09

@anmonteiro: I think that’s it, later we can be careful to only dump tables we know we created

adamfrey01:02:08

@tmorten what does the “result” from the mutation look like that gets passed to default-merge?

adamfrey01:02:07

when I do a remote mutation and return a result from the server, the tempid migration is working fine but my server response is getting merged into my app-state if I use the default merge. Like my app state has a migration name as a key`place/create` and the response as a value:

{:keys [:places/list],
  :tempids
  {[:place/by-id <#C06DT2YSY>/id["b6d04510-384d-4912-a4eb-64f6791b0b9e"]]
   [:place/by-id 17592186045425]}}, 
 :om.next/tables #{}}

adamfrey01:02:43

is that expected? or avoidable in an obvious way?

hueyp02:02:49

you can filter out symbols before you give it the cb

hueyp02:02:57

top level keys that are symbols

hueyp02:02:21

withdrawn!

adamfrey02:02:25

why withdrawn? what didn’t work?

george.w.singer02:02:25

Is it true that a call to (om.next/reconciler ..) returns an object which -- when dereffed -- returns the current state of your application?

george.w.singer02:02:53

This seems true when messing around in a REPL

hueyp02:02:45

@adamfrey: because I don’t know how temp-ids work … I haven’t used them yet so I just remove symbols

tmorten03:02:45

@adamfrey I also am using tempids, I'll take a look at see what comes in via the response and let you know

tawus04:02:47

If I have a remote mutation for (om/transasct! [(some/mutation) :update/key]). Should I expect the :update/key be updated after the mutation call is complete?

iwankaramazow07:02:15

@jlongster: there was/is (???) something weird with context that makes me stay away from it. Something with shouldComponentUpdate, if a component higher in the tree returns false, context doesn't re-render deeper down. You're probably aware of this, but I find this introducing a lot of fragility...

iwankaramazow07:02:58

@george.w.singer: yes that's pretty much it, you get some extra stuff too like :om.next/queries and :om.next/tables. The tables is a set of the keys in your atom state that represent ident- 'tables'/parts of your global state atom.

iwankaramazow07:02:55

@tawus: in general if that's the desired result, you should implement your parser that way. Don't expect anything to happen out of the box.

george.w.singer07:02:15

@iwankaramazow: it also seems that the reconciler has normalized app state data, while the actual app-state doesn't retain the auto-normalized data

george.w.singer08:02:04

Question: when a component makes calls to (om.next/query ..) via IQuery, is the env that is passed to the parser's reader function set to (i) the root app state, or (ii) the component's local props?

iwankaramazow08:02:50

what do you mean by the actual app-state data?

iwankaramazow08:02:05

from what I understand there's a single source of truth: the reconciler

george.w.singer08:02:33

I mean the root app state fed to the reconciler (which might have been mutated through the reconciler)

bbss08:02:03

You mean what the components get fed right?

iwankaramazow08:02:17

or the initial app state?

bbss08:02:20

You decide with your reads how the components get their data from the normalized format to the tree form that components use.

bbss08:02:47

So unless you can use db->tree for your reads it's not quite auto-normalized.

bbss08:02:23

But I am no expert by any means so could be wrong.

bbss08:02:50

auto-denormalized in case of db->tree actually

george.w.singer08:02:55

I mean what the components get fed when instantiated via (om.next/factory..)

george.w.singer08:02:09

Is THAT what is passed to env as the :state key?

bbss08:02:25

They don't get fed anything, you feed it with props.

george.w.singer08:02:53

(om.next/factory ..) returns a factory function that is fed with props

george.w.singer08:02:04

And when you feed that factory function with props

george.w.singer08:02:24

Does the env draw from those props to source its :state key?

george.w.singer08:02:42

in the parser's reader function?

bbss08:02:21

No it draws state from what you supply the reconciler

iwankaramazow08:02:56

the initial-state gets merged in the beginning

iwankaramazow08:02:03

after that you can discard it

iwankaramazow08:02:08

everything comes from the reconciler

george.w.singer08:02:13

What's weird is that I see in lots of tutorials (query ...) declarations that draw directly from props.

george.w.singer08:02:55

Are the props just fed in to immediately render a value?

george.w.singer08:02:24

and then the (query ...) call is to signal "these props are stateful, and might change?

bbss08:02:28

The root component (the one the query composes to) gets fed props by the reconciler, from there it is up to you to split the right data into props for non root components.

iwankaramazow08:02:04

probably the [(points/increment ~props)]`

iwankaramazow08:02:32

props is an arbitrary name here

iwankaramazow08:02:01

it just means it gives the current value for name & points to the mutation

iwankaramazow08:02:26

in fact you could only pass the :name, look the :points for that name up in the state (from the env), and update it

bbss08:02:49

is there a convenience function to easily merge in an existing db state with a diff db state, or do I have to manually update all the links?

iwankaramazow08:02:10

what do you mean with diff db state?

iwankaramazow08:02:01

without overwriting existing stuff?

bbss08:02:31

say I have :task/list [:task/by-id 123] :task/by-id {123 {:id 123 :name "I am a burden"}} and I use tree->db and to create a new task and end up with :task/list [:task/by-id 456] :task/by-id {456 {:id 456 :name "I am a burden too"}}

bbss08:02:46

yes, without overwriting

bbss08:02:24

do I manually have to conj the id to task/list and update the :task/by-id map?

iwankaramazow08:02:55

if this is local only, yes. If you merge incoming remote data, then you can use (defn custom-merge-tree [a b] (if (map? a) (merge-with into a b) b))

iwankaramazow08:02:17

and then pass on your reconciler :merge-tree custom-merge-tree

bbss08:02:39

hmm. okay, interesting.

bbss08:02:47

hoping to get to the remotes stuff soon

iwankaramazow08:02:06

I started out by putting my navbar on the server 😂

iwankaramazow08:02:31

just fetching a list of nav-items that gets normalized on the client with idents

bbss08:02:30

I am doing something pretty simple first too. Making something to track my activities, only added difficulty is I am using a query with recursion.

bbss08:02:48

{:name :some-task :should-take (minutes 5) :sub-tasks [{:name "something that has to be done during the task" :should-take (minutes 2)}]}

bbss08:02:12

hoping to somehow use web audio to bug me to track the tasks so I can get better at estimating tasks.

iwankaramazow08:02:10

that mutation just swaps everything on the local state

george.w.singer09:02:51

In the "Components, Identity, and Normalization" tutorial (https://github.com/omcljs/om/wiki/Components%2C-Identity-%26-Normalization), there is a call to (query [this] '[:name :points :age]) within the Person component definition. Yet there is never a parser read function defined which takes any of those keys (`:name`, :points, :age). Isn't this a problem?

george.w.singer09:02:37

Why isn't* this a problem?

bbss09:02:34

Because only the root key read gets called, after that it is up to the read function to call the parser recursively if you want to split up your read.

bbss09:02:56

-is my impression, please let me know if I am wrong-

iwankaramazow09:02:32

(defn get-people [state key]
  (let [st @state]
    (into [] (map #(get-in st %)) (get st key))))

iwankaramazow09:02:22

this functions makes sure everything gets read

iwankaramazow09:02:25

As @bbss says, only the root keys get read, in this example :list/one :list/two

iwankaramazow09:02:04

The parser dispatches on :list/one :list/two

iwankaramazow09:02:28

As you can see in the tutorial, they both return {:value (get-people state key)}

iwankaramazow09:02:52

the function get-people makes sure every key gets read

iwankaramazow09:02:36

for :list/one you'll first look up (get st :list/one) with get-people

iwankaramazow09:02:51

that gives you

[[:person/by-name "John"]
  [:person/by-name "Mary"]
  [:person/by-name "Bob"]]

iwankaramazow09:02:25

then the map function applies #(get-in st %) to the vector above

iwankaramazow09:02:24

basically it'll do (get-in st [:person/by-name "John") (get-in st [:person/by-name "Mary") (get-in st [:person/by-name "Bob")

iwankaramazow09:02:04

and st contains

:person/by-name
 {"John" {:name "John", :points 0},
  "Mary" {:name "Mary", :points 0, :age 27},
  "Bob" {:name "Bob", :points 0},
  "Gwen" {:name "Gwen", :points 0}, 
  "Jeff" {:name "Jeff", :points 0}}

iwankaramazow09:02:46

the return value gives you {:name "John", :points 0} for [:person/by-name "John"]

iwankaramazow09:02:16

in the tutorial the query '[:name :points] is redundant

iwankaramazow09:02:21

it doesn't get used

george.w.singer09:02:38

So when (query ...) is called

george.w.singer09:02:47

does it automatically send a read request to the parser?

george.w.singer09:02:14

And then -- in this case the parser says "I already have the information about that read, and I got it another read, so here is the value you're looking for"

thiagofm09:02:36

@iwankaramazow: Just saw the https://github.com/awkay/om-tutorial link -- good stuff! I wish I knew about this before. Perhaps add to the wiki? (maybe it was there but I didn't see it...)

thiagofm09:02:47

Would've made my life way easier

iwankaramazow09:02:09

@thiagofm: yea that tutorial is a goldmine 😄 Everything you need is in there I'm not in charge of the wiki...

iwankaramazow09:02:44

@george.w.singer: the reconciler will send the query from your Root Component through the parser

iwankaramazow09:02:04

it's up to you to pick that query apart

iwankaramazow09:02:17

the top level keys get recursively parsed though

iwankaramazow09:02:34

there's nothing shady going on here, it's basic recursion

thiagofm09:02:59

It was probably a lot of work, great stuff

thiagofm10:02:37

@iwankaramazow: do you think it's wrong to query something inside a mutate method? I have a case where it my mutation query depends on other attributes, which are really about another component

thiagofm10:02:24

(I'm using datascript)

iwankaramazow10:02:52

Most mutations happen based on some attributes (that might be or not be available at a component)

iwankaramazow10:02:27

For intercomponent comunication I use graph-style queries

thiagofm10:02:32

I could still query this data in the component level, but this would be just passed down to the mutation

iwankaramazow10:02:34

a la 'thinking with links'

iwankaramazow10:02:09

that's the approach I'm doing now

iwankaramazow10:02:25

but the state is available in your env at the mutation in the parser

iwankaramazow10:02:30

so you can get it from there

iwankaramazow10:02:40

mutations have to be side-effect-free though

iwankaramazow10:02:52

not sure if deref'ing your state is pure

thiagofm10:02:38

I'm making a simple game, so let's say the player can buy a bunch of upgrades. To display his total money he made in the current tick/second, I can either query this data in the component(What he bought isn't getting displayed in that component or is anywhere related) and pass it to the mutation that does this calculation and updates the total money, or query inside the mutation. Querying inside the mutation kind of help, as it's pretty clear what's happening, but yeah, dunno if this is functional 😞

thiagofm10:02:37

@iwankaramazow: what do you mean by side-effect free? I'm only transacting inside it, so it's like a side effect, no?

iwankaramazow10:02:34

given the same arguments you should always have the same result

thiagofm10:02:12

That is true, it's not sideeffect free 😞

iwankaramazow10:02:17

a simplistic example, if you (println "test") that isn't pure

iwankaramazow10:02:24

inside a mutation

iwankaramazow10:02:43

for all we know your computer may crash and we don't see "test"

thiagofm10:02:00

So if I transact inside it, it isn't side effect free anymore right?

iwankaramazow10:02:29

transact inside a mutation?

iwankaramazow10:02:57

no, the thunk has to be side-effect free at :action in your parser

iwankaramazow10:02:36

you probably shouldn't transact inside a mutation, I'm not sure though

iwankaramazow10:02:12

that's datascript

iwankaramazow10:02:23

yea that's valid

iwankaramazow10:02:37

looks pretty ok to me

iwankaramazow10:02:40

but I'm no expert

thiagofm10:02:17

But yeah, I think I kind of get it now. It's a better idea to query stuff in the component and have in the mutation perhaps only the transaction(and some pure function call if that's the case). Otherwise I'm not passing arguments to that mutation and it wouldn't feel side-effect free or pure, as by passing different arguments, i get a different result transacted everytime.

thiagofm10:02:29

Does this make sense?

thiagofm10:02:19

{:action (fn [] (let [[id total] (first (d/q '[:find ?id ?e :where [?id :lambdas/total ?e]] (d/db state))) per-second (ffirst (d/q '[:find ?e :where [_ :lambdas/per-second ?e]] (d/db state))) next-second-total (lambda-coins/next-second per-second)] (d/transact! state [{:db/id id :lambdas/total (+ total next-second-total)}] :values)))}) For example here I query [id total] and per-second. This could've been done in the component side

iwankaramazow10:02:19

apart from the side-effect-free stuff (which I'm totally not sure about) , it's indeed a better idea to keep your mutations only about transactions

thiagofm10:02:14

Thanks for you help, that was very enlightening simple_smile

iwankaramazow10:02:57

pure or side-effect-free is only about 'given the same input of a function, you always get the same output'

iwankaramazow10:02:16

the input for that function may differ, but the result is always the same for a given input

iwankaramazow10:02:08

I'm totally not sure if a read of your datascript db may happen in a mutation

iwankaramazow10:02:37

someone should shed some more light on this, help? 😄

thiagofm10:02:51

I wonder what (d/transact!) would return, but I guess if I send all the needed parameters to update the value needed, it's would have less side-effects

thiagofm10:02:19

Makes sense to query this in the component. I'm still wrestling with the whole concept. Before I was using reagent and even though you can probably avoid all the side-effect you want if you write things well, my code was full of state

thiagofm10:02:05

Om next seems to take this idea of side effect-free to another level. The idea for example to my root component have all the necessary queries from the children components was neat, but took me some time to figure out why it was like this

iwankaramazow10:02:36

I've never used Reagent

iwankaramazow10:02:13

side effect-free is all about making state management more predictable and reliable

iwankaramazow10:02:32

this is a huge bonus when your state is complex

iwankaramazow10:02:08

David Nolen had some pretty awesome revelations, when designing Om 😄

danielstockton10:02:45

@thiagofm: I guess you don't use set-query anywhere? I'm interested how people make that work with datascript

jimmy11:02:13

hi guys what is the proper way to use om computed. What I'm trying now is

(om/computed props {:text "something"})
(prn (om/get-computed this))
I got nil in return

iwankaramazow12:02:35

(defui Component
  Object
  (render [this]
          (let [computed-props (om/get-computed this)]
            (dom/div nil "stuff"))))

(def component (om/factory Component))

iwankaramazow12:02:41

(component (om/computed props {:text something}))

iwankaramazow12:02:45

@nxqd: that should do it

thiagofm12:02:38

And then I pass om/props to children components and have the query on each separate component

thiagofm12:02:03

I don't know if this is right, but after talking with a couple of people from here this is how I was able to make it work

thiagofm12:02:08

This is also my first take on om next, I've decided to use datascript from the beginning so I can learn how to query datomic while I write this project. After I knew about that get-query usage I feel like I'm able to do whatever I want regarding to that, now I'm more like reflecting on how to use it right.

thiagofm12:02:33

So I also don't know what is the way of doing it without using datascript simple_smile

danielstockton12:02:14

This is slightly different, that's get-query not set-query

danielstockton12:02:57

If you're updating your queries in runtime with set-query!, you'll eventually discover that om doesn't work with datascript unless you make some modifications

danielstockton12:02:21

om sets a ::queries key in the app-state and it expects the state to be an atom

danielstockton12:02:59

I'm in the same position as you, I'd rather learn datomic/datascript than worry about normalization

iwankaramazow12:02:05

@danielstockton: there isn't much about normalization ;) om/db->tree takes care of the heavy lifting 😄

thiagofm12:02:27

@danielstockton: oh, I get it. Yeah, with datascript I don't think I need to use it

iwankaramazow12:02:00

@thiagofm: how do you implement routing?

danielstockton12:02:06

you might want to, for example it seems like the preferred method of routing is using set-query!

danielstockton12:02:09

although there are alternatives

danielstockton12:02:35

im using union queries for now

danielstockton12:02:41

@iwankaramazow: true, but i like that om promises to separate state from everything else, which isn't quite true at the moment

danielstockton12:02:08

and i like datascript, you can do some powerful stuff that would be harder to implement with atoms

iwankaramazow12:02:23

@danielstockton: never thought of it that way, but I see what you mean

danielstockton12:02:53

i think it will change in future, it's not hard to make set-query state agnostic

iwankaramazow13:02:02

I plan to experiment with datascript once this side project is pretty much finished

danielstockton13:02:04

and provide a default implementation for atoms

iwankaramazow13:02:57

are there any other problems with datascript atm?

iwankaramazow13:02:04

in relation to Om.Next*

danielstockton13:02:20

i think that's the main one, there are other pros/cons

danielstockton13:02:36

tempids is another con, you have to either have a separate attribute which you can update or delete temporaryclient-side datums and re-transact datums from a remote

danielstockton13:02:08

and i think a pro is that deletes are easier, i don't have to worry about deleting from multiple places

danielstockton13:02:31

dunno if tempids is a real con, just requires a different approach

iwankaramazow13:02:10

the way I'm handling deletion now is in one place

iwankaramazow13:02:32

I just delete the reference in a list for example

iwankaramazow13:02:06

that is, the entity keeps existing in the :item/by-id portion of my state

danielstockton13:02:24

right, in case its referenced elsewhere

danielstockton13:02:29

and it exists indefinitely?

danielstockton13:02:34

or you have some cleanup methods

iwankaramazow13:02:21

for the moment most code is 'cave-men-style', no cleanup methods

iwankaramazow13:02:05

ps: do you have a Datomic backend along the datascrip?

danielstockton13:02:20

yeah, im just toying also, things will become clearer as more people create serious projects with it

danielstockton13:02:56

yep, datomic backend

danielstockton13:02:22

so i send the datums from the server and just transact them, it makes some things easier

iwankaramazow13:02:26

so the integration datascript/datomic is pretty smooth?

danielstockton13:02:00

so far but as i said, just toying at the moment

danielstockton13:02:08

might be issues i haven't come across yet

danielstockton13:02:04

having datomic also simplifies the parser, you can pretty much take the om query and use it as pull syntax

iwankaramazow13:02:09

Definitely, I switched my sql database to datomic

iwankaramazow13:02:26

and I have to say I'm pretty amazed about how much less work I have to do...

danielstockton13:02:24

datomic seems great for 95% of typical use-cases and i imagine it's easy to write parsers for the other 5% that you might want to store elsewhere

danielstockton13:02:01

reports or whatever that is

iwankaramazow13:02:38

The only problem is I can't even bring this stuff to my team at work

iwankaramazow13:02:39

Clojure(script) & datomic is too much of a leap for most people

thiagofm13:02:55

@iwankaramazow: I'm not doing routing and I just have intentions of doing just a couple of requests to the backend, most of the game is client side

thiagofm13:02:39

Only after I'm done with this project I'll try to explore more how to do a SPA

thiagofm13:02:24

Yeah, I also don't work with clojure, mostly ruby, mutable stuff... There aren't many jobs, it's hard to find one

danielstockton13:02:11

datomic is actually a perfect fit for my work i think, i just dont have the confidence to fully push it yet which is why i want to experiment in toy projects

thiagofm13:02:13

I find datomic very awesome simple_smile It's great to be able to use datascript and learn a bit more about querying on it

jlongster14:02:33

@iwankaramazow: yes, context makes shouldComponentUpdate more ambiguous, but only if the context ever changes. Context is great for exposing singletons throughout the whole app, and redux does that to connect everything to the store. We should be fine using it to connect the reconciler and indexer because you can't "switch them out" in the middle of your app

jlongster14:02:11

or you could always return true in sCU if the context has changed at all, thus everything would rerender, but we should be able to just ignore context in there

jimmy14:02:05

@iwankaramazow: hmm, I think I can use (om/computed) to store computed values ( cache ) while working within the component not passing from outside.

iwankaramazow16:02:41

@jlongster: thanks for the clarification, I had a lot of problems with Redux trying to implement int18n trough context.

tony.kay18:02:00

@dnolen: Working on a support VCR viewer for app state history. I'd like to include client timestamps in the history. Wondering if you would accept a PR for tacking a timestamp onto the app-state entries in history as metadata?

jlongster19:02:16

I have tempid migration working but I need to generate idents that map to a local table, like [:transaction/by-id 1]. How is the server supposed to know what the local id key should be for that id?

george.w.singer19:02:10

Is my bit understanding of om.next queries correct? Here it is: 1. Only the root query actually gets read by the parser; the results of this root query read are then fed in through the props of its child components. 2. The non-root component queries -- known as "fragment queries" -- aren't actually read by the parser; however, they are used behind the scenes for properly re-rendering your UI when state changes. Is this correct?

jlongster19:02:53

@george.w.singer: the non-root queries are still read by the parser, but it's up to you if you recursively call the parser, use db->tree to execute the query, or do something else

george.w.singer19:02:41

You mean place db-tree within your read function?

george.w.singer19:02:48

"Om only looks for the query on the root component of your UI! Make sure your queries compose all the way to the root! Basically the Root component ends up with one big fat query for the whole UI, but you get to reason about it through composition (recursive use of get-query). Also note that all of the data gets passed into the Root component, and every level of the UI that asked for (or composed in) data must pick that apart and pass it down." -- https://awkay.github.io/om-tutorial/#!/om_tutorial.E_UI_Queries_and_State

george.w.singer19:02:58

That's where I got (1) and (2) above.

jlongster19:02:57

yes just the root query is the only thing passed to the parser, and it executes the whole entire query

george.w.singer19:02:53

So the child queries don't actually have any tangible effect when your app starts?

george.w.singer19:02:11

They are just using the props fed into them by the fat query read from the parent?

george.w.singer19:02:28

Ok, last question on this 😀 : Why call them "props" if they are stateful?

george.w.singer19:02:00

The child components get fed their queries from the root component, but that data is subject to change later on

george.w.singer19:02:40

Is it just a carry-over from react to refer to them as props?

jlongster19:02:36

they are still just props fed in from something, whether it's the parent component or the indexer doesn't matter

dnolen19:02:17

@george.w.singer: the point is to maintain the semantics that the root component is a function from props -> render tree

dnolen19:02:39

that the root of the application is stateful doesn’t have any bearing on the root component acting more or less as pure function

george.w.singer19:02:28

👍 Thanks 🙂

iwankaramazow19:02:17

@jlongster: just curious, but why no uuid?

jlongster19:02:53

@iwankaramazow: not sure what you mean

iwankaramazow19:02:09

'How is the server supposed to know what the local id key should be for that id?' Is this a normal use case?

iwankaramazow19:02:42

it seems logical to generate unique random id's, merge those back in

iwankaramazow19:02:54

om takes care of the rest ?

iwankaramazow19:02:06

(I might have misunderstood your question..)

jlongster19:02:46

no, I mean the local table key

jlongster19:02:42

if an ident is [:foo/by-id 1] and I add a new item, I add [:foo/by-id (om/tempid)] and then later replace it, but for it to be replaced the server has to return an ident with the same key like [:foo/by-id 2]

jlongster19:02:23

I mean, the client can send the name of the key to be used as well, but all of the examples I've seen hardcode something like :db/id

iwankaramazow19:02:52

Ah ok, good point

iwankaramazow19:02:22

I'm just starting with tempids, I'll report back if it works

iwankaramazow19:02:26

from Tony's tutorial: On the server, convert tempids to real IDs. Return a :tempids map from the server. Keys are tempid ident, the values are the new real ID idents.

jlongster19:02:59

right, I have that all working

jlongster19:02:19

but right now I have to hardcode :transactions/by-id in the ident, but there are other idents like :accounts/by-id that need to work too

iwankaramazow19:02:36

Just looked at the copaste example

iwankaramazow19:02:05

seems sending them as params looks like the only solution

jlongster19:02:34

yeah, that works

dnolen20:02:35

@anmonteiro: reviewing your PR, I’d like to do some renaming here to make the code clearer - I can’t say I like how I named things before and now that the logic is going to get a little more sophisticated my bad naming choices are becoming apparent.

dnolen20:02:44

I’ll leave comments in the current PR.

anmonteiro20:02:08

@dnolen: perfect. I am also not happy with the naming

anmonteiro20:02:22

but I couldn't really find better alternatives

dnolen20:02:35

I think local is getting overloaded here

anmonteiro20:02:44

you know, computer science & naming for one, not being a native speaker for seconds simple_smile

dnolen20:02:46

in most cases I think we should just say get-instance-query

dnolen20:02:06

local just doesn't mean anything

anmonteiro20:02:31

so get-local-query-data should also be get-instance-query-data?

anmonteiro20:02:46

just saw your comment

dnolen20:02:07

or actually maybe since we’re already using component to always means instance we can just be more consistent about it

iwankaramazow20:02:31

I noticed there's a tempid?available in cljs om.next but not in clj om.next.server...

iwankaramazow20:02:48

Totally not a priority, but might be nice in the future

iwankaramazow20:02:46

:face_with_rolling_eyes:

dnolen20:02:57

@anmonteiro: also get-local-query is a bit strange since it also checks component?, I’m not sure I understand the docstring.

anmonteiro20:02:14

@dnolen: the docstring definitely needs changing too

anmonteiro20:02:27

basically this is the old get-query

anmonteiro20:02:34

which falls back on the class

anmonteiro20:02:20

I meant local in the sense that it doesn't leverage the indexes

dnolen20:02:22

@anmonteiro: ah, and the new get-query uses the indexer

anmonteiro20:02:49

but then again, local doesn't really mean anything 😛

dnolen20:02:33

@anmonteiro: in the dev cards bit you invoke get-local-query is this intentional? Just testing something?

anmonteiro20:02:44

@dnolen: unintentional. remains from a previous version where I thought get-local-query had to become part of the public API

anmonteiro20:02:05

I've also noticed that I'm calling it unnecessarily in the build-index* function.

anmonteiro20:02:09

needs changing

dnolen20:02:25

@anmonteiro: k I’m leaving comments on all these cosmetic things first, then let’s do another PR and I’ll review a second time (good to read over this at least twice on my end)

anmonteiro20:02:03

@dnolen: I didn't understand if you had finished commenting or still going through it

dnolen20:02:38

still going through it

dnolen21:02:44

@anmonteiro: so it seems cascade-query does the missing work of updating all the query templates?

anmonteiro21:02:20

@dnolen: except the case where the query is changing in a union which we treat differently in build-index*

anmonteiro21:02:34

because there might or might not be a class that holds the union query

dnolen21:02:45

@anmonteiro: hrm, in general you need a class to hold the union query though

anmonteiro21:02:55

@dnolen: I've tinkered with cases where I don't, and it doesn't seem like a problem

anmonteiro21:02:10

e.g.:

(defui Root
  static om/IQuery
  (query [this]
    [:app/route {:route/data {:tab1 [:a :b] :tab2 [:c :D}}]))

anmonteiro21:02:42

it's used a lot for e.g. routing

anmonteiro21:02:09

where we don't want a specific Router class or something

anmonteiro21:02:20

I've toyed with both cases

dnolen21:02:21

and I guess in these cases you don’t need the wrapper class to compute idents?

anmonteiro21:02:18

there'll be no idents

anmonteiro21:02:35

because each "page" is only shown once at a time

dnolen21:02:42

@anmonteiro: so union-keys param in cascade-query is to ignore these cases?

anmonteiro21:02:05

@dnolen: all union cases

dnolen21:02:09

or rather, treat them differently

anmonteiro21:02:30

because e.g. in the Unions tutorial, DashboardItem and Post have the same data-path

anmonteiro21:02:04

so when changing a union query we want to eliminate the union key that leads to the path because it's not there

anmonteiro21:02:54

I don't think I'm explaining myself well enough

anmonteiro21:02:40

build-index* will keep the extra union keys in the path it computes, and the rendered components skip the union key in their data paths

anmonteiro21:02:45

that's more like it

dnolen21:02:17

yeah I’m reading over build-index* now

dnolen22:02:21

@anmonteiro: k comments done - mostly boring stuff - send me a new PR I’ll give a second read and we can merge it

dnolen22:02:28

looks pretty good after the first read through

anmonteiro22:02:15

@dnolen: cool, thanks, but I don't think I'll get around to it today

dnolen22:02:35

@anmonteiro: that’s fine, no rush

anmonteiro23:02:37

@dnolen: you mentioned changing dp to key-path

anmonteiro23:02:52

should the indexer key also be changed? data-path->components to something else?

dnolen23:02:47

@anmonteiro: hrm, we should keep the language consistent whatever the case

dnolen23:02:00

if we use data-path everywhere then should probably stick with that

anmonteiro23:02:16

OK so changing to data-path instead of key-path

anmonteiro23:02:15

@dnolen: actually got around to it today, you want a fresh PR, right?

dnolen23:02:29

@anmonteiro: cool! yes please