arunoda / react-komposer

Feed data into React components by composing containers.
MIT License
733 stars 70 forks source link

Add displayName to composers #140

Open zvictor opened 7 years ago

zvictor commented 7 years ago

Problem

I can't define the display-name of the middle container in some cases, having to stick with a MyContainer -> Container(MyComponent) -> MyComponent pattern.

screen shot 2016-12-02 at 14 06 29

Solution

Inspired by react-komposer-plus, the goal is to be able to define the display-name in the composers, as seen here. The pattern would then become MyContainer -> withRedux(MyComponent) -> MyComponent.

arunoda commented 7 years ago

And I think this even possible:

Container = compose(...)
Container.displayName = 'something';

Are we trying to something else here?

zvictor commented 7 years ago

It works well for components and containers, not for composers.

The following works:

Component = ...
Container.displayName = 'something';

Container = compose(myFn)(Component)
Container.displayName = 'something';

The following doesn't:

Component = ...

WithRedux = compose(myFn)
WithRedux.displayName = 'something'; // useless

WithTracker = compose(myFn)
WithTracker.displayName = 'something'; // useless

Composer = merge(
  WithRedux,
  WithTracker
)
Composer.displayName = 'something'; // useless

Container = Composer(Component)

When using merge an extra container is created with the name forced to be Container(...)

screen shot 2016-12-02 at 14 06 29
arunoda commented 7 years ago

But you could do this:

const Container = merge(
  WithRedux,
  WithTracker
)(Component);

Container.displayName = 'something';
zvictor commented 7 years ago

Yes, I could. As I said, it works well for components and containers, not for composers. This PR is about composers, not containers.

In your example there will be a something -> Container(InnerComponent) -> Component structure. I am not worried about the something occurrence, but the Container(InnerComponent).

arunoda commented 7 years ago

Okay. I am good to take this. Could you add this feature to the README as well? So, I could merge and this and do a release.