tonsky / rum

Simple, decomplected, isomorphic HTML UI library for Clojure and ClojureScript
Eclipse Public License 1.0
1.79k stars 126 forks source link

Support multiple context-types mixins #136

Open chpill opened 7 years ago

chpill commented 7 years ago

Hi,

Rum supports having multiple :child-context defined in mutliple mixins by collecting them and instructing react to merge them when needed which is great.

To be able set or access a key value pair in the react context, you must first respectively set the :childContextTypes or :contextTypes as :class-properties in rum mixins. The :class-properties of the mixins of a component are also collected, but they are then merged in a shallow way, that does not allow to aggregate the different :childContextTypes (or :contextTypes) maps.

For example, let's say that I have put into my react context the "a" and "b" objects. Now I want to use mixins to tell react I want to see those in the context of the child component my-component:

(defcc my-component 
  <  {:class-properties {:contextTypes {"a" js/React.PropTypes.object}}
     {:class-properties {:contextTypes {"b" js/React.PropTypes.object}}
 [react-component]
  (println (aget react-component "context"))
  => #js {:b my-object}
)

In this example, only "b" is reachable , because it was in the last mixin.

To provide a bit of context of how I encountered the issue, I am using the derivatives library mixins and I'm also writing some mixins to pass a scrum reconciler into the react context. Sadly, each time I want to use the 2 sets of mixins together on the same component, I have to make wrapper components and it is quite tedious...

Changing the way :class-properties are merged would be a breaking change (and maybe not a good idea...). We could add top level mixin keys :context-types and child-context-types that could aggregate those key/value pairs correctly?

chpill commented 7 years ago

I made a small PR to illustrate my suggestion https://github.com/tonsky/rum/pull/137/files

chpill commented 7 years ago

Any update on this? For reference, I run into this problem in https://github.com/chpill/re-frankenstein, where I have had to create some workaround mixins to put a the end of the mixin list https://github.com/chpill/re-frankenstein/blob/5666ed53e9ff3563d0738122c19a79c9c40ce1cf/src/re_frame/rum.cljs#L63-L68.