decorator around mixins

[ksmth]

I saw the deprecation warnings for using mixins are reverted with 0.13.3, so the official story is now to use mixins, always. I think it'd be nice to also support ES6 classes and not force everyone to use mixins in their code if they'd prefer not to.

I came up with a first draft on how it could be implemented by reusing the mixin internally and exposing a decorator function.

https://jsfiddle.net/ksmth/t34ak1q3/10/

// ES7
@routerState
class MyComopnent extends React.Component {
    // ...
}

// ES6
class MyComopnent extends React.Component {
    // ...
}
MyComopnent = routerState(MyComopnent);

// React.createClass without mixins
var MyComopnent = React.createClass({
    // ...
});
MyComopnent = routerState(MyComopnent);

[mjackson]

@smth I think you're on the right track here. Care to submit a PR that builds on top of the next branch?

[ksmth]

Yep, gladly. A few questions though, as this would be my first contribution. The contribution guidelines clearly state that no code should be merged without providing tests. I haven't found any tests for Navigation though. I assume this is, because they just delegate all calls to the underlying router instance. All that's left would be tests, that verify that wrapped components receive the props and route lifecycle / transition methods are still called.

Further, how should the mixin methods be passed down? My fiddle outlines two approaches:

// Individual props
<Component {...this.props} {...stateApi} />

// Single prop
<Component {...this.props} routerState={stateApi} />

I think I'd favor the first approach, as you could be more explicit in your component's propTypes, expressing your expectations.

Are there any more methods that should be delegated to the wrapped component other than routerWillEnter / routerWillLeave?

[SevereOverfl0w]

I would love to see this, I'd miss my es6 support too much :(

[BerkeleyTrue]

👍 currently using router on context to get around this, but this seems to be a no-no until parent-based context comes in 0.14.

[SevereOverfl0w]

https://jsfiddle.net/t34ak1q3/16/

This runs it as an actual decorator, incase anybody was interested.

[agundermann]

Thanks for sharing 👍

I wonder if it wouldn't be preferable to use class inheritance though, in order to avoid the extra wrapper component which breaks refs and animations.

Something like

var stateDecorator = (Component) => {
  class Derived extends Component {}

  Object.assign(Derived.prototype, StateMixin); 
  return Derived;
}

[juhaelee]

I think @taurose makes a good point about the breaking refs and animations. HoC might solve one problem, but it could cause more problems as well. Is the best way to access the router methods for an ES6 class is through the contextType class property?

Component.contextTypes = {
  router: PropTypes.func.isRequired
};

Then to access router methods, this.context.router.[method]

[lixiaoyan]

+1

[appleboy]

+1

[ryanflorence]

Michael and I have been talking about what to do about our mixins, we'd like to not have them and use higher order components like relay/redux but that would break support for a lot of the current react environment (like TransitionGroup, it needs the children to be app controlled)

After we release 1.0 we'll experiment with an HoC (createRouteComponent or something) until we work out any issues like TransitionGroup, and then only have HoCs at 2.0 (or something like that)

[0xR]

If you need a HoC today, you can make one yourself like so: https://github.com/este/este/blob/master/src/client/components/exposerouter.js

[gaearon]

I wonder if it wouldn't be preferable to use class inheritance though, in order to avoid the extra wrapper component which breaks refs and animations.

I would advise against doing this. Inheritance means you must be super careful: things will almost work but sometimes horribly break (e.g. adding contextTypes or lifecycle hook to base class will blow up consumer code in unknown ways). When every other library uses inheritance for this, there are bad incompatibilities. At least HOCs make things clear: you're getting a different component, and there'll be indirection, but no surprises varying from one lib to another.

[acdlite]

HOCs also have the advantage of working with stateless function components, too.

[taion]

I'm going to drop this from the "revisit" tag - for most use cases, context is okay now that it's actually documented.

I'm not completely happy with any of the non-mixin-like solutions we have for the Lifecycle mixin right now, but we can re-open that discussion once we actually have a good idea for how to deal with it.