Fork me on GitHub
#om
<
2017-04-14
>
peeja17:04:26

@anmonteiro It appears to me that the compassus parser, for remote reads (that is, with a target), returns not a query but a single-element vector containing the query. Is that a known issue?

peeja17:04:46

(It doesn't seem to break Om, though I'm not sure why.)

anmonteiro17:04:38

do you need a different behavior?

peeja17:04:53

Wouldn't it be better to make the parser do the right thing?

anmonteiro17:04:32

@peeja so we previously had expr->ast in the parser

anmonteiro17:04:38

but you noticed that wasn’t the right behavior 🙂

peeja17:04:52

But we own the parser. We can make it return anything, right?

peeja17:04:57

We can just unwrap it there

anmonteiro17:04:12

I don’t recall what the problem was exactly

peeja17:04:30

I think it was that it returned the query wrapped in a vector 🙂

anmonteiro17:04:12

oh right, we were doing (expr->ast (first ret))

peeja17:04:18

Yeah, that was it

anmonteiro17:04:27

thus ignoring (rest ret)

anmonteiro17:04:40

I don’t see any easy fix

peeja17:04:46

I do. I can PR it

peeja17:04:07

We just need to make it the parser's job, not the read's job

anmonteiro17:04:41

@peeja PR welcome if you know how it should be fixed

peeja17:04:54

Hmm. Do we have some guarantee that :compassus.core/mixin-data and :compassus.core/route-data won't be read at the same time?

peeja17:04:08

The current setup appears to assume they won't

anmonteiro17:04:05

I think they could?

anmonteiro17:04:13

when you’re re-rendering from root

peeja18:04:59

Oh, no, you're right. There are test cases that cover it.

peeja19:04:03

@anmonteiro So, if I'm reading this right, if :compassus.core/mixin-data and :compassus.core/route-data are read at the same time, the result of the parser will be a vector of two elements, one for each of those queries. Then wrap-send will concatenate those queries into a single query, and that's what'll go to the send.

peeja19:04:15

But: if the two queries have any keys in common, that'll break

peeja19:04:07

I'm not sure what the best way to deal with that is, but that's a potential issue that exists today.

anmonteiro20:04:44

I think that’s fair

anmonteiro20:04:03

@peeja thanks. I’ll review during the weekend