Fork me on GitHub
#fulcro
<
2017-08-28
>
wilkerlucio14:08:56

@tony thanks for opening the issue, I was trying to figure here first if it wasn't some mistake of mine, I'll try to pinpoint the problem exactly, I'm using custom networks so this might be affecting the result too, I'll let you know as soon as I have some time to debug it properly

tony.kay16:08:23

Fulcro-sql update: 0.1.0-SNAPSHOT on clojars. Now supports recursive queries (`...` and n); however, n does not actually limit, and there is no loop detection. So it will not work for you yet if your data goes in circles.

tony.kay17:08:12

OK, that wasn’t so hard. Update pushed again. Now has recursion limit and loop detection support on 0.1.0-SNAPSHOT.

tony.kay17:08:49

I also added support for bi-directional transform multimethods for converting Om props to/from SQL props. That was an oversight on my part. I’m not enamored with it at the moment, I think @wilkerlucio suggestion that the schema should just support a function instead of a map is probably the way to go. I like the schema being pure data. Still thinking it over. The property transform functionality could therefore change, but that would have a minor impact.

gardnervickers18:08:30

Hey! We’re seeing an issue with df/load. On a slow connection (limited to “slow 3g” in chrome), we have df/load‘s being fired from componentDidMount. With a slow connection the app state load markers get correctly placed and an entry is made in :ui/loading but the network queue never tries to send the request. Setting :parallel false resolves the issue. I’m tracking it down now but if anyone has any ideas it would be appreciated.

gardnervickers18:08:44

This only happens in advanced compilation

claudiu19:08:27

Noob question. Just can't seem to figure it out, missing something basic. 😞 I have a listing that I load from the db elements ident [article/by-id 1], and when I click on a link I want to open a modal that has ident: "modal/by-type article" and have the article details there (since the article is partially loaded) I am trying to have in the overlay query something like {:article (om/get-query ArticleDisplay)}

tony.kay19:08:04

@claudiu Did you look at the modal example in the bootstrap section of devguide?

tony.kay19:08:37

@gardnervickers Hm. Optimizations break it?

gardnervickers19:08:23

Yea, :advanced. It’s super strange that it happens only when there’s a network delay.

tony.kay19:08:38

that is bizzarre

gardnervickers19:08:55

Maybe a race condition getting the network send loop up I’m thinking

tony.kay19:08:56

So, parallel loads bypass the queue

tony.kay19:08:22

and you implied you’re doing parallel true, right?

gardnervickers19:08:31

That fixes it for us

tony.kay19:08:47

oh…you mean skipping the queue fixes it.

tony.kay19:08:06

your description above was backwards, I think

gardnervickers19:08:14

Ahh sorry about that.

tony.kay19:08:48

and none of the network interactions fail, right? Just slow.

gardnervickers19:08:01

They never happen

gardnervickers19:08:11

The request is never made

gardnervickers19:08:21

The load markers just stay in app state perpetually

gardnervickers19:08:36

The Network tab in chrome doesn’t show anything.

tony.kay19:08:40

hm. that’s no good.

tony.kay19:08:15

AH, I think your suggestion might, in fact, be right

gardnervickers19:08:27

But weirder, we have a df/load in our :started-callback that hits the queue and that works correctly.

tony.kay19:08:30

if componentDidMount fires before the networking has started, it would hang

tony.kay19:08:42

add-root happens before initialize is complete

tony.kay19:08:06

but networking has started by then

tony.kay19:08:22

you’re not using websockets of custom networking, right?

tony.kay19:08:13

does anything in your onComponentMount rely on stuff in started-callback?

tony.kay19:08:01

or alternate unions?

gardnervickers19:08:22

Yes it’s a child of a union

gardnervickers19:08:37

If that’s what you mean? Sorry I might not be following

tony.kay19:08:06

that is the current init order. I’m thinking add-root might be too early

gardnervickers19:08:23

Ahhhh would my union initial state not show up then?

tony.kay19:08:26

cause that’s where your onComponentMoust is going to fire

tony.kay19:08:42

your union main branch would be there, but not the non-main ones

tony.kay19:08:52

I think this is a bug, no matter what

tony.kay19:08:12

just trying to think if there was a reason for the order, or just a screw-up

gardnervickers19:08:44

Yea hmm I don’t think this is what we’re seeing but it does sound like a bug. This happens if we’re just calling df/refresh! without any conditionals in componentDidMount

gardnervickers19:08:19

And the load markers are correctly placed in app state.

tony.kay19:08:57

So, I’m going to push a change on that anyway…1.0.0-beta9-SNAPSHOT is on clojars with that reordered

tony.kay19:08:17

I wouldn’t think that would fix anything for you, but I’d appreciate you trying it

gardnervickers19:08:27

Sure thing, thanks

tony.kay19:08:11

Actually, that might break things

tony.kay19:08:20

I’ll test it here as well

tony.kay19:08:11

so far so good. It doesn’t break things. There is no concurrency in the browser, so I don’t expect a race condition so much as a hard-to-reason-about code path. The atom has watches on it that trigger re-render, so some of the swapping that happens during startup could trigger renders.

tony.kay19:08:35

but in all cases I can think of, the networking should be ready already

gardnervickers19:08:57

That SNAPSHOT breaks my routing lib, there’s no app-root for the reconciler passed to :app-started callback.

tony.kay19:08:10

ah, ok, that helps

gardnervickers19:08:12

And it does not solve our issue, thanks though I’ll keep looking.

tony.kay19:08:18

I knew there was a reason for the order

tony.kay19:08:07

pushed a new snapshot. The order is now after the merge of alt unions, but before callback

gardnervickers19:08:56

I’ll give that a shot

tony.kay19:08:13

@gardnervickers That won’t fix it…let me try one more thing.

claudiu19:08:40

@tony.kay Looked a bit at the bootstrap example. Was doing it right actually, just was not careful and did assoc into [:modal/by-type :article] instead of [:modal/by-type :article :article]. 🙂

tony.kay19:08:18

@gardnervickers I found something that looks fishy in networking. Try the latest beta9-SNAPSHOT with your adv opt

gardnervickers19:08:37

Thanks, will do

gardnervickers19:08:13

Hrm nothing changed

gardnervickers19:08:25

I can confirm that we’re not hitting real-send at all

tony.kay19:08:55

so, what I changed was the callback logic in networking. If the callback doesn’t trigger right, then it won’t unblock the send queue

tony.kay19:08:07

which would keep the next request from getting to the real-send

gardnervickers19:08:42

Hrm maybe I got the wrong snapshot version, that sounds like my problem

tony.kay19:08:43

for each networking, there is a payload queue, and a response queue. The response-channel is what blocks from getting the next request. The callbacks in networking trigger an item to be placed on response-channel, which frees the processing to get the next item in queue

tony.kay20:08:04

if you’re hung, then you’re either waiting on response-channel and nothing it posting to it, or the main queue is empty. The latter could be cause by 1. The code that places stuff on the queue (load) ran before the queue existed. 2. Something cleared the queue.

gardnervickers20:08:45

I’m pretty sure this is a core.async problem where we’re relying on something to finish before it actually does. When I step through with the debugger everything works correctly.

tony.kay20:08:55

There’s only one thread…so I’m not sure how that’s possible. I have to take care of some stuff this afternoon. If you have any further insights I’ll be glad to try patches.

gardnervickers20:08:34

Anywhere (go ...) is called without a corresponding take schedules the body for some time in the future.

(go (print 1))
(go (print 2))
(go (print 3))
can happen in any order.

gardnervickers20:08:41

Thanks for you help, I’ll keep digging.

claudiu20:08:54

does the problem still persist if you put :parallel true ?

gardnervickers20:08:42

No the problem goes away with :parallel true

tony.kay20:08:32

backwards again 🙂

gardnervickers21:08:11

I see my query being placed on the relevant :send-queue but the sequential processing loop never gets it.

roklenarcic21:08:07

I cannot seem to get ident loads to work

roklenarcic21:08:11

(load comp-or-app [:person/by-id 3] Person)

roklenarcic21:08:05

been wrangling this for 3 hours now and I always get Uncaught Error: No protocol method IMapEntry.-key defined for type cljs.core/PersistentArrayMap: {[:medlist-by-level "amp"] [:level :ui/selected {:items [:id :name :level]}]}

roklenarcic21:08:40

When loading by ID, what should the map I call the callback look like?

roklenarcic21:08:02

{[:person/by-id 3] {:name x :id y}} ?

roklenarcic21:08:38

or should it be {:person/by-id {3 {:name x :id y}}}

tony.kay21:08:13

@roklenarcic The error you’re getting is unrelated to that load.

roklenarcic21:08:55

hm ok, but non-ident loads work fine

tony.kay21:08:17

as do ident loads…they are all over the place in existing code…and your call looks fine

tony.kay21:08:28

but your error is for something else

roklenarcic21:08:55

the stacktrace suggests it occurs when om.next tries to merge idents

tony.kay21:08:33

ah, that’s better. OK, the shape of your server return value should be: { [:person/by-id 3] {:id 3 :name "boo"}}

tony.kay21:08:45

if you’re using defmethod, then wrap that in a map with a :value key

tony.kay21:08:52

if you’re using the macros, they do that for you

roklenarcic21:08:11

ok I'll look into that

tony.kay21:08:23

but what is medlist-by-level?

tony.kay21:08:33

and why is it even in this convo?

roklenarcic21:08:49

i'm not using the Person example verbatim

tony.kay21:08:27

oh wait @roklenarcic I may have mis-spoken. The macros will wrap it for you all the way to the value

tony.kay21:08:03

as will the defmethod

tony.kay21:08:16

You’re in a parser, it will put the key back on the response…and the key was the ident

tony.kay21:08:22

you return just person

tony.kay21:08:52

{:value person} if using your own read emitter, or just person if using the defquery-entity macro.

roklenarcic21:08:37

the actual call is (df/load app [:medlist-by-level "amp"] MedList {:remote :rest :params {:filter "a"}})

roklenarcic21:08:54

an it's in :started-callback

tony.kay21:08:16

and you know you return a tree if data from the server, right?

roklenarcic21:08:18

MedList has ident [:medlist-by-level level]

tony.kay21:08:43

and the networking is custom REST stuff?

roklenarcic21:08:48

and it returns data from a REST call, currently mocked to (ok {[:medlist-by-level "amp"] [:items [] :level "amp"]})

roklenarcic21:08:10

in my Network impl

tony.kay21:08:17

I see…reading…

tony.kay21:08:31

what is the query on MedList?

roklenarcic21:08:55

(query [this] `[:level :ui/selected {:items ~(om/get-query MedListItem)}])

tony.kay21:08:11

no need to quote that FYI

tony.kay21:08:46

the value in that map must be a vector of maps or a map…you have a vector

roklenarcic21:08:12

ok, I'll fix that

tony.kay21:08:47

For clarity: results are always trees. A join can point to-one (a map) or two-many (a vector of maps).

roklenarcic21:08:46

hm, same error for (ok {[:medlist-by-level "amp"] {:items [] :level "amp"}})

tony.kay21:08:47

so merge is trying to treat your vector like a map (since it isn’t a vector of maps), and when it does you get an error about it not having a method that is expected on maps

tony.kay21:08:38

feel free to ask about those quicker than 3 hours 🙂 I appreciate not being hounded every 5 mins, but hate to see ppl waste that much time on something that a second pair of eyes can spot easily 🙂

roklenarcic21:08:38

the weird thing is.. why does the error talk about the query

roklenarcic21:08:45

rather than data returned

tony.kay21:08:50

try cleaning your build and make sure. That should work now

tony.kay21:08:15

and you ahve a proper ident function on those classes, right?

gardnervickers21:08:00

@tony.kay We tracked down the problem to the event handlers here https://github.com/fulcrologic/fulcro/blob/develop/src/main/fulcro/client/network.cljc#L94-L95 Wrapping them in (do ...) solves our issue. I’m not sure why that is though.

tony.kay21:08:05

@gardnervickers ooops…shouldn’t that be in opposite order? Register for events before triggering send?

tony.kay21:08:39

that is async…though I would expect problems with super high speed, not super low

tony.kay21:08:11

@roklenarcic any luck with clean build and reload with cleared cache?

gardnervickers21:08:26

We found this fixes our issue though #(do (response-ok this xhrio ok))

tony.kay21:08:53

I cannot believe that is a reliable fix. I pushed a reorder of those statements to beta9-snapshot on clojars

gardnervickers21:08:35

Trying that in my checkouts now

roklenarcic21:08:02

no luck, om.next/merge! is being called with this as query [({[:medlist-by-level "amp"] [:level :ui/selected {:items [:id :name :level]}]} {:filter "a"})]

roklenarcic21:08:22

seems to me that it has one pair of braces too many

tony.kay21:08:06

yes, that query is wrong

tony.kay21:08:14

oh, no it isn’t

tony.kay21:08:16

it has parameters

tony.kay21:08:20

cause you added parameters

tony.kay21:08:37

that query is ok

tony.kay21:08:22

what value is it being called with?

wilkerlucio21:08:43

@tony.kay question regarding multiple remotes, to send mutations to a custom remote we can use the different keys on the mutation map, is there a way to select which remote to use during a fetch/load usage?

gardnervickers21:08:28

@tony.kay removing the (do ... and putting the send after binding the event handlers resolves my issue as well.

tony.kay21:08:40

@roklenarcic Ah…it is a bug, actually

tony.kay21:08:47

I just got it to trigger in a demo

roklenarcic22:08:28

{[:medlist-by-level "amp"] {:items [], :level "amp"}} is the value

roklenarcic22:08:09

yeah that query causes the following call down the line: (conj! transient-array-map a-list-of-1-vector)

tony.kay22:08:09

yeah, what you have is right. merge-ident isn’t handling the parameterized query correctly. I think this is an Om Next bug, but I want to check to make sure. We augment the merge pipeline

tony.kay22:08:38

take the params off of the load, and it will work

roklenarcic22:08:53

yeah that was the only difference between the sample and my code

tony.kay22:08:58

@gardnervickers I’d call that the real bug/fix for that

gardnervickers22:08:11

Sweet thank you for your help

tony.kay22:08:48

@wilkerlucio load has a :remote parameter, if that is what you’re asking

tony.kay22:08:04

as does load-action

tony.kay22:08:36

@gardnervickers thanks for tracking it down…not sure how that made it for two years without causing a problem.

wilkerlucio22:08:43

humm, nice, I'm doing some experiments with multiple networks, I though I was going to have a lot to do, but seems the multiple remotes just have a lot of helpers on that 🙂

roklenarcic22:08:24

so let's say I have 3 lists with search inputs, currently they have idents [:medlist-by-level "amp"] [:medlist-by-level "vmp"] etc... how would you load the search without using ident load with filter as parameter?

tony.kay22:08:44

@wilkerlucio multiple networks is used in serveral places in the demos and such. File uploads use a separate remote for doing progress reports, websockets are in the chat demo, and can be used in tandem with non-websocket remotes.

wilkerlucio22:08:24

thanks for letting me know, seems a good place to look into

tony.kay22:08:59

@roklenarcic So, it is not problem using parameters with ident loads…that is a bug that should get fixed. In terms of your actual question: I would be targeting a field instead of loading an entity. Your search results are not really related to an entity on the server. I’d have a UI component with a :search-results field, do a (load :search/medlist MedListItem {:params {:filter "boo"} :target [:medlist-by-level "amp" :search-results]})

tony.kay22:08:37

the target is always just a conj on the ident + field

tony.kay22:08:34

maybe include “amp” in your parameters, so you have a single keyword to worry about for what to ask the server

tony.kay22:08:00

in your case, I think you called this :items

tony.kay22:08:31

I’ll open an issue on ident load with parameters.

wilkerlucio22:08:00

when it gets here, the query contains the ui part, but the output doesn't, adding the :not-found marker at this point

wilkerlucio22:08:23

which in turn removes it when merging the ident

wilkerlucio22:08:47

I feel like there is some place where strip-ui must be called, but it's not being

wilkerlucio22:08:04

trying to figure where this is exactly, just dumping here to see if it clicks something for you

tony.kay22:08:39

yeah, it should not be reaching there

tony.kay22:08:43

now that I think about it, the strip should really be earlier in the chain. I think it is all the way down at the send. That keeps them from going to the server, but still makes them participate in the mark/sweep…I wonder why it works for me, though.

tony.kay22:08:12

oh, I think the mark is meant to not bother marking the UI stuff

tony.kay22:08:54

Yeah, that’s in the tests. mark-missing does not mark UI attributes as missing

tony.kay22:08:10

there may be a bug there, where mark-missing isn’t working on data with the shape you have

wilkerlucio22:08:47

my ui is a join, as in: {:ui/new-contact [:contact/id :contact/github]}

wilkerlucio22:08:38

the bug might be in the mark-missing since the strip-ui is working correctly in terms of data sent to the remote

wilkerlucio22:08:10

@tony.kay found a minimal case: (fulcro.client.impl.om-plumbing/mark-missing {:a 1} [:a {:ui/anything [:a]}])

wilkerlucio22:08:30

outputs: {:a 1, :ui/anything :fulcro.client.impl.om-plumbing/not-found}

tony.kay22:08:31

that’s pretty minimal 🙂

tony.kay22:08:59

could you open an issue…I’m working on this other one

wilkerlucio22:08:17

I'll just add info on the one you already created

wilkerlucio22:08:25

@tony.kay I think I can work on this one

tony.kay22:08:44

that would be great. This other one is a bug in Om Next.

tony.kay22:08:01

merge-idents doesn’t like queries with params

wilkerlucio22:08:08

just one thing, how do you work to run the tests on dev? you use lein test or there is a faster way to feedback loop?

tony.kay22:08:35

Ah, tests: I would recommend the new fulcro spec browser interface for client and server

tony.kay22:08:45

it allows you to focus tests, which is nice

tony.kay22:08:15

run the figwheel build and include -Dtest to get client tests…you know that one already?

tony.kay22:08:25

for server tests, start a repl and run (server-test-server), then use the URL http://localhost:8888/fulcro-spec-server-tests.html

tony.kay22:08:30

both interfaces look the same

tony.kay22:08:00

if you clikc on the menu in UL corner, you can select only :focused tests…then add that keyword to a spec of interest:

(specification "boo" :focused 
  ...)

wilkerlucio22:08:02

cool, setting up everything now 🙂

tony.kay22:08:03

CONTRIBUTING.md has instructions as well, I think they are up to date

tony.kay22:08:29

the old test-refresh still works, but the focused browser interface makes it look like cave-man times

wilkerlucio22:08:39

yes, testing cljs in general still a daunting task, most people are still depending on test-refresh, which is too slow...

wilkerlucio22:08:00

nice, Fulcro Spec is looking really good! last time I saw it was just a bunch of outlines XD

tony.kay22:08:47

I don’t like the icons on top for failed filtering, etc…but it is much nicer to use

tony.kay22:08:07

I think at least some kind of hover to tell you what they mean would be nice. On the wish list 🙂

tony.kay22:08:14

other priorities

tony.kay22:08:27

the focused stuff is my true love, though

tony.kay22:08:39

being able to turn on/off test sets rocks.

tony.kay22:08:56

esp when testing this SQL stuff where I want to turn on/off integration stuff on the fly

wilkerlucio22:08:25

yes, it's nice 🙂

wilkerlucio22:08:43

had you though about being able to focus tests from the UI?

tony.kay22:08:47

@roklenarcic turns out this was patched on 3/17/2017 in Om Next. I just did the same patch and was about to submit it…and it conflicted

tony.kay22:08:56

which is a date before beta1…not sure why it isn’t on beta1

tony.kay22:08:29

oh…unless the merge of the PR happened after…in which case the history would show the old patch date even though it isn’t in a newer release

tony.kay22:08:11

confimed. beta1 does not have the patch.

tony.kay22:08:01

@roklenarcic The issue in Om Next was #853

tony.kay22:08:22

Of general interest to the channel (and probably beyond). This is Javascript underneath, so there are some fun hacky things you can do to debug/work on errors like this while avoiding checkouts and that whole mess. To work on the patch for the issue I was just looking at, I needed to be able to edit Om Next’s om.next/merge-idents function. So, I copied the code for it into one of my own namespaces. From there, you just have to know what the name gets mangled to in js. I just did this:

(defn install-hack []
  (js/console.log "Installing hack")
  (set! js/om.next.merge_idents merge-idents))
Then I could edit my local version until it worked.

tony.kay22:08:03

of course, I needed to run install-hack to install it, and watch for hot-code reload overwriting it.

wilkerlucio22:08:19

@roklenarcic @tony.kay in this case, would not be easier to just use the om.next from master?

tony.kay22:08:39

? what do you mean

tony.kay22:08:54

I was just doing that to debug…I don’t want to do a whole local install to maven

wilkerlucio22:08:08

ah, the copy namespace trick, I use that too, was using to debug the previous bug 🙂

tony.kay22:08:12

once we know it is patched (as we do now), then of course that is the way

wilkerlucio22:08:34

I said that because you mentioned this fix was merged in Om, just not released yet

tony.kay22:08:48

I see…yeah, but I didn’t know that until I fixed it 😜

tony.kay23:08:02

which is the downside of not using checkouts

wilkerlucio23:08:15

by the way, one PR of mine that got merged will break some of Fulcro tests, hehe, heads up: there is an issue with strip-ui, because it forces all AST nodes to have children (adding for those that doesn't have it), the PR that I got merged enables having children on call types, and this will make the output from ast->query to change for those cases

wilkerlucio23:08:25

not a major thing, I just got reminded about it

tony.kay23:08:57

@wilkerlucio children on call types…was that the thing you wanted for your graph queries?

wilkerlucio23:08:38

it's kind of a requirement there, in GraphQL when you do a mutation you must declare the returned data from the mutation, otherwise it doesn't run

wilkerlucio23:08:38

but I still have to debug something weird that's happening there, because when I try to add children on the mutation this breaks the entire process, still have no idea why, its on my queue to investigate eventually...

tony.kay23:08:09

are there tests?

wilkerlucio23:08:20

for what part?

tony.kay23:08:25

or do you know what PR?

tony.kay23:08:29

I wanted to look it over

tony.kay23:08:04

oh boy…that kind of makes a mess 😞

wilkerlucio23:08:04

well, on most cases it should not affect, since nobody seems to be using call with children, what mess cases came to your mind?

tony.kay23:08:18

I mean in general maps in a query were always joins…now it is more vague.

tony.kay23:08:41

what does that expression mean, exactly? Run a mutation that you expect to return something?

wilkerlucio23:08:30

yes, think on cases where you are creating or updating some entity, instead of doing a follow up read in a separated requests, you can pull data from it directly from the mutation

tony.kay23:08:35

I guess it won’t affect loads, since it would never appear there

wilkerlucio23:08:43

this is a common thing in GraphQL world

wilkerlucio23:08:55

had you played with the GH GraphQL API? https://developer.github.com/v4/explorer/

tony.kay23:08:40

I see, so you could say [{(add-card ~params) [{[:person/by-id ~id] [:db/id ...]}]}]

tony.kay23:08:54

to add some data to a person and then refresh them

wilkerlucio23:08:27

I would be more like: [{(create-user {:name "bla"}) [:user/access-token]}]

wilkerlucio23:08:42

the read is rooted at the created entity

tony.kay23:08:10

so, then, did you update the Om Next plumbing to support this 😜 ?

wilkerlucio23:08:26

nop, but it's there available now 🙂

wilkerlucio23:08:44

I think it might be some optimisation point for later

wilkerlucio23:08:58

right now I just needed that to make my graphql mutations valid

tony.kay23:08:00

but the current parser, if it isn’t modified to support it, just throws away the return value

tony.kay23:08:47

but now that you have a query available, it should be a minor fix to get this working without needing to hack into merge, which would be nice

wilkerlucio23:08:28

expect somewhere in the Om pipeline, doing a mutation with children just breaks stuff, I want to dig to figure why is that, it's bugging me

tony.kay23:08:12

of course, the query, if nested, would have to be pulled from a component

tony.kay23:08:28

perhaps it isn’t intended to be recursive (joined) in nature?

wilkerlucio23:08:13

what you mean by recursive joined in nature?

tony.kay23:08:42

[{(create-user {:name "bla"}) ~(om/get-query Person)}]

tony.kay23:08:59

person might have joins going down into the graph

wilkerlucio23:08:29

that's ok I think, as long as the nodes are available I don't see any restrictions

tony.kay23:08:32

oh, but how are we supposed to know the ident of the target of the mutation

wilkerlucio23:08:51

you make the ident before, using tempid

tony.kay23:08:53

you might run this from a callback

tony.kay23:08:07

in another component

wilkerlucio23:08:56

ah, you mean how to automatically know where to merge the data from the response, right?

tony.kay23:08:05

Perhaps I’m being too vague. Let me spell out what I’m seeing in my head 😉 1. You send the mutation with query as it’s rv query. The server can figure out from the parameters and mutation name what entity you’re affecting. As such, it can return data corresponding to your query…As if you’d issued a load with [{ident query}]…so it can return a map {ident map-value.

tony.kay23:08:43

2. Om can pretend that it issued that query, and merge the result

tony.kay23:08:14

actually, the ident is present (from the server) so Om can even make up the original query once it has the return value

tony.kay23:08:22

and call merge

tony.kay23:08:03

Of course that doesn’t completely eliminate the desire to do some action in response to the return (which still requires augmenting merge), but it does serve a subset of cases that are useful.

wilkerlucio23:08:00

yes, this gives you a good extra access point relative to the affected operation, I think it's a nice to have

wilkerlucio23:08:11

* confirmed that ui mark missing is fixed in my demo 🙂

tony.kay23:08:17

it wouldn’t be a change to the parser, so much as a change to merge

wilkerlucio23:08:23

lately I've being thinking about a future where we have many graph apis around, and that we can "plug" one into another

wilkerlucio23:08:48

@tony.kay if you are down to, this is the demo I've being playing with: http://407d8e53.ngrok.io

wilkerlucio23:08:54

(slow, dev build...)

wilkerlucio23:08:42

in this one you can see the integration of 2 graphql API's, there is Graph.cool one that handles storage of groups and contacts, and each contact is a github handler

wilkerlucio23:08:00

by using the github handler I can hook it up into Github GraphQL

tony.kay23:08:16

it wants a token

tony.kay23:08:25

is this an API token to your own github?

wilkerlucio23:08:27

ah, right, it's a github token, you can issue a personal token

wilkerlucio23:08:53

you can issue a readonly here ^^^

wilkerlucio23:08:48

by having this apis "connected", the Contact component can look like this:

wilkerlucio23:08:50

(om/defui ^:once Contact
  static fulcro/InitialAppState
  (initial-state [_ _] {})

  static om/IQuery
  (query [_] [:contact/id :contact/github
              {:contact/github-node
               [:github/avatar-url :github/name :github/company]}])

  static om/Ident
  (ident [_ props] [:Contact/by-id (:contact/id props)])

  static css/CSS
  (local-rules [_] [[:.container {:text-align "center"}]
                    [:.avatar {:width "100px" :height "100px"}]])
  (include-children [_] [])

  Object
  (render [this]
    (let [{:contact/keys [github github-node]} (om/props this)
          css (css/get-classnames Contact)]
      (dom/div #js {:className (:container css)}
        (dom/div nil
          (dom/img #js {:className (:avatar css)
                        :src       (:github/avatar-url github-node)}))
        (dom/div nil github)
        (dom/div nil (not-found (:github/name github-node) ""))))))

wilkerlucio23:08:50

I find this really cool, to be able to "jump" from one graphapi to another

tony.kay23:08:55

yeah, there’s a lot of potential

tony.kay23:08:11

your changes break 2 tests, FYI

tony.kay23:08:49

oh, maybe 3

tony.kay23:08:44

actually, 2 of them were on a utility function that was technically wrong…and the new version fixes the function (which breaks the test). Wrong is kind of a strong word, since the function in question was just for logging.

wilkerlucio23:08:05

hehehe, devil details

tony.kay23:08:48

so, it looks like ast->query now always puts mutations into a map when converting back from an AST…is that right?

tony.kay23:08:06

the test that is failing is showing a mutaiton that got changed to be in a map

tony.kay23:08:20

with an empty [] query

wilkerlucio23:08:45

that happens if you have children I think, if the children is nil this should not happen

tony.kay23:08:29

ah, ok, so strip-ui does need to change slightly

wilkerlucio23:08:34

yeah, I noticed that when using the beta, the strip-ui currently adds children to everybody

tony.kay23:08:42

relatively easy fix.

tony.kay23:08:54

thanks for the heads-up

wilkerlucio23:08:00

no problem 😉

tony.kay23:08:09

pushed a beta9-SNAPSHOT with your patch @wilkerlucio

wilkerlucio23:08:38

thanks, one less lein install to do when changing computers 🙂

tony.kay23:08:30

@gardnervickers I’m seeing a PR…confused. I thought we had that fixed