Fork me on GitHub
#om
<
2016-10-19
>
wilkerlucio01:10:02

please consider this minimal case:

wilkerlucio01:10:35

the resulting idents are [:a 1] and [:b 2], I expected it to generate 2 tables (`:a` and :b) but only table :a is generated, and the item [:b 2] ends up there

wilkerlucio02:10:35

and this results in the further queries to miss for the :b

wilkerlucio10:10:53

@anmonteiro thanks, I was trying last, and I was unable to make a test case failing (so seems to be fixed on master :)), but at same time I'm having issues to use the master version on my project, I tried using lein install on Om and using the snapshot, but it seems to keep holding on the old version here

anmonteiro10:10:29

@wilkerlucio: that just seems like a caching issue unrelated to Om

anmonteiro10:10:06

If you're a Cursive user, I've heard a lot about it caching too much

anmonteiro10:10:20

Can't confirm though, I use emacs

wilkerlucio10:10:42

yeah, can be it, I'm cursive user

wilkerlucio10:10:48

ha, just tried again and now it's working 🙂

anmonteiro10:10:36

@wilkerlucio great! so you confirm you issue is fixed in master then?

wilkerlucio10:10:47

yes, the issue is fixed in master

akiel12:10:07

Is :om.next/abort public API?

ag18:10:41

hey guys… can someone help me to refresh my mind. why would in read method if I set :value locally it works, but fetching from remote would fetch the data but won’t put that data into the state atom?

dnolen19:10:49

@anmonteiro hrm, strangely when I try to run the Om tests with 1.9.293 I get a bunch of failures, have you seen this?

dnolen19:10:14

yeah same for bin/test

anmonteiro19:10:00

same for boot test

anmonteiro19:10:20

@dnolen does rolling back ClojureScript fix it?

anmonteiro19:10:41

right, stupid question, master is passing

dnolen19:10:23

@anmonteiro seems related to protocol change ...

anmonteiro19:10:25

@dnolen could it be the implements? patch

anmonteiro19:10:38

yeah, exactly

dnolen19:10:58

because we rely on implementation details here, doh!

dnolen19:10:00

simple fix

anmonteiro19:10:21

@dnolen should we not use implements??

anmonteiro19:10:24

or what’s the problem?

dnolen19:10:50

it’s just that we make the type constructor (class) implement the protocol

dnolen19:10:23

I’m just going to hack this in for now - but this probably means we need a standard way to do this

dnolen19:10:47

(set! ~(#'cljs.core/extend-prefix type-sym pprefix) true)

dnolen19:10:31

so actually for Om to work with <= 293 we will need a version check

dnolen19:10:33

in that macro

anmonteiro19:10:02

I still don’t get what the problem is

anmonteiro19:10:09

we do specify!, shouldn’t it take care of that?

dnolen19:10:35

in om/next.cljc

anmonteiro19:10:44

the advanced stuff

anmonteiro19:10:42

@dnolen by standard way, you mean compiler support for static methods in JS classes?

dnolen19:10:14

nah just a standard way to set a protocol property to future proof

dnolen19:10:23

we can’t really fix this backwards

dnolen19:10:32

(set-protocol-property! …)

dnolen19:10:49

and change ClojureScript to use that, and then anybody can use it that’s doing low-level stuff like this

dnolen19:10:58

and protect themselves from any other lower changes

anmonteiro19:10:16

so Om is not compatible with CLJS 1.9.293

dnolen19:10:32

but \clojurescript-version\ is a thing in the compiler

anmonteiro19:10:48

yeah, it’s in cljs.util or something right?

dnolen19:10:55

so making this work is pretty easy

anmonteiro19:10:31

at least you found it early enough

dnolen19:10:25

boom fixed

dnolen19:10:04

@anmonteiro going to cut, anything else need going in here?

anmonteiro19:10:56

@dnolen minor one, but this one could go in if @ethangracer has submitted a CA: https://github.com/omcljs/om/pull/707

anmonteiro19:10:26

other than that good to go

dnolen19:10:24

@anmonteiro don’t have a CA for them

anmonteiro19:10:39

not important anyway

anmonteiro19:10:47

can be fixed in a further release

dnolen19:10:53

1.0.0-alpha47 released

anmonteiro19:10:05

@dnolen typo, it’s 47 🙂

dnolen19:10:25

heh thanks

ag21:10:26

hey guys… so I am modifying ast in read method (on the fly) then doing {:remote modified-ast}, but it still sending unmodified one… why?

peeja21:10:37

@ag Can you verify that your modified-ast is actually being computed and returned there?

peeja21:10:04

(rather than something else being the code that's actually called for some reason)

ag21:10:04

yeah… it’is…

peeja21:10:39

If you remove the :remote key entirely, does it stop sending the query?

peeja21:10:07

Dang. Then I'm all out of ideas. 😕

dnolen21:10:44

@ag because the AST stuff is a bit half baked

peeja21:10:59

Oh, I know why!

dnolen21:10:00

we store the old form and use it if it’s there

peeja21:10:33

@dnolen When I did that, it complained that it was nil (or the query became nil? I don't remember now)

peeja21:10:40

Does that ring a bell? Was that fixed?

dnolen21:10:44

if somebody’s wants to audit and clean this up that would be great

dnolen21:10:54

@peeja doesn’t ring a bell because I haven’t heard that much feedback in this area

peeja21:10:58

Might be an issue of composing with Compassus

ag21:10:59

@peeja might be… interesting.. if I modify ast’s children - that doesn’t work… if I assoc params to entire ast - works

ag21:10:03

¯\(ツ)

anmonteiro21:10:45

@ag because the ast->query function actually uses the :query in the AST to convert back and not the children

peeja21:10:14

@ag Right, so after you make your changes, try dissocing the :query from the AST and see if it works

peeja21:10:37

It's essentially a stale cache

ag21:10:32

@anmonteiro oh.. ok… then you’re the person who can advise me. If query of the ast is something like this: [({:color/header [:color-id :name]} {:color-id nil})] what would be the best way to set the params? Compassus merges whatever route-params you give it into the state. So my plan was to traverse ast’s :children and replace params with what in route-params

anmonteiro21:10:22

sounds like a plan

ag21:10:32

but that’s not working…oh… what if after modifying ast I do ast->query and then query->ast

anmonteiro21:10:52

that would work

peeja21:10:53

That's what I've been doing, and it works for me

anmonteiro21:10:03

but Om should be enhanced in that respect

anmonteiro21:10:21

it’s kinda heavy for your app to be doing ast->query and query->ast all the time

anmonteiro21:10:42

IMO if Om Next sees a :query in the AST it can just use it because it’s faster

anmonteiro21:10:55

otherwise (you dissoc) it should just re-compute the whole query

anmonteiro21:10:07

as David said, the AST stuff is kinda half-baked

ag21:10:10

meh… you’re guys are smart… you’d come up with something.. me? I just want to build "Hello world" apps

anmonteiro21:10:13

esp. ast->query

anmonteiro21:10:38

@ag btw still haven’t gotten around to try your example

anmonteiro21:10:54

was it caused by the issue you opened in Compassus?

anmonteiro21:10:04

or do I still need to look at it? 🙂

ag21:10:05

@anmonteiro oh no… I have found a way.. the issue is still relevant in terms that it should still enforce default value whether third param or not.

anmonteiro21:10:46

yeah, it’s definitely a bug

anmonteiro21:10:54

I can actually fix it right away

peeja21:10:10

Does Om currently provide any tools that might help with merging two queries? I'm trying to do something like:

(query-merge
 [{:person/parent [:person/favorite-color]}
  :person/favorite-color]
 [{:person/parent [:person/date-of-birth]}
  :person/date-of-birth])
and get
[{:person/parent [:person/date-of-birth
                  :person/favorite-color]}
 :person/date-of-birth
 :person/favorite-color]

peeja21:10:12

Dang. Got some code to write, then. 🙂

anmonteiro21:10:12

@ag btw the merge issue sounds sketchy

anmonteiro21:10:30

I’d try to understand what the actual underlying problem is

anmonteiro21:10:41

could be an issue in Compassus, but at first it doesn’t sound like it

ag21:10:43

@anmonteiro I’ll try to work on cleaner repro sample

ag21:10:13

btw… I also stumbled a couple of times on error at (.forceUpdate c) in init! which doesn’t happen all the time and it’s easy to fix by just wrapping it in (when c ,,,

ag21:10:16

also I found it’s a bad idea to name main entry point to cljs app with a bang. e.g. init! JS doesn’t understand bangs.

ag21:10:52

right… that’s the correct line

anmonteiro21:10:50

that’s just the template, not a compassus bug

ag21:10:45

@anmonteiro yeah, I know. not saying it’s a bug

anmonteiro21:10:58

whenever you get a chance to read

ag21:10:18

eh… guys… running om/ast->query om/query->ast now modified my ast. turning it from type :join into type :root. Can I tell it not to do that?

anmonteiro21:10:33

@ag heh, that’s right

anmonteiro21:10:42

use om.next.impl.parser/expr->ast

anmonteiro21:10:51

btw fixed your issue, want me to cut a snapshot release?

ag22:10:18

@anmonteiro it’s alright, I don’t think it’s necessary… I’ll wait for more stuff/fixes… up to you though

anmonteiro22:10:15

@ag just #12 (and #16 if we confirm it’s a bug) holding up next release

ag22:10:08

eh, guys… now. when I try to modify ast using om/ast->query om.next.impl.parser/expr->ast it still manges the ast… so if it’s a simple query it works. but if the ast contains query like this:

[({:color/header [:color-id :name]
          :color/details [:id :description :title]} {:color-id nil})]
it seems to be cutting off the second part completely. leaving it with ({:color/header [:color-id :name]} {:color-id “3”})

ag22:10:21

what am I doing wrong this time?

ag22:10:26

oh… I think I just found om.next.impl.parser/ast->expr

ag22:10:22

but om.next.impl.parser/ast->expr doesn’t care about parameters in ast 😞

ag22:10:19

so yeah… 1) om.next.impl.parser/ast->expr retrieves query from an ast, but it doesn not set query params based on the key :params in the ast.

ag22:10:57

2) ast->query does retrieve query from ast and sets the params based of :params key of the ast, BUT. it modifies the query, if you have a join query it takes only the first item in it. e.g. :

`[({:foo/foo [:id] :bar/bar [:id]} {:id ~’?id})]
will lose its :bar part

ag23:10:25

when you run (parser env query) is there a way to pass parameters to all involved dispatch methods?

petterik23:10:23

@ag I don't think you can do it without some hacking. Either modify the query or put the parameter in env and wrap a function around your read/`mutate` which extracts the parameter and puts it in the params

ag23:10:02

@petterik alright, thanks!