Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init-state does not work in mixins #28

Open
danielytics opened this issue Jul 27, 2014 · 3 comments
Open

init-state does not work in mixins #28

danielytics opened this issue Jul 27, 2014 · 3 comments

Comments

@danielytics
Copy link

I'm not sure if this is related to #25 or not and its very possible that I'm simply using this wrong (or that its not supported, although the docstrings say that it is). The following test code prints nil instead of "test" as I expect:

(defmixin test-mixin
  (init-state [_]
    (println "init-state")
    {:foo "test"}))

(defcomponentk test-component []
  (:mixins test-mixin)
  (render-state [_ {:keys [foo]}]
    (println foo)
    (dom/div foo)))

Note that "init-state" is printed, so it does get called - the state simply doesn't get merged into the child.


A failed workaround that I tried is to set the state in will-mount instead, but this doesn't work because if the component tries to access the state in its own will-mount, this state won't have been set (mixin lifecycle functions seem to get called after the components lifecycle functions).

The workaround I eventually went with is less than ideal:

(defmixin test-mixin
  (init [_ & [s]]
    (let [my-s {:foo "test"}]
      (if s
        (merge my-s s)
        my-s))))

(defcomponentk test-component []
  (:mixins test-mixin)
  (init-state [_]
    (.init owner {:bar "test2"}))
  (render-state [_ {:keys [foo]}]
    (println foo)
    (dom/div foo)))
@loganlinn
Copy link
Member

Thanks for reporting & the concrete examples. Yeah, this is related to #25 where init-state in a mixin is effectively broken -- the docs should be updated to reflect this. I have started looking into issue, but haven't had time to find a complete solution. In case you're interested in taking a stab, here's what I saw:

The first issue that needs to be resolved is that React expects getInitialState to be a JS object that can be merged into this.state. Currently, mixin's init-state returns a cljs hash-map, so it needs to be wrapped in a JS object similar to what's happening here. This object needs to have the hash-map from init-state at a key other than __om_state in order to not collide with default Om state. Key should probably be based off of mixin name.

The second, potentially trickier, issue is how to access/modify this state from mixins; the owner passed to mixins is currently the same owner that's given to the Om component, but needs to be changed to an object that implements Om's state protocols to not use __om_state, but whatever key is used from mixin's getInitialState. I mention that this is tricky because owner is still probably expected to be React component, so just a a reification of those state protocols probably wont do.

Hoping to get some time later this week/weekend to dig deeper on this problem, but any help would be appreciated!

@danielytics
Copy link
Author

Hmm, this gets even more complicated if you want to support both allowing mixins to add to the components local state (which is what I was trying to do) and to allow them to have their own mixin-local state (as mentioned in #25).

How is this handled in plain React?

@loganlinn
Copy link
Member

In plain React, AFAIK mixins can read/write the same state as component. They are just not allowed to return objects from getInitialState with keys that collide with any other initial state. As a consequence, it's not possible to merge with Om component's init state from mixin's getInitialState because it's nested under __om_state key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants