Clojurians
# om

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

markstang 00:59:29

Has anything changed with ref-cursors with the new release of ClojureScript?

markstang 01:00:12

Or has anyone had any issues with ref-cursors not firing?

dnolen 01:06:45

I haven’t heard anything so far

markstang 01:14:18

I put an observe on app-state and it triggered my rendered-state, but when I observe sub items they don't trigger it.

markstang 01:15:46

I recently upgraded to the latest version of ClojureScript, but my version of Om is at .90

markstang 01:17:06

[org.omcljs/om "0.9.0"]

dnolen 01:19:35

@markstang: make a minimal thing and file an issue - don’t link to a project or anything like that

dnolen 01:19:47

you should be able to demonstrate the problem with a few lines of code inline in the issue

andrewboltachev 04:32: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?

andrewboltachev 04:39:27

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

noonian 04:42: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.

andrewboltachev 04:44: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.

noonian 04:45:27

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

noonian 04:45:35

and the local value otherwise

andrewboltachev 04:46:58

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

andrewboltachev 04:47:27

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

andrewboltachev 04:48:03

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

noonian 04:49: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.

noonian 04:49: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})))

andrewboltachev 04:50: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}}}

noonian 04:51:08

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

noonian 04:51:40

I suspect to make quick-starts simpler

noonian 04:52: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

andrewboltachev 04:53:47

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

noonian 04:54:26

Yeah, I just modified your version

noonian 04:54: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

andrewboltachev 04:57:55

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

andrewboltachev 04:58:17

I mean would render be called

noonian 05:05: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

noonian 05:05:45

I have to take off

andrewboltachev 05:05:59

Thanks for great help @noonian !

andrewboltachev 05:06:08

I did try this autocomplete now

noonian 05:06:12

Np, take care!

andrewboltachev 05:06:17

it doesn't work on my versions of stuff

noonian 05:07:00

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

andrewboltachev 05:07:08

ok :simple_smile:

andrewboltachev 05:09:11

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

andrewboltachev 06:10: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

txus 11:38:34

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

txus 11:38:56

I could even reuse the same parser

nano 11:53:11

humm... well crap.. having an om app in figwheel mode crashes the Samsung Smart TV Tizen browser :disappointed: 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?

dottedmag 12:21:44

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

nano 12:22: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.

nano 12:23:35

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

dnolen 13:51:29

@nano does the SmartTV use WebKit?

dnolen 13:52:15

@txus nice!

nano 14:03: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

dnolen 14:04:30

yeah that might be it

dnolen 14:04:56

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

dnolen 14:05:11

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

nano 14:05:19

cool.. i'll try that out.

dnolen 14:05:24

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

nano 14:12:56

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

nano 14:13:01

@dnolen++

nano 14:29:59

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

dnolen 14:33:48

:simple_smile:

anmonteiro 14:35:28

wow that's great.

anmonteiro 14:35:49

Where are you on the reloading issue?

nano 14:41: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.

anmonteiro 14:42:17

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

anmonteiro 14:43:37

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

nano 14:44:47

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

anmonteiro 14:45:06

@nano: you can't have both :simple_smile:

nano 14:50: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.

dnolen 15:43:52

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

dnolen 15:44:11

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

dnolen 15:44:23

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

anmonteiro 15:54: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

dnolen 15:55:34

@anmonteiro: that’s not how React works

dnolen 15:55:45

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

dnolen 15:55:49

patching the prototype doesn’t matter

anmonteiro 15:55:56

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

dnolen 15:56:11

component local state isn’t stored on the class

dnolen 15:56:15

it’s stored on instances

anmonteiro 15:56:20

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

dnolen 15:56:22

and not even really instances

dnolen 15:56:30

React keeps an internal state map

anmonteiro 15:56:31

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

dnolen 15:56:38

and assigns component local state based on diffing

anmonteiro 15:56:55

cool to know that, I didn't

dnolen 15:57:16

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

anmonteiro 15:57:32

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

anmonteiro 15:57:40

if I'm able to find one

anmonteiro 16:33:44

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

anmonteiro 16:34:03

GH issue?

dnolen 16:34:03

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

dnolen 16:34:11

the constructor will be called every time

anmonteiro 16:34:23

oh that's not what I understood

dnolen 16:34:45

this is why the state map lives outside components

anmonteiro 16:34:49

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

dnolen 16:34:49

and is handled via diffing

dnolen 16:35:27

yes that would be the bug

anmonteiro 16:35:33

:simple_smile:

anmonteiro 16:36:08

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

anmonteiro 16:36:28

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

dnolen 16:36:33

@anmonteiro: PR welcome

nano 17:05: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.

dnolen 17:05:58

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

nano 17:06:12

Will do.

dnolen 17:06:12

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

dnolen 17:06:43

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

dnolen 17:07:24

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

dnolen 17:07: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.

nano 17:08:23

family play time, back in ~6h

jlongster 18:07:59

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

jlongster 18:15:42

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

jlongster 18:16: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)

jlongster 18:16: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

jannis 18:17:01

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

jlongster 18:17: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

anmonteiro 19:00: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

dnolen 19:02:21

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

dnolen 19:02:49

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

dnolen 19:03:19

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

dnolen 19:03:27

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

anmonteiro 19:03:43

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

dnolen 19:04:35

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

dnolen 19:04:44

the React integration bits generally are

anmonteiro 19:05: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

jlongster 19:05: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.

anmonteiro 19:05:49

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

dnolen 19:05:58

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

dnolen 19:06:18

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

anmonteiro 19:06:42

didn't know about that

dnolen 19:07:00

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

dnolen 19:07:03

instances could override

anmonteiro 19:07:08

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

dnolen 19:07:37

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

dnolen 19:07:56

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

dnolen 19:08:06

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

dnolen 19:08:11

@jlongster: cool!

anmonteiro 19:09:00

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

anmonteiro 19:09:32

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

dnolen 19:09:42

cool, thanks!

taylor.sando 19:47: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.

dnolen 20:28: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

dnolen 20:28:48

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

taylor.sando 20:31:13

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

dnolen 20:32:05

you can do it however you like

dnolen 20:32:15

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

sjol 21:19: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?

anmonteiro 21:49:39

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

anmonteiro 21:50:15

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

anmonteiro 21:51:31

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

anmonteiro 21:53: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

anmonteiro 21:54:05

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

anmonteiro 21:55: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

dnolen 22:11:02

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

anmonteiro 22:11:19

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

anmonteiro 22:11:26

all directly on the DOM

anmonteiro 22:11:42

so I've found the root cause now

anmonteiro 22:12:07

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

anmonteiro 22:12:20

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

dnolen 22:13:51

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

dnolen 22:13:59

that should be easy to handle in user code too though

anmonteiro 22:14:02

I'm not sure what to do given this information

anmonteiro 22:14:29

what do you mean handle that in user code?

dnolen 22:14:30

nothing wrt. to add-root!

dnolen 22:14:44

yes, for dev make sure you only call that once

anmonteiro 22:15:09

uhm, let's see how that pans out

dnolen 22:16:01

@anmonteiro: it’s easy to do

dnolen 22:16:09

just put the root component in a var initialized to nil

anmonteiro 22:16:10

just did

dnolen 22:16:18

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

noonian 22:16: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.

anmonteiro 22:16:45

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

dnolen 22:17:13

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

anmonteiro 22:17:22

OK but there's a problem

anmonteiro 22:17:28

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

anmonteiro 22:17:54

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

noonian 22:18: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

anmonteiro 22:19:12

@noonian: trying that now

anmonteiro 22:19:24

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

anmonteiro 22:19:31

depends on the use case, I suppose

anmonteiro 22:19:52

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

noonian 22:20: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.

anmonteiro 22:21:36

though it doesn't seem to be doing anything

dnolen 22:24:08

@anmonteiro: pretty sure figwheel has a hook for reloads

dnolen 22:24:22

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

anmonteiro 22:26:58

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

hugod 22:40: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

dnolen 22:42:25

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

anmonteiro 22:44: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

hugod 22:44: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.

anmonteiro 22:45: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

dnolen 22:45:49

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

dnolen 22:45:57

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

anmonteiro 22:47:41

@dnolen: yep

anmonteiro 22:48:14

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

anmonteiro 22:48:30

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

dnolen 22:50:23

@anmonteiro: I would test by examining the prototype

dnolen 22:50:42

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

dnolen 22:50:48

should be simple enough to do with Chrome DevTools

hugod 22:52: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.

anmonteiro 22:53:55

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

hugod 22:54:44

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

dnolen 22:56:12

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

dnolen 22:56:47

@anmonteiro: forceUpdate should work

anmonteiro 22:57:16

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

anmonteiro 22:57:32

I have a theory though, but might be wrong

dnolen 22:57:32

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

dnolen 22:57:34

that should be good enough

anmonteiro 22:57:54

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

dnolen 22:58:22

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

anmonteiro 22:58:24

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

dnolen 22:58:40

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

dnolen 22:58:53

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

dnolen 22:59:01

that might just be a bug on our end then

anmonteiro 22:59: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