Fork me on GitHub
#om
<
2015-11-05
>
drcode00:11:14

@davewo @dnolen: Thanks for the info!

thosmos06:11:53

@drcode: port Om.Next to js? 😉

dnolen12:11:34

@thosmos: that wouldn’t actually be so hard, the most challenging bit would be what to do with the query part

bplatz12:11:08

I'm missing somewhere the idiomatic way to build up a query for only rendered components. I see two possibilities, both of which seem like I'm missing something easier: (a) extensive use of Query Params and manipulation of them or (b) Looking at current state and conditionally returning null for things that I know aren't being rendered.

dnolen12:11:39

a) is the one that naturally comes to my mind for me

dnolen12:11:59

if you think of queries like urls then it’s a natural place to put result size parameterization

bplatz12:11:54

Result size make sense, I'm thinking more binary. Like I'm looking at the dashboard, and the "chat" nav item has not been clicked. I don't want to load the chat data until it is.

dnolen12:11:37

right, Relay has a notion of delayed queries - but I wonder about that

dnolen12:11:50

seems like sugar for something easily expressed by just changing the query

dnolen12:11:27

there is an reading code advantage to the declarative aspect of having syntax for it ...

bplatz13:11:35

Yes, just changing the query seems perfect, but my inclination is to return just the parts I need from IQuery. However, I've observed IQuery is only called once, not at each render. At least that is what appears to happen. I'd want IQuery called each time, access om/props from within it and return a different query.

dnolen13:11:29

yeah we’re not going to do that

dnolen13:11:42

you can use life cycle methods to accomplish the same thing anyway

dnolen13:11:07

and you can write sugar over defui if you want this for all of your own components

bplatz13:11:23

OK, I'll explore that path for it.

bplatz13:11:29

This will be a common challenge for large apps that have deep functionality, much of which is infrequently accessed.

dnolen13:11:08

I don’t see any challenges here at all … only a challenge based on the way you to achieve this as far as I understand it

dnolen13:11:23

if somebody clicks a button, I would just change the query then and there

dnolen13:11:29

and avoid all this props talk

grounded_sage13:11:34

(dom/render-to-str (om/build some-widget data)) -- Would this would need to be done on the server?

dnolen13:11:58

dom/render-to-str is for server side rendering yes

grounded_sage13:11:34

Ok so that means even if I used Om with a different backend language I would still technically need JVM if I wanted to have a faster first page view and keep my SEO up

dnolen13:11:00

@grounded_sage: you would need a JavaScript engine, not a JVM

dnolen13:11:23

@bplatz: one way that’s more declarative is to put all the fragments you intend to use in IQueryParams

dnolen13:11:33

then you can see all the things that a component might add to the query

dnolen13:11:51

then when you change the query just add the var you need

bplatz13:11:58

So using om/set-query! within the context of a transaction sounds like your recommendation. That sounds good, only downside I see is that it separates where you can easily reason about what a given query might be.

grounded_sage13:11:59

Do I need a JavaScript engine to run it when I am using Clojure on the backend?

dnolen13:11:34

@bplatz that’s not even true simple_smile

bplatz13:11:39

@dnolen that sounds like another good option.

dnolen13:11:46

not only do we log all transactions we log all query changes

dnolen13:11:59

all state transitions are copy pasteable / explorable

bplatz13:11:57

I was referring to the logic that determines what a query might be. Part of it would end up being handled in a function that is detached from the component itself. Your recommendation of using Query Params is a way to avoid that.

dnolen13:11:45

@bplatz: ah, yes I see what you mean and it’s clearer what your original question was about.

dnolen13:11:04

yeah I would use IQueryParams if I wanted to make it clear what types of queries a component might make

dnolen13:11:08

so of course working on recursive queries reveals things I do not like with current parse behavior

dnolen13:11:51

once you’re doing something recursive having to read from the atom only to rewrap a part of the atom to make a recursive call seems gratuitous

dnolen13:11:08

thinking that we should also pass :data key in addition to :state?

dnolen13:11:17

maybe :pure to constrast with :state but also make a correspondence?

jannis13:11:29

I can't follow. Can you give an example?

bplatz13:11:48

So the :data key, if I understand it, would just be a map you pass in or similar?

bplatz14:11:02

One item that has bothered me is common query items that are repeated at many levels of the query tree, for example something like :user/language. Most components will want to know it, so it ends up sitting in, with my case, 2/3 of all components. Keeping with the purity of the parser, it is looked up with every recursion. I was thinking I'd optimize some of this with memoize as I figure out the implications.

bplatz14:11:03

(I'm using datascript, so in my case it is a datascript query that is executed)

dnolen14:11:23

right that’s a different problem

dnolen14:11:36

I’m not sure that real global information like that should be transported by queries

dnolen14:11:20

@bplatz the scaffolding for :shared is there, I would probably propagate this at the root

dnolen14:11:27

@jannis I need to think about this some more once I can actually express my dislike for how parse looks in this case I will

bplatz14:11:02

@dnolen :shared works great for these sort of environmental variables, thanks for pointing that out. If IQuery can be analogous to a REST request, :shared can be analogous to HTTP headers... a context.

dnolen14:11:26

@bplatz yeah I’m fixing up shared a bit now

dnolen14:11:46

will probably cut a release with this bit and the other recent tweaks

bplatz15:11:26

FWIW, at first blush :context seems a bit more descriptive of intention (or at least how my intention is of using it). Extends nicely to the idea of reusing code in different contexts, like React Native. Query defines data needed, context could modify behavior of the component.

dnolen15:11:23

@bplatz: it’s a little bit better but it’s pretty overloaded

dnolen15:11:36

in React it gets used for a lot of things

dnolen15:11:49

and in Om Next we already had this before it was ever public in React and we called it shared

dnolen15:11:16

there you go

dnolen15:11:33

the new thing needs some tweaks to account for remotes

dnolen15:11:47

so you can supply static :shared

dnolen15:11:12

as well as :shared-fn which will compute shared properties from the root props.

dnolen15:11:20

these get merged together.

dnolen15:11:26

and every Om Next component will see them.

bplatz15:11:58

Function is a nice addition. I'll have to look to see if the function is executed only on the first root render, or every props change.

bplatz15:11:35

But most things I think (should) be stored there are likely fairly static in nature.

anmonteiro15:11:13

@dnolen: reconciler docstring states all properties are required

anmonteiro15:11:27

probably a typo?

dnolen16:11:38

@bplatz: it’s run every root render but this shouldn’t really be problem since the function should be side effect free.

dnolen19:11:15

@leontalbot: that’s a lot code for what is as far as I can tell a very simple problem / misunderstanding

dnolen19:11:37

just make the tiniest possible thing demonstrating the conceptual issue you are having

dnolen19:11:01

if I can recall correctly I just don’t see how this isn’t solved by having

dnolen19:11:27

:questions [[:question/by-id 0] [:question/by-id 1]]

dnolen19:11:52

@leontalbot: can you explain why this doesn’t work for you ^ ?

leontalbot19:11:33

You are right. I'll narrow down the problem tonight

joshfrench21:11:18

i was surprised that om/computed clobbers any existing computed props, but i’m doing this within a union component so i may only be seeing the outlier case

dnolen21:11:47

@joshfrench: there’s no particular reason for the behavior we could merge instead

joshfrench21:11:10

ordinarily you wouldn’t pass down the complete props wholesale except in the case of a union or a pass-through, right? i’m trying to decide if it would be weirder to find inherited props in om.next/computed or to have to manually reconstruct them at every level

dnolen21:11:29

well “inherited"

dnolen21:11:03

I think it’s important to not abuse the feature

dnolen21:11:20

accumulating stuff at each level just isn’t going to work

dnolen21:11:35

and if you’re doing that be ready for very weird bugs

dnolen21:11:18

for this reason the current behavior of clobbering maybe even the right one

dnolen21:11:30

and we should just document it

dnolen21:11:55

and people that want to play the dangerous game can do so knowingly

joshfrench21:11:33

that’s my hunch, for all the cases where merging would be a reasonable convenience there are probably more where it’s a code smell

dnolen21:11:54

k I’ll document the current behavior and we’ll see how it plays out

jannis22:11:47

I could imagine merging computed to be quite confusing. Imagine A passing down a click-fn to a child B, which in turn has another child C that also allows a click-fn to be passed down. You could easily end up passing the root click-fn to both subcomponents by accident and weird things might happen.

jannis22:11:48

If we don't merge, the click-fn will only be set in child C if it's explicitly passed down from B. I think at least for callbacks that's desirable.

jannis22:11:56

Does this make sense?

jannis22:11:56

I think it helps a lot if you have to assemble computed at each level. It'll be very explicit in each component, making it easier to understand what's going on.

dnolen22:11:01

yes, I’ve updated the docstring to clarify the behavior

adamfrey22:11:06

Does anyone understand how creating new Todos work in David’s om-next-demo re: Getting the datomic generated Todo :db/id attribute back to the client? I can’t find anywhere where todos/create-temp is being transacted

dnolen22:11:37

@adamfrey: just never got around to do the temporary todo bits

dnolen22:11:02

exercise left for the reader so to speak simple_smile

adamfrey22:11:36

right, that’s where I am right now.

dnolen23:11:39

yeah the problem here is the temp id problem

adamfrey23:11:12

I have a thing that works for part of the problem. In my resource creation, I’m pushing into the list new item, but with an id of :temp, which renders it to the screen. Then I kick off the :remote true transaction

adamfrey23:11:50

then my client reads the list again and I modified my parser to have this:

adamfrey23:11:57

(defmethod read :places/list
  [{:keys [state selector]} _ _]
  (let [st @state]
    (if (contains? st :places/list)
      (let [places (get-places st selector)
            has-temp-place? (some #(= (:db/id %) :temp) places)]
        (if has-temp-place?
          {:remote true}
          {:value places}))
      ;; First pull
      {:remote true})))

dnolen23:11:31

@adamfrey: yeah I thought about this some more I think this needs a standard solution

dnolen23:11:55

temp-ident

dnolen23:11:30

yeah this is too common a problem with remotes

dnolen23:11:37

you need to be able to create temporary ident

dnolen23:11:59

and then bang it when the real id comes back so that your database is correct