Initialise state in constructor an refactor tests

[dagda1]

The state is initialised in the constructor now:

  constructor(props = {}) {
    super(props);

    let { Type, type, value } = this.props;

    Type = Type || type;

    // TODO: this can go if we only have type and PropTypes.func.isRequired
    if (!Type) {
      let name = this.displayName || this.name || Microstates.name;
      console.error(`${name} expects Type prop to be specified but none was received.`);
      return;
    }

    let microstate = create(Type, value);

    this.state = { next: microstate };
  }

I'd like to have one type props and change its proptypes to PropTypes.func.isRequired and remove the falsy check in the above.

I've also moved the subscription of the observable to componentDidMount

  componentDidMount() {
    let observable = Observable.from(this.state.next);

    this.subscription = observable.subscribe(this.onNext);
  }

You want to avoid doing anything that can cause side effects to after the initial render.

Ideally I would not create the Microstate in the constructor and do it in componentDidMount and instead initialise in the constructor with this.state = { next: value }; but I'm not sure what the ramifications of this are.

I've also removed the stateful variable used to assert in the tests.

[taras]

I'd like to have one type props and change its proptypes to PropTypes.func.isRequired and remove the falsy check in the above.

From what I understand, PropTypes only does warning. It won't prevent the component tree from attempting to render when Type is not specified. Not providing type is a developer error, the app should essentially stop rendering if Type is not present.

I've also moved the subscription of the observable to componentDidMount

I'm ok with this because the net difference is very minor but it's worth pointing out that doing setup in constructor is a very low cost operation because microstates are lazy. Very little work is done before the component is rendered and the render functions starts to pull state and transitions from the microstate. For this reason, it's fairly safe (IMO) to leave the subscription in the constructor. Although, I'm fine with this because technically it's correct.

Regarding un-subscription, I left it out previously because I don't think it's actually necessary because microstates are pure. They don't store anything or change anything outside of them. Once the component is garbage collected, the microstates and the subscription are garbage collected as well.

Unless React does some component instant pooling/reuse, I doubt there'll be any difference.

With all of that said, I'm good with these changes because they're technically correct on all accounts.

[dagda1]

@taras happy to make any changes or be challenged on anything and thanks for the feedback.

[dagda1]

it's fairly safe (IMO) to leave the subscription in the constructor.

This will cause an extra render cycle initiated from the constructor which you generally want to avoid. componentWillMount is discourage for this reason also and does not fire when server rendering.

There is talk of deprecating componentWillMount

[taras]

It's all good. We can merge and I'll release. Thank you for your first contribution :)