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

Support multiple context-types mixins #136

Open
chpill opened this issue May 8, 2017 · 2 comments
Open

Support multiple context-types mixins #136

chpill opened this issue May 8, 2017 · 2 comments

Comments

@chpill
Copy link

chpill commented May 8, 2017

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
Copy link
Author

chpill commented May 9, 2017

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

@chpill
Copy link
Author

chpill commented May 22, 2017

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.

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

1 participant