Clojurians
# om

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

dnolen 00:12:12

cut 1.0.0-alpha40 with the latest fixes / enhancements

takeoutweight 00:23:50

question: Is it considered in-spec for a mutate handler to transact! a new transaction when it's handling an existing transaction? Or should the transactions represent distinct & isolated user intents as much as possible?

anmonteiro 00:28:26

@takeoutweight: I wouldn’t recommend doing that

anmonteiro 00:29:03

you can always do things like: (om/transact! ‘[(some/action!) (other/action!)])

dnolen 00:30:11

yes, calling transact! from within a mutation is not recommended

takeoutweight 00:34:25

Makes sense. In our experiment the use case was having the second transaction conditionally depend on data computed when handling the first so issuing both transactions in the same transact! wasn't quite right. But it sounds like keeping the UI logic factored in a a way that all the cascading operations can be kicked off by a single transaction would be the way to go.

dnolen 00:39:30

@takeoutweight: for better or worse om.next is designed around a distaste for chain reactions

danburton 01:19:31

Is it normal to call om/set-query! from within a transaction, or would you recommend that this be called directly next to (rather than triggered by) om/transact! instead?

dnolen 01:25:09

@danburton: would not do that in a mutation, I would do it next to

dnolen 01:25:42

rule of thumb - don’t nest any top level mutation calls

hlolli 15:18:51

I dont get this one. I have two factories inside a div, both are instanciating a seperate component. If I give both a unique :keyfn they dont render because of children sharing same key. But if I only give one :keyfn, then it stops complaining.
So this does not work:
(def search-query (om/factory SearchQuery {:keyfn :search-query})) (def search-results (om/factory SearchResults {:keyfn :search-results})) But this will (and vice versa with search-results carrying a :keyfn)
```

(def search-query (om/factory SearchQuery {:keyfn :search-query})) (def search-results (om/factory SearchResults)) ```

cjmurphy 15:35:56

What I've noticed is that if React is not given a function to create the key then $null is put on the end of the key that it makes up. $null will be different to say what :search-query returns.

cjmurphy 15:37:55

@hlolli ^^

dominicm 15:38:43

https://github.com/omcljs/om/issues/673 is that related to this issue then @cjmurphy ?

dominicm 15:39:51

Looking at the om.next api, it appears that factory and build are equivalent.

dnolen 15:41:49

@dominicm: they are different actually

cjmurphy 15:41:56

I'm going on my observations when using devtools, that's all.

dnolen 15:42:01

build makes instances factory makes a factory fn

dominicm 15:43:49

@dnolen: Makes sense why the example in that github issue is different to my own using the current om.

The fn returned by factory is like having build in an anonymous function?

dnolen 15:44:39

@dominicm: almost, factory returns a constructor fn which has the same shape as normal React DOM constructor fns

dnolen 15:44:48

build signature is very different

dnolen 15:46:11

@hlolli: sounds like it's possible both of the keyfns might return nil so then you would have the same key?

dominicm 15:46:16

ah, that makes sense. Close enough in utility. Makes sense that issue is reported in both versions.

dnolen 15:47:13

yeah I need to look more closely at 673 - we used undefined specifically to get React to ignore but maybe something changed in React internally that makes that not work anymore

hlolli 15:49:00

sorry afk

dominicm 15:49:30

@dnolen: My understanding based on the log messages is that React is now doing (= undefined undefined) and collapsing the children as being the same.

I wonder if this means that React is using something like hasOwnKey instead of (not= component[key] undefined)


I just realised that I just made a horrible mess of blending js and cljs to make pseudo code. Not sure if that's good or bad.

hlolli 15:50:17

yes, both return nil, the same key in this case is nil.

dnolen 15:51:38

@hlolli: k right, so yes probably related to 673, will try to look at that later in the afternoon

hlolli 15:52:58

ok, just to flood, then this is the error message. @dnolen cool!
Encountered two children with the same key, `null`. Child keys must be unique; when two children share a key, only the first child will be used.

dnolen 15:53:22

@hlolli: ah ok that’s a different problem, not related to 673 then

dnolen 15:54:25

@hlolli: so the question is what is the expected behavior if we provide :keyfn

dnolen 15:54:35

currently we assume you will produce something

dnolen 15:54:44

and that nil is a valid key value

hlolli 15:56:06

yes, it would save sanity to allow nil to be a valid key. I guess react only complains about siblings being duplicated.

dnolen 15:56:24

so nil is a valid key right now

dnolen 15:56:31

but two components want to use it

hlolli 15:56:44

but then the question is, why keyfn doesnt associate the assigned key. Yes, which is strange.

hlolli 15:57:47

if both are siblings lack :keyfn or both have one, it fails, if one is different than other, it works.

dnolen 15:57:47

@hlolli: it does

dnolen 15:58:12

@hlolli: because in one case it the key is nil in the other case the key is undefined which React ignores

dnolen 15:59:03

so if they both have it they both get assigned nil

dnolen 15:59:16

if they both don’t have it they both get assigned undefined

dnolen 15:59:24

the later case is 673

hlolli 16:00:49

ok, so I understand it right that :keyfn :any-value here = :key nil

cjmurphy 16:06:40

@hlolli: look at elements in chrome developer console. Every element has a data-reactid. You will see how they are made up with your examples. I know that's not a theoretical answer, but doing that always gets me out of trouble and it seems simple enough to make sure all sibling elements have a different data-reactid. Always proving a function seems like a good idea as well. I know not completely answering your question :slightly_smiling_face:

hlolli 16:10:28

@cjmurphy Thanks, good to know. I always make sure that when creating elements with cljs functions to give :key wherever possible. That way at least I lose all warning, which is nice.

cjmurphy 16:13:20

Yeah, directly :key for dom elements and :keyfn for om next components. I think you will be able to see them all in the developer console, even the ones not being displayed because they are duplicates. Not 100% on that thou.

anmonteiro 16:33:49

@dnolen: @dominicm: #673 is present in React 15 but not in React 14. Previously setting key to undefined would count as an absent key, but in React 15 keys are explicitly set to undefined

anmonteiro 16:34:02

this is what I’ve gathered from looking at the issue a few weeks ago. A few things might have changed meanwhile

dnolen 16:35:22

@anmonteiro: ok good to know, well in anycase just conditionally adding key should cover us, simple enough to do

anmonteiro 16:35:33

Yeah, I think that would be the fix

anmonteiro 16:35:50

@dnolen: meanwhile, I’ll try upgrading React to 15.2 to see if this has been eventually fixed/changed upstream

dnolen 16:36:13

@anmonteiro: cool, yeah would be interested in hearing about that