account for child context and default props

[dtothefp]

1. the provideReactor decorator allows passing additional options for child context but the nuclearComponent decorator does not expose this context other than for the reactor.
2. defaultProps are not exposed if iterating through this.props.children if the children use the use es6 React Component Class syntax contetextTypes and defaultProps are exposed as static methods and can be accessed in the nuclearComponent

Use case 1:

• internal module has components as well as it's own nuclear stores and actions. These actions need the reactor in order to function. It is nice to have the Actions that have the reactor exposed to them passed down as context rather than having to pass the reactor to the actions at each use in the component.
• below will currently not work because on line: 35 or nuclearComonent the context exposed to children is hardcoded to

contextTypes: {
      reactor: React.PropTypes.object.isRequired,
},

• example

//my-module/Actions.js

export default function(reactor) {

  const action1 = function (initialValues) {
    reactor.dispatch('ACTION_1', {initialValues});
  };

  const action2 = function (data) {
    reactor.dispatch('ACTION_2', { data });
  };

  return {action1, action2};
}

//my-module/Input
@nuclearComponent({
  hasError: Getters.formErrors //these come from local module
})
export default class Input extends React.Component {
  static contextTypes = {
    Actions: React.PropTypes.object.isRequired
  };
  handleChange(e) {
    this.context.Actions.action1(e.target.value); //from parent context
  }
  render() {
    let classes = this.props.hasError ? 'errorr' : 'dope';

    <input className={classes} onChange={::this.handleChange} />
  }
}

//my-module/Form
@provideReactor({
  Actions. React.PropTypes.object.isRequired
})
@nuclearComponent({
  hasError: Getters.formErrors //these come from local module
})
export default class Form extends React.Component {
    static contextTypes = {
      Actions: React.PropTypes.object.isRequired
    };

   handleSubmit(e) {
     this.context.Actions.action2(/*data from form*/);  //from parent context
   }
   render() {
     let classes = this.props.hasError ? 'form-error' : 'dope-form';

     return (
          <form className={classes} onSubmit={::this.handleSubmit}>
             <Input type="text" name="dope" />
          </form>
     );
   }
}

// bootstrap
import makeModuleActions from 'my-module/Actions';
import Form from 'my-module/Form';
import reactor '../from/some/local/reactor/path';
import {formErrorStore as formErrors}  from 'my-module/Stores';

reactor.register({
  formErrors
});

React.render(<Form reactor={reactor} Actions={makeModuleActions(reactor)} />, document.body);

Use case 2:

• what if I want to do something dynamic with a components this.props.children and I need to have access to defaultProps on the children. In the current state of nuclearComponent if a child uses the nuclearComponent decorator those default props are obfuscated before the child component class is instantiated.
• below will currently be problematic because the nuclearDecorator does not account for defaultProps and if I want to do something like gather initial form data such as type when iterating through children, and submit data to a Store during an initial lifecycle method, components created dynamically in React.Children.map/React.cloneElement loop will have initially undefined values for type in this case.

//my-module/Input
@nuclearComponent({
  hasError: Getters.formErrors //these come from local module
})
export default class CheckBox extends React.Component {
  static defaultProps = {
    type: 'checkbox'
  };
  static contextTypes = {
    Actions: React.PropTypes.object.isRequired
  };
  handleChange(e) {
    this.context.Actions.action1(e.target.value); //from parent context
  }
  render() {
    let classes = this.props.hasError ? 'errorr' : 'dope';

    <input className={classes} onChange={::this.handleChange} />
  }
}

//my-module/Form
@nuclearComponent({
  hasError: Getters.formErrors //these come from local module
})
export default class Form extends React.Component {
    static contextTypes = {
      Actions: React.PropTypes.object.isRequired
    };

   constructor(props) {
     this.initalData = [];
   }

   handleSubmit(e) {
     this.context.Actions.action2(/*data from form*/);  //from parent context
   }
   render() {
     let classes = this.props.hasError ? 'form-error' : 'dope-form';

     return (
          <form className={classes} onSubmit={::this.handleSubmit}>
             {React.Children.map(this.props.children, (child, i) => {
                //if child component uses @nuclearComponent any default props not put directly on the 
                //jsx element will not be available
                this.initialData.push(child.props); //type will not be in this object
                React.cloneElement(child, {someNewProp: 'something'}); //child.props.type will be undefined
             }, this)}
          </form>
     );
   }
}

//external-application/form-with-checkbox
@provideReactor({
  Actions. React.PropTypes.object.isRequired
})

export default FormWithCheckbox extends React.Component {
  render() {
    return (
      <Form>
        <Input name="i_have_no_type_specified_but_default_to_checkbox" />
      </Form>
    );
  }
}

//bootstrap
import makeModuleActions from 'my-module/Actions';
import Form from 'my-module/FormWithCheckbox';
import reactor '../from/some/local/reactor/path';
import {formErrorStore as formErrors}  from 'my-module/Stores';

reactor.register({
  formErrors
});

React.render(<FormWithCheckbox reactor={reactor} Actions={makeModuleActions(reactor)} />, document.body);

[Sinewyk]

There's no mistake in everything you just pointed out. It works exactly as it should.

NuclearComponent comes with the fact that you as the user (a user) acknowledge that you are using the Higher order Component pattern and you should know what you are doing. This pattern breaks refs because you don't actually have access to the "inner" component, you have a reference to the wrapper.

When you said that child.props.type is undefined ... it's normal, because child is not your Input, it's your wrapped Input.

If you want to precisely see what happens, I strongly suggest using using the react developer extension with chrome https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi and check your render tree.

You will see that inside your form you don't directly have the input, you will have the NuclearComponent(Input) and then your Input. If you do not send a type to your wrapped component, then your inner component should correctly have your type defaulting to "checkbox".

And for the first point, I'm not sure if we want people to start using the nuclearComponent to have access to tons of stuff. It feels bad to me. The provideReactor is a simple helper that anyone can create/swap with anything they want. If you use it to inject specific context, you should handle it yourself and not tweak around the nuclearComponent whose sole responsability should be listening to the reactor and setting state in response.

Unless I made a mistake I won't merge this one either ^^.

I'm not stopping you from using your createActions and passing it as a props, but you don't need to tweak this module to make it happen right now.

edit: "Checkbox" to "Input" 3rd paragraph edit: "but" last line

[dtothefp]

@Sinewyk I have deeply inspected how your code works, and how it relates/is visualized in the react render tree. If I hadn't inspected your source code I wouldn't have know that the provideReactor decorator accepts arguments. If this is not intended you should potentially throw an error if the user passes arguments to the decorator

//potentially throw error here
// support decorator pattern
  if (arguments.length === 0 || typeof arguments[0] !== 'function') {
    additionalContextTypes = arguments[0]
    return function connectToReactorDecorator(ComponentToDecorate) {
      return createComponent(ComponentToDecorate, additionalContextTypes)
    }
  }

and this code is pointless if you intend to now pass any context other than the reactor

var childContextTypes = objectAssign({
    reactor: React.PropTypes.object.isRequired,
  }, additionalContextTypes || {})

That said, I don't agree with what you are saying about context because you are essentially breaking the intended passing of context/dependency injection that React provides, if the user implements the nuclearComponent. If I have a parent passing a context to a child and both components implement nuclearComponent it seems not correct to break this functionality, which is what happens in the current implementation. If you can show me a way to pass contexts between components using the nuclearComponent decorator that would be great, otherwise you should document that you are breaking expected behavior.

[Sinewyk]

You can pass additional context via provideReactor. But it's not meant to be used directly by the nuclear component/mixin, roll your own solution to consume your additionalContextTypes. Mixin, decorator, whatever, you can do it ^^.

I do not fiddle with the inner component contextTypes in the nuclearComponent helper because that's what the wrapper is for. It takes the reactor from the context and send it as a prop. If you inject something else in the context it's not the nuclearComponent wrapper responsibility to have access to it. It only cares about the reactor in the context.

Either you do "pure" stuff, with higher order component and your wrapped component ONLY receive props. Which is what NuclearComponent does. It takes the reactor from the context, and the data from the state and inject it as props of the inner component.

Or you use the mixin and you have direct access to state and context.

Use the mixin if you want direct access (state/context), use the component if you want to receive props (and only props, no access to the context, but you received reactor as a prop).

NB: when initially creating this I was doing what you suggested. Fiddling with the Component.contextTypes in the nuclearComponent. It was a total mess. You still received the wrapped instead of the inner component, you didn't know anymore which was which.

[Sinewyk]

@dtothefp Don't hesitate to come at https://gitter.im/optimizely/nuclear-js if you want to directly talk to some people :smile:

[dtothefp]

@Sinewyk thanks for the chat link. I worked previously at Optimizely with @jordangarcia so I ask him questions from time to times.

Your answers above about mixin's are not applicable. Why would I even bother with all these questions about decorators or implementing them if I was using es5 style React.createClass. I'm asking these questions because I'm using React 13 with es6 Class syntax, therefore the Mixin is not applicable.

I think as more people start to adopt React 13 and es6 Classes Nuclear and your addons will face more questions as I'm asking. I would suggest either clearing up the docs on your decorators and consider actually writing them in es6 with React 13 Class syntax

I suggest this partially because not only does your component break the context chain but it does not allow for doing cool stuff that extending classes allows.

//my-module/Input
@nuclearComponent({
  hasError: Getters.formErrors //these come from local module
})
export default class Input extends React.Component {
  static contextTypes = {
    Actions: React.PropTypes.object.isRequired
  };
  static defaultProps = {
    type: 'text'
  };
  handleChange(e) {
    this.context.Actions.action1(e.target.value); //from parent context
  }
  render() {
    let classes = this.props.hasError ? 'errorr' : 'dope';

    <input className={classes} onChange={::this.handleChange} type={this.props.type} />
  }
}

//my-module/Checkbox
export default class Checkbox extends Input {
  static defaultProps = {
    type: 'checkbox'
  };
  handleChange(e) {
   //do extra stuff to check the `checked` attribute, etc
   //action context potentially inherited from the Input class, not sure if this works ?
    this.context.Actions.action2(e.target.value); //from parent context
  }
}

//make the component
<form>
  <Input type="textarea" />
  <Input /> //defaults to type of text
  <Checkbox /> //doesn't need a type, it knows it
</form>

[dtothefp]

@Sinewyk PS I don't understand what this means...can you elaborate?

"You can pass additional context via provideReactor. But it's not meant to be used directly by the nuclear component/mixin, roll your own solution to consume your additionalContextTypes. Mixin, decorator, whatever, you can do it ^^."

[Sinewyk]

Nothing stops you from mixing both styles when you want to except you. Using one style over the over (createClass or React.Component) is a moot point. createClass and mixins are not going away for a long time http://reactjsnews.com/composing-components/

I strongly advise against using classes in javascript, I'm not going to talk about that here https://medium.com/javascript-scene/the-two-pillars-of-javascript-ee6f3281e7f3. So I'm skipping so subclassing part ... just don't do it please.

provideReactor is simply an helper to attach stuff to react context. nuclearComponent is a Higher Order Component reading the reactor attached to the context and whose sole responsability is passing the nuclear data + the reactor as props to the wrapped component.

I do not break the context chain, you just don't have direct access to it and wanting to give you direct access to it creates a mess. If you want to guard yourself you should probably use something like this.context.reactor || this.props.reactor in critical paths where you do not know if you are using createClass or es6 classes if that's what you are worried about.

A simple example maybe

// #1 ?
function accessI18n(Component) {
  Component.prototype.getI18n = function () {
    return this.context.i18n;
  }
};

// #2 ?
function accessI18n(Component) {
  return class extends Component {
    getI18n() {
      return this.context.i18n;
    }
  }
}

@nuclearComponent({ /* dataBindings stuff */})
@accessI18n
class Child extends React.Component {
  render() {
    return <div>i18n: {this.getI18n()}</div>;
  }
}

@provideReactor({i18n: React.PropTypes.string.isRequired})
class App extends React.Component {
  render() {
    return <Child/>;
  }
}

React.render(<App reactor={reactor} i18n="en_GB"/>, /* dom */);

... that said, I'm going to think about trying passing a series of keys to pass along as prop from the context with the nuclearComponent so that we can simply just use provideReactor and nuclearComponent alone and avoid additionals wrappers simply to read something from the context.

Which would mean that the previous example would simply be

@nuclearComponent(
  {/* dataBindings stuff */},
  /* passing an array of keys to read from context and pass as props*/ ['i18n']
)
class Child extends React.Component {
  render() {
    return <div>i18n: {this.props.i18n}</div>;
  }
}

@provideReactor({i18n: React.PropTypes.string.isRequired})
class App extends React.Component {
  render() {
    // nuclearComponent read i18n from context, passed it as props to wrapped component
    return <div>i18n: {this.props.i18n}</div>;
  }
}

React.render(<App reactor={reactor} i18n="en_GB"/>, /* dom */);

Would that be better ?

[dtothefp]

@Sinewyk yes I am not a huge fan either of abstracting away prototypal inheritance in favor of a class based syntax and it's scary to think there will be a whole class of developers that actually don't fully understand what is going on under the hood of a es6 Class .....hint hint CoffeeScript devs :neckbeard:

I chose classes for React because I'm working within a sizable team of differing abilities, and I'd like to not have people context switching between React.createClass and React.component I also wanted to protect developing a legacy codebase from the beginning i.e. if React uses Class syntax from here on out, which from you link is ambiguous.

Anyhow, thanks for your detailed descriptions above, they are extremely useful as I generally have a hard time understanding some of your descriptions. It does make sense to separate concerns of the decorators, although hardcoding the reactor context in the nuclearComponent does seems a little bit scary to me.

You last example is more along the lines of what I was thinking, having a designated key for arguments that are not data bindings, or as you showed a type such as an Array designated for binding various context. I think this is a more friendly api and would remove the hardcoding of the reactor context in the nuclearComponent.

I'm curious if this is something you consider implementing anytime soon?

[dtothefp]

@Sinewyk also I'm curious if there is some way to use extends without your decorators using React.Component?

[Sinewyk]

I don't know why you find it scary that I'm hardcoding the context when everyone is doing it, example for react-router => https://github.com/rackt/react-router/blob/master/modules/State.js.

It's just the way people do things I guess.

And I didn't understand what you meant by

is some way to use extends without your decorators using React.Component?

[dtothefp]

Hmmm...from what I see on Mixins in react https://facebook.github.io/react/docs/reusable-components.html#mixins, which is what this example is of https://github.com/rackt/react-router/blob/master/modules/State.js. "A nice feature of mixins is that if a component is using multiple mixins and several mixins define the same lifecycle method (i.e. several mixins want to do some cleanup when the component is destroyed), all of the lifecycle methods are guaranteed to be called."

In your case if I'm understanding correctly with nuclearComponent the context chain is broken https://github.com/jordangarcia/nuclear-js-react-addons/blob/master/nuclearComponent.js#L34 unless an additional decorator is added, both of which add additional "Higher order Component" as you stated earlier. I've not looked into React source code but I'm assuming a Mixin does not create a higher order component, it seems to merge desired functionality into component lifecycle and execute it in order of Mixin position in the Array.

It seems addition of Higher Order Components is quite different than execution of Mixins and has more tendency to block intended functionality of React, in this case context. That is why I'm calling it scary because if users implement your decorator their expected context functionality is blocked. I understand your argument for separation of concerns, maybe more documentation is the only solution.

As for the second question about extension of Classes I am working on a demo that will make my question more explicit.

I found this gist and subsequent conversation helpful for both matters even though the actual code is not implemented correctly

https://gist.github.com/sebmarkbage/ef0bf1f338a7182b6775

[Sinewyk]

the context chain is broken

I just don't understand what you mean when you put that here.

You want access to the reactor in es6 ? Use nuclearComponent and you have it as a prop. You don't want to use the component ? just attach

contextTypes: {
  reactor: PropTypes.object.isRequired,
}

and there you have it (if you used provideReactor or something similar).

I break nothing. There's no "chain".

If I have this render tree

App
  ChildA
    ChildB
      ChildC

And App provide a context X, ChildA and ChildC both declare their contextTypes such that they have access to X, even if ChildB does not declare it, ChildC will have access. There's no chaining necessary for the context to be propagated through the tree if you declare that you are aware of it and wants it.

[dtothefp]

@Sinewyk haaaa...nevermind. I just realized my stupid mistake. For some reason I was thinking

contextTypes: {
  reactor: PropTypes.object.isRequired,
}

was setting a context, but it was getting the context from the provideReactor decorator higher order component. Duh, confusing

[Sinewyk]

Nah, it does not set anything. It only declares what it should be (context) aware of.

Few, glad we cleared that up. I thought I did something completely wrong when I had no idea what you were talking about :relaxed:

[dtothefp]

@Sinewyk thanks for the help on this :smile: