Fork me on GitHub
#om
<
2016-01-20
>
andrewboltachev02:01:22

My callback receives data from remote read, but components aren't updated after that https://github.com/andrewboltachev/html2om/blob/master/src/cljs/html2om/core.cljs#L118 What can cause this?

andrewboltachev02:01:27

UPD: figured out that neither props contain :om-text/om-text nor render is called on cb launch

noonian02:01:51

@andrewboltachev: I think the issue is that your read fn for :om-text/om-text will never return a :value. Your components only ever get data from your parser, so you need the read fn to return the local :om-text/om-text data from your apps app-state.

andrewboltachev02:01:56

@noonian: yep I were looking at "autocomplete" example from devcards (this one uses custom remote also), and it were providing :value for remote read also.

noonian02:01:27

Yeah, you probably want to only return a remote if the local value isn’t already in the state

noonian02:01:35

and the local value otherwise

andrewboltachev02:01:58

such "memoization" is cool, but say I want it to call the server in any case — would I need :value then also?

andrewboltachev02:01:27

and, how would it work then? i.e. when I return {:value "" :remote true}

andrewboltachev02:01:03

would the value be first set to empty string and then to the fetchd one or what?

noonian02:01:23

Not sure, but I think you need to be careful of an infinite cycle of remote requests. I haven’t played with om.next for a bit, but I think after you remote callback happens, the data gets merged to the app state and then your parser will get called again to re-render the component. If the parser always returns a remote that basically means always hit the server so it could keep making requests.

noonian02:01:48

something like this is probably where I’d start:

(defmethod readf :om-text/om-text
  [{:keys [state] :as env} k params]
  (let [st @state
        local-val (:om-text/om-text st)]
    (if local-val
      {:value local-val}
      {:remote true})))

andrewboltachev02:01:57

not sure what it is, but e.g. for me it's {:type :prop, :dispatch-key :om-text/om-text, :key :om-text/om-text, :params {:html {:value safdsfsfd}}}

noonian02:01:08

Yeah, I think :remote true is syntax for returning :remote ast where ast is the value from the parser environment.

noonian02:01:40

I suspect to make quick-starts simpler

noonian02:01:24

but if you return the ast, you can tweak the ast node yourself so the remote query is not exactly what the local query is

andrewboltachev02:01:47

btw, one branch in your code is original to mine, i.e. just {:remote true}

noonian02:01:26

Yeah, I just modified your version

noonian02:01:54

because it only returns {:remote true} if the local data isn’t there, in my version it will only send 1 request to the server

andrewboltachev02:01:55

and... would UIs that implement IQuery with (:om-text/om-text {:html ?html}) be updated then?

andrewboltachev02:01:17

I mean would render be called

noonian03:01:17

I think after you return {:remote true}, then your server will return the data and it will get merged into the app state. At that point, your parser will be called with the query for your component and your component’s render function will be called with any local data returned from the parser

noonian03:01:45

I have to take off

andrewboltachev03:01:59

Thanks for great help @noonian !

andrewboltachev03:01:08

I did try this autocomplete now

noonian03:01:12

Np, take care!

andrewboltachev03:01:17

it doesn't work on my versions of stuff

noonian03:01:00

Just make sure at some point you return a :value for all of your query keys. Otherwise, you components wont get that data.

andrewboltachev03:01:11

though, works... (seemed like not at first)

andrewboltachev04:01:41

@noonian: FYI fixed. And the way it works is a bit unclear for me. See I had to return the map with both keys: https://github.com/andrewboltachev/html2om/blob/master/src/cljs/html2om/core.cljs#L88

txus09:01:34

last night I integrated local storage persistence for one component with an om.next remote! so good

txus09:01:56

I could even reuse the same parser

nano09:01:11

humm... well crap.. having an om app in figwheel mode crashes the Samsung Smart TV Tizen browser 😞 Not sure how to debug, as there is no way of checking logs remote. I did a hack by redirecting console.log to a <div> but no luck catching any javascript errors. Might be some relevant log happens at the time of the crash, but then the browser is gone so can't see that. Via wireshark (tried loading the app content directly from my laptop via an iframe) I've seen that it reaches the loading of transit writer. I'm not using transit, but my attempts to simply comment out any transit loads didn't help (the idea being either OOM or that transit writer somehow triggers some bug in the browser). A production build works just fine though. Is it possible to do partial minification and still have figwheel? Any suggestions?

dottedmag10:01:44

@nano: A stupid question: does it fail in emulator?

nano10:01:52

@dottedmag: The emulator made me cry so I gave it up and developed the app in chrome instead simple_smile but now it's time to start using the device specific apis so no escape any longer. But good point. Should try it out.

nano10:01:35

Hopefully it's reproducible in the emulator and it can give me more details on what happens.

dnolen11:01:29

@nano does the SmartTV use WebKit?

nano12:01:45

@dnolen: User-Agent: Mozilla/5.0 (SMART-TV; Linux; Tizen 2.3) AppleWebkit/538.1 (KHTML, like Gecko) SamsungBrowser/1.0 TV Safari/538.1 Accept-Charset: iso-8859-1, utf-8, utf-16, *;q=0.1

dnolen12:01:30

yeah that might be it

dnolen12:01:56

versions of WebKit have a very bad bug with deeply nested invokes

dnolen12:01:11

you may need to modify your dev builds with :static-fns true compiler option to avoid issues

nano12:01:19

cool.. i'll try that out.

dnolen12:01:24

it’s a known WebKit problem - already filed with Apple

nano12:01:56

Excellent, now it starts at least, doesn't connect to figwheel websocket though. But that's hopefully easier to figure out why.

nano12:01:59

works! oh this is brilliant.. just hacking in the sofa while the 55" TV runs the UI ❤️

anmonteiro12:01:28

wow that's great.

anmonteiro12:01:49

Where are you on the reloading issue?

nano12:01:22

Keeping state? Haven't looked any more at that. Moved on to get the figwheel setup running on the TV instead. It would be nice if state was kept, but my navigation depth in the app is only 3 steps so it's not that much of an inconvenience. Not sure if you saw but I think it's the static IQuery that messes up. My routing details in the app-state are correct after the reload, but the component seems to get its original query on reload.

anmonteiro12:01:17

@nano: right, that makes sense because defui will just overwrite those functions on reload

anmonteiro12:01:37

but that makes sense, IMO, if you change those functions you want them to be changed on reload

nano12:01:47

Mhm.. but as I apply set-query when updating the route it would be nice to keep that.

anmonteiro12:01:06

@nano: you can't have both simple_smile

nano12:01:07

Yep, would be fine with not being able to reload the root component's query without a full reload of the page as that's not something that will change much.

dnolen13:01:52

@nano (defui ^:once Foo …) should work for this case

dnolen13:01:11

as long as the class doesn’t change on reload you should be able to preserve component local state

dnolen13:01:23

queries are stored in the app state atom - so you’ll want to protect that too with defonce

anmonteiro13:01:59

@dnolen: I've expressed this concern previously, and please correct me if I'm wrong. I think that all defui ^:once does is preventing a new class to be instantiated (by not calling its constructor). However, from the experiments I've done with reloading, the class's prototype is patched and then add-root! mounts this new (changed) class, which doesn't keep component local state

dnolen13:01:34

@anmonteiro: that’s not how React works

dnolen13:01:45

if the type doesn’t change you will preserve component local state

dnolen13:01:49

patching the prototype doesn’t matter

anmonteiro13:01:56

right, I know they don't create a new instance

dnolen13:01:11

component local state isn’t stored on the class

dnolen13:01:15

it’s stored on instances

anmonteiro13:01:20

but somehow I wasn't able to get compnent local state to be maintained

dnolen13:01:22

and not even really instances

dnolen13:01:30

React keeps an internal state map

anmonteiro13:01:31

and I'm not sure what I've missed then

dnolen13:01:38

and assigns component local state based on diffing

anmonteiro13:01:55

cool to know that, I didn't

dnolen13:01:16

in anycase, it should work - but it may be that there’s a bug that prevents it from working

anmonteiro13:01:32

@dnolen: right, so I'll try again and report back with a minimal case

anmonteiro13:01:40

if I'm able to find one

anmonteiro14:01:44

@dnolen: I'm definitely seeing the constructor getting called on reload

dnolen14:01:03

@anmonteiro: like I said that’s not how React works

dnolen14:01:11

the constructor will be called every time

anmonteiro14:01:23

oh that's not what I understood

dnolen14:01:45

this is why the state map lives outside components

anmonteiro14:01:49

well, then, if defui contains this: (set! (.-state this#) (.initLocalState this#))) then local state will never be maintained...

dnolen14:01:49

and is handled via diffing

dnolen14:01:27

yes that would be the bug

anmonteiro14:01:08

so there's a bug; I just was expecting React not to call the constructor everytime

anmonteiro14:01:28

should be an easy fix, do you want to work on it or should I?

nano15:01:22

@dnolen: My query changes after I reload, even with (defui ^:once on all components, and (defonce app-state (atom ....)). I'm doing routing via queries, but the om/IQuery func declares a default route, [:session/route :session/params {:child (route->query :categories)}], and this is the query I get after performing a change and figwheel reloads.

dnolen15:01:58

@nano you will need to produce minimal case and file an issue

nano15:01:12

Will do.

dnolen15:01:12

no links to external repos, everything must be inline in the issue

dnolen15:01:43

the issue is very low priority for me - but I’m happy to take a PR provided you submit a CA

dnolen15:01:24

otherwise, it will have to wait post the error stuff & routing hooks

dnolen15:01:54

making progress on the error stuff again, so hopefully won’t be too long before I can help out on these lower priority issues myself.

nano15:01:23

family play time, back in ~6h

jlongster16:01:59

the only time that :dispatch-key and :key are different in the query AST is with an entity reference (ident), right?

jlongster16:01:42

yeah that's the code I was looking at, thanks

jlongster16:01:19

@jannis: I got a basic app working that runs queries on the backend against a sqlite db! this is going to be awesome (needs some more fleshing out before it's actually practical)

jlongster16:01:56

right now all I do is convert a join query into a SELECT statement which is straight-forward, not sure what else I will need yet

jannis16:01:01

Cool! It's great to see people use Om Next sucessfully with all kinds of backends

jlongster16:01:55

the straight-forward-ness of the API is what made me try it (relay/falcor look way more complicated). plus, I've been meaning to actually use CLJS on a project

anmonteiro17:01:00

@dnolen: regarding the local state reload issue, not finding it straightforward to know where to set! the state; if the constructor is called everytime and we don't set it there then the component state will be null

dnolen17:01:21

@anmonteiro: maybe you should see if mounted? works in the constructor

dnolen17:01:49

it maybe also be the case that we need to set state when the component mounts before user code

dnolen17:01:19

@jlongster: that’s cool that you have a sqlite thing going

dnolen17:01:27

been wondering how long that would take, a blog post about that would be sweet

anmonteiro17:01:43

@dnolen: OK thanks for pointing me in some new direction; will check that out

dnolen17:01:35

@anmonteiro: it might be a little bit fiddly but that’s OK

dnolen17:01:44

the React integration bits generally are

anmonteiro17:01:23

@dnolen: yep, I've been looking at some React source to try and see if there are any extension points related to this bit

jlongster17:01:35

@dnolen: definitely going to blog once I feel more comfortable with this! it's very basic but I'm eager to get filtering and other queries working. should be pretty neat.

anmonteiro17:01:49

it seems that if we were using React.createClass it would be easier

dnolen17:01:58

@anmonteiro: related, I’d completely forget that static prop is a thing

dnolen17:01:18

(defui Foo static prop state #js {:foo 1}) might be another route

anmonteiro17:01:42

didn't know about that

dnolen17:01:00

@anmonteiro: would need to set it on the prototype I think

dnolen17:01:03

instances could override

anmonteiro17:01:08

OK let's see what I come up with given these new bits of info

dnolen17:01:37

er an that should be a map above, not a JS literal

dnolen17:01:56

I would try the property direction last since we need to special case it in the defui macro

dnolen17:01:06

the immutable state prop needs to be wrapped in a JS object etc., etc.

anmonteiro17:01:00

@dnolen: right. I would also be more comfortable going in a way other than that

anmonteiro17:01:32

gotta run now, I'll play with all this later

dnolen17:01:42

cool, thanks!

taylor.sando17:01:52

Is there a way to get child components? I know there's a children function, but looking at what is actually returned by react, if you examine .-props and .-children, it is null. Also, react-ref, which is -.refs <name> just returns a dom node, not a component. I ask because I was looking into using set-query! and making it work from the root to child components. The root usually uses get-query, but it's grabbing the static query of the components, and not necessarily the instance of the child component's current query--which may have changed during runtime.

dnolen18:01:17

@taylor.sando: you can set-query! from the root, i.e. make your own way for components to set queries if this is necessary

dnolen18:01:48

attempting to do this by walking children is unlikely to ever be supported

taylor.sando18:01:13

So all parameters that would change would need to be stored at the root, or in the app state?

dnolen18:01:05

you can do it however you like

dnolen18:01:15

Om only ships the primitives here, write whatever abstractions and helpers you want/need

sjol19:01:32

I've been trying to get reforms into my project but I can't seem to have a for start with initial values they actually get erased once I load up the component. Any tips or alternatives to pre-populate a form with reforms? or should I be using something like formative?

anmonteiro19:01:39

@dnolen: something I just confirmed which I'm not sure is expected behavior

anmonteiro19:01:15

whenever we reload a file, these methods are triggered in an Om Next component: componentWillMount and componentDidMount

anmonteiro19:01:31

however, if I use BHauman's defonce-react-class, the component will trigger componentWillReceiveProps, componentWillUpdate and componentDidUpdate instead

anmonteiro19:01:52

I somehow was expecting that the component would remain mounted, allowing us to set the initial state in e.g. componentWillMount before user code, as you suggested

anmonteiro19:01:05

it seems that React also uses componentWillMount for some of their own initialization logic

anmonteiro19:01:23

so atm I'm considering that add-root! is doing something that causes Om Next components to unmount on reload, and I'll start pursuing this direction

dnolen20:01:02

@anmonteiro: I would make sure this isn’t a devcards problem before going on a wild goose chase first

anmonteiro20:01:19

@dnolen: I haven't been using devcards to test all this

anmonteiro20:01:26

all directly on the DOM

anmonteiro20:01:42

so I've found the root cause now

anmonteiro20:01:07

whenever we call add-root! again (e.g. on reload), remove-root! unmounts the component.

anmonteiro20:01:20

And I knew this, I was just choosing not to see it simple_smile

dnolen20:01:51

ah yeah I though you were talking about things below the root

dnolen20:01:59

that should be easy to handle in user code too though

anmonteiro20:01:02

I'm not sure what to do given this information

anmonteiro20:01:29

what do you mean handle that in user code?

dnolen20:01:30

nothing wrt. to add-root!

dnolen20:01:44

yes, for dev make sure you only call that once

anmonteiro20:01:09

uhm, let's see how that pans out

dnolen20:01:01

@anmonteiro: it’s easy to do

dnolen20:01:09

just put the root component in a var initialized to nil

dnolen20:01:18

if it’s been set, don’t add-root! etc.

noonian20:01:26

I’ve been calling add-root! on each figwheel reload to re-render the app, but I suppose you could achieve the same behavior using the touch! helper when that lands.

anmonteiro20:01:45

@dnolen: what I did was defonce the add-root! call?

dnolen20:01:13

@anmonteiro: heh right, that’s the one-liner version

anmonteiro20:01:22

OK but there's a problem

anmonteiro20:01:28

now the component just doesn't update at all on reload 😄

anmonteiro20:01:54

Local state is maintained ofc (yay?) but if I change the render method it doesn't update

noonian20:01:23

until touch! lands, you can probably pull the root component instance out of the reconciler and call .forceUpdate on it directly on figwheel reload

anmonteiro20:01:24

also, not sure touch! is supposed to be used to do that

anmonteiro20:01:31

depends on the use case, I suppose

anmonteiro20:01:52

@noonian: there's also force-root-render! which seems more appropriate

noonian20:01:59

Ah, that sounds perfect for this use case. I haven’t been able to play with Om.next very much lately (got a new job) so I didn’t know it existed.

anmonteiro20:01:36

though it doesn't seem to be doing anything

dnolen20:01:08

@anmonteiro: pretty sure figwheel has a hook for reloads

dnolen20:01:22

you can supply a callback that does what you need for reloading there

anmonteiro20:01:58

it does, yes. Let's see. However (.forceUpdate c) doesn't seem to update the component when changing the render method

hugod20:01:06

An attempt at a minimal (failing) attempt to use om.next with the React Native Navigator and ListView components. https://github.com/hugoduncan/navigator-repro

dnolen20:01:25

@hugod: so you can’t repro that outside of React Native?

anmonteiro20:01:27

@dnolen: so can't get component to update without calling add-root! again. And if I do so it blows away local state. Doesn't seem like a very good position to be in

hugod20:01:56

@dnolen: I can’t imagine it is a problem outside of React Native. I get the impression that the Navigator and ListView components manage their children in ways that om.next doesn’t expect.

anmonteiro20:01:33

I'm not sure it's a figwheel thing or that I'm doing something wrong; the instance just doesn't get the new render method on reload, I don't know why

dnolen20:01:49

@hugod right so do you have a theory why it doesn’t work?

dnolen20:01:57

@anmonteiro: so you see the old render method get invoked though?

anmonteiro20:01:14

it's calling componentWillUpdate, render and componentDidUpdate (now as expected). the old ones though simple_smile

anmonteiro20:01:30

could it be a defui bug? I just don't see where

dnolen20:01:23

@anmonteiro: I would test by examining the prototype

dnolen20:01:42

double check the prototype of the reloaded thing and that of the component

dnolen20:01:48

should be simple enough to do with Chrome DevTools

hugod20:01:24

@dnolen: I’m guessing the component props don’t have the right omcls$… values set on them, but I can’t see what the expected code path is in full-query for a non root component that doesn’t have a class-path match and doesn’t have a path set on it.

anmonteiro20:01:55

@dnolen: before doing that, let me rule out anything I might be doing wrong. How would you make a component update on reload?

hugod20:01:44

@dnolen: I’m wondering if path should be set via ident, and/or why full-query doesn’t use ident

dnolen20:01:12

@hugod: your theory doesn’t match anything that might be related to React Native so far

dnolen20:01:47

@anmonteiro: forceUpdate should work

anmonteiro20:01:16

@dnolen: it's how I'm doing it. I've tried that one and force-root-render!

anmonteiro20:01:32

I have a theory though, but might be wrong

dnolen20:01:32

@anmonteiro: I’m just talking about inspecting the prototype

dnolen20:01:34

that should be good enough

anmonteiro20:01:54

the thing is that the instance that's already there wouldn't get the new prototype

dnolen20:01:22

@hugod there’s also the React Native channel, have other people mentioned this issue?

anmonteiro20:01:24

I believe this is what happening. we would need to set __proto__ on it, but that's not cross-browser compliant

dnolen20:01:40

seems like there’s at least a couple other people trying Om + RN

dnolen20:01:53

@anmonteiro: proto__ is a de facto standard at this point

dnolen20:01:01

that might just be a bug on our end then

anmonteiro20:01:53

@dnolen: I don't get it. A live instance doesn't have a prototype, AFAIK. so setting the class's prototype would not do anything

hugod21:01:31

@dnolen: well, the components will do things like create components outside of an om render! call. I guess I could try and implement Navigator and ListView like components outside of ReactNative.

dnolen21:01:07

@hugod: so you’re passing along a Om view to be constructed? A factory?

jlongster21:01:31

you can use Object.setPrototypeOf as well to set prototypes

hugod21:01:18

@dnolen: As an example, I push a view onto the navigator, by passing it the factory function, which then gets used by the navigator component to re-render itself.

dnolen21:01:38

@hugod: right so that pattern may need more support

dnolen21:01:54

we dynamic bind a couple of things that children need to read

dnolen21:01:18

this has to be done in render because of React implementation details

hugod21:01:34

@dnolen: right, and my project above includes such a binding (`with-om-vars`)

dnolen21:01:12

@hugod so are you are or aren’t you saying the issue is that the dyn vars don’t get propagated correctly with the RN views?

jlongster21:01:16

@anmonteiro cool! I think it's the same behavior as setting proto__, just more standards compliant. although Safari doesn't support sPO (according to MDN). if just for dev, should be ok

anmonteiro21:01:52

@jlongster: yep, I went to read it on MDN, they also show a polyfill

hugod21:01:16

@dnolen: I think I am correctly propagating the dynamic var bindings. The list view is not behaving correctly though, so I am trying to understand what else is missing.

dnolen21:01:54

@hugod: do you have a link to what you’re doing with with-om-vars

dnolen21:01:07

I generally don’t like surveying repos for context

dnolen21:01:14

which is why I haven’t looked at it yet

dnolen21:01:49

@hugod the macro isn’t useful to me … where are you using that … that’s all that matters

dnolen21:01:11

if you haven’t monkey patched the RN render methods unlikely to make any difference at all

anmonteiro21:01:03

Alright. So this is working:

(defonce root (atom nil))
(defn init []
  (if (nil? @root)
    (let [target (js/document.getElementById "app")]
      (om/add-root! reconciler RootView target)
      (reset! root RootView))
    (let [c (om/class->any reconciler RootView)]
      (js/Object.setPrototypeOf c (.-prototype @root))
      (om/force-root-render! reconciler))))

anmonteiro21:01:27

for reload hooks w/e

anmonteiro21:01:47

@dnolen: my question is: should the prototype patching make its way into Om?

anmonteiro21:01:04

I'd say no, it seems to me that it's only a dev time helper

anmonteiro21:01:14

but you might be thinking of something I'm not

dnolen21:01:26

@anmonteiro: if there’s a way that covers most browsers I don’t see the problem

dnolen21:01:42

i.e. prototype setter helper that works for Safari, FF, Chrome, IE

anmonteiro21:01:57

@dnolen: could get away by using MDN's proposed polyfill

anmonteiro21:01:10

which is setting __proto__ if setPrototypeOf is not available

anmonteiro21:01:35

I just wouldn't know where in Om's code to put it, actually. maybe in force-root-render! ?

dnolen21:01:35

sounds fine to me, just needs some verification

anmonteiro21:01:56

mind that we need both access to the class and the instance

anmonteiro21:01:29

force-root-render! is in the context of the reconciler so it has access to both

dnolen21:01:13

@anmonteiro: so the problem may be that we blow away the prototype during relaod

dnolen21:01:24

line 189 of the defui macro, might want to mess with that first

anmonteiro21:01:13

@dnolen: the problem of what?

anmonteiro21:01:14

the mounted instance won't even see that update, IMO

anmonteiro21:01:23

since it's instantiated?

dnolen21:01:31

@hugod that should work

dnolen21:01:40

@anmonteiro: prototype lookups are dynamic, it should work

dnolen21:01:53

@hugod your repo is just not minimal enough for me

dnolen21:01:02

way too much stuff I don’t want to look at

dnolen21:01:23

I’ll happily look at a small RN project that demonstrates the issue and debug it

hugod21:01:06

@dnolen: Thanks - I’ll try and shrink it some more then

anmonteiro22:01:17

@dnolen: you were right about the defui macro. that line is problematic.

anmonteiro22:01:41

question: is it OK that we blow the prototype away if ^:once is not specified?

dnolen22:01:05

I don’t think it’s a problem and we can probably tweak that later

anmonteiro22:01:49

alright then. patch incoming

anmonteiro22:01:28

that should do it. force-root-render! now works as expected on reload

anmonteiro22:01:34

thanks for the guidance!

steveb8n23:01:15

I’m trying to find the devcards based example that someone built showing how to wrap a client parser around a 3rd party api. Does anyone know where that is? Asking for a friend 😉 update: I found it… https://wilkerlucio.github.io/om-cookbook/#!/om_cookbook.Parsing_Service_Databases_as_Remote

thosmos23:01:50

@sjol I've used reforms by using the rum integration: https://github.com/aspasia/rum-reforms In the render method, I created a regular atom around the props, added a watcher, passed that to reforms, and inside of the watcher's callback did an Om transact with the updated data. It's not really rum specific, since it just uses an atom instead of a cursor.

andrewboltachev23:01:01

Hi guys I've just finished (though rather "started") "HTML2Om" project, intended to help converting HTML to (dom/div #js {:className ...} ...) Om syntax. It's Om Next webapp itself. Please check it out: https://github.com/andrewboltachev/html2om Feedback and critics would be highly appreciated!