This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2016-09-02
Channels
- # beginners (61)
- # boot (84)
- # cider (43)
- # cljsrn (2)
- # clojure (99)
- # clojure-android (3)
- # clojure-austin (2)
- # clojure-italy (5)
- # clojure-russia (43)
- # clojure-spec (93)
- # clojure-uk (41)
- # clojurescript (94)
- # clojutre (1)
- # cloverage (8)
- # core-async (31)
- # cursive (3)
- # datomic (14)
- # defnpodcast (1)
- # editors-rus (7)
- # events (1)
- # hoplon (15)
- # leiningen (3)
- # luminus (6)
- # om (142)
- # onyx (86)
- # other-languages (4)
- # pedestal (1)
- # planck (1)
- # portland-or (5)
- # re-frame (96)
- # reagent (16)
- # ring-swagger (17)
- # rum (73)
- # specter (25)
- # untangled (14)
- # yada (142)
going to do one more alpha, if I don’t hear any bad news will work on beta next week as well as updating all the GitHub content to point to new stuff instead old stuff
@dnolen hrm I’m not sure about your transform-reads
fix
it seems to me that it causes unnecessary reads
@anmonteiro the original idea was just wrong
whenever it matches a key in the indexer, a tx like [(do/it!) :foo]
becomes [(do/it!) :foo {:foo [:transformed]}]
@dnolen right, I’m happy to provide the optimization patch if you agree that the example above should result in:
[(do/it!) {:foo [:transformed]}]
I’m worried it might break some code that uses that assumption
e.g. parsers expecting a query and getting nil
(as a result of :foo
)
previously transform-reads
would transform [(do/it!) :foo]
to [(do/it!) {:foo [:transformed]}]
. This implies that the parser for :foo
will get [:transformed]
as its query
currently it will get both :foo
(where query in the env is nil
) and {:foo [:transformed]}
, with the right query
well that was the purpose of transform-reads
for me I think
that we’d actually get the right query in the parser
@dnolen then again, this problem will be solved with the optimization I mentioned above
so probably not a reason for concern
it just doesn’t make sense to me that specifying :foo
to re-read after a transaction doesn’t get transformed into the actual query for :foo
only if you bake in a lot of assumptions about how someone might construct a transact value
@dnolen I can definitely agree to that. I think what I’m suggesting is orthogonal to that concern
I think it can be summarized as: if we find :foo
in the indexer, use its query. If no indexer is available or we can’t find it, use the key that we have at hand
but don’t put both :foo
and its transformed query in the result
I’m just concerned because this little change influences every transaction that specifies keys to re-read
now I’m not following
unless A) you made some assumption about that key being associated with some query everywhere B) you have side effects during reads (yuck)
C) you return a large data structure that path-meta
needs to walk more than one time now 🙂
@dnolen ^ this is the sole reason for my concern about reading more than once
@hueyp already in alpha42 😛
@dnolen I’ll try to put something together tonight
@anmonteiro cool thanks
@dnolen I had one more optimization idea for the parser. I’m pretty sure we don’t need to call path-meta
when doing recursive parsing
so the parser we feed to env
should ideally have elide-paths
true
I think we really just need to call path-meta
on what’s returned at the top-level
since other metadata stuff will be overwritten anyways
@hueyp nope, it’s what I’m proposing
hey all. What's wrong with this component?
(defn button [props & children]
(reify
om/IRender
(render [_]
(dom/button #js
{:className "my-button"}
children))))
also how would I merge that #js {:className "my-button"}
object into the incoming props
?
lazy maps of react components should be fine but I think your wanting to (apply str children)
so (button nil (map #(dom/p nil %) ["+" "-"]))
would work (maybe, not sure if button
can contain p
), but a list of strings will cause issues i think
yeah let me explain better, (defn button [props & children] (list props children))
would return (nil ("+"))
this is the output I'm getting:
<span data-reactid=".0.0.1:$text:0">+</span><span data-reactid=".0.0.1:$cljs$lang$protocol_mask$partition0$:0">393216</span><span data-reactid=".0.0.1:$cljs$lang$protocol_mask$partition1$:0">0</span>
@nickt even if you’re using om.core
, you can use the latest alphas
you get React updates if nothing else
ok so a om.now component has two fields
(fn [data owner]
(reify
om/IRender
(render [_]
(dom/p nil (:text data)))))
heh, trying to get back in the om.now mindset.. the first field is usually an om cursor, which is a extended clojure map or vector, but you can provide om/build
with whatever you want for it
you can also provide data in https://github.com/omcljs/om/wiki/Documentation#build via :state :init-state and :opts
(defn button [data owner]
(reify
om/IRender
(render [_]
(dom/button #js
{:className "my-button"}
(om/get-state owner :text)))))
;usage
(om/build button {} {:init-state {:text "+"}})
like, in react itself, I could write a Button
component with exactly the same API as <button>
class Button extends Component {
render() {
return <button {...this.props}>{this.props.children}</button>;
}
}
@dnolen question regarding the transform-reads
patch:
(transform-reads r [:foo :bar])
where we can find a query [{:foo [:bar]}]
should return which?
1. [{:foo [:bar]}]
2. [{:foo [:bar]} :bar]
@anmonteiro I’m inclined to optimize left to right
Before your fix it would return [{:foo [:bar]} {:foo [:bar]}]
now it returns [:foo {:foo [:bar]} :bar {:foo [:bar]}]
I’m inclined to say 1.
awesome
@nickt well om.now is a layer of abstraction over the react props, so the idea is you pass data via the reified fn's first arg or in the third arg with :opts :state or :init-state
, same concept as react's props
i.e. the button gets an onCclick prop, but I also want to supply my "my-button" class naemn
(om/build button {:js-style {:background "red"}})
and in button (dom/button #js {:style (clj->js (merge (:js-style data) {:color "white})))
@anmonteiro cool thanks!
@dnolen actually hold off on merging, adding one more test
the user might want to specify the same mutation twice, and I think I’m being too restrictive by using a set
and it’s probably incorrect anyway as the set doesn’t guarantee order
so reads might happen before the mutation
@anmonteiro yeah have to keep the order
@dnolen k updated the PR
@dnolen you mentioned a while ago wanting to merge Cellophane into Om
which release would that be targeting, since you’re now thinking about Beta
@anmonteiro yes would be nice for that to get in before whatever the “final” release is
@dnolen OK I can probably start putting together a branch that includes that migration
and we’d schedule it to be merged after you cut beta1 or something
such that it’s still not a part of that release
@dnolen there’s one more thing. Would you be against having a build.boot
file in Om? It could either coexist with project.clj
or even be a total migration
I’m just asking because migrating to Boot in some of my projects has been a total productivity booster
including automatically running tests on file save, etc
awesome. which option?
coexisting with project.clj or just totally migrate?
it’ll definitely make it easier to merge Cellophane in