Use explicit constructor in React components

[Andarist]

This boils down to a fact that super has to be called with all arguments if the constructor is not specified (when we use explicit constructor, we explicitly call super(props)), so arguments have to be looped and super constructor has to be applied with them.

Implicit constructor

var List =
/*#__PURE__*/
function (_PureComponent) {
  _inheritsLoose(List, _PureComponent);

  function List() {
    var _this;

    for (var _len = arguments.length, args = new Array(_len), _key = 0; _key < _len; _key++) {
      args[_key] = arguments[_key];
    }

    _this = _PureComponent.call.apply(_PureComponent, [this].concat(args)) || this;
    _this._instanceProps = initInstanceProps(_this.props, _assertThisInitialized(_this));
    return _this;
  }

  return List;
}(PureComponent);

Explicit one

var List2 =
/*#__PURE__*/
function (_PureComponent2) {
  _inheritsLoose(List2, _PureComponent2);

  function List2(props) {
    var _this2;

    _this2 = _PureComponent2.call(this, props) || this;
    _this2._instanceProps = initInstanceProps(props, _assertThisInitialized(_this2));
    return _this2;
  }

  return List2;
}(PureComponent);

[bvaughn]

Was this a necessary part of the change as well?

I like the property initializer syntax.

[Andarist]

output with "empty" constructor (just super call)

var List3 =
/*#__PURE__*/
function (_PureComponent3) {
  _inheritsLoose(List3, _PureComponent3);

  function List3(props) {
    var _this3;

    _this3 = _PureComponent3.call(this, props) || this;
    _this3._instanceProps = initInstanceProps(_this3.props, _assertThisInitialized(_this3));
    return _this3;
  }

  return List3;
}(PureComponent);

I'm testing on babel@7 but babel@6 should produce similar code in this regard.

Let's also focus only on the actual part that differ

With property initializer

_this._instanceProps = initInstanceProps(_this.props, _assertThisInitialized(_this));

Without property initializer

_this._instanceProps = initInstanceProps(props, _assertThisInitialized(_this));

So the difference is really negligible (but its there 🤣 ). Manual initializing can leverage props from the arguments and doesn't have to reach for it to the this, so it's shorter by few characters (assume mangled variable name for props, like a against mangled this reference b.props)

Can adjust it either way - this difference is micro even for me 😄 Before the PR they were also not unified, so this PR actually unified style around this - just maybe not in the way you'd like to. Let me know which version do you chose :)

[bvaughn]

This change doesn't actually seem necessary 😄 Good.

[bvaughn]

😆 Looks like our comments overlapped.

Yeah, I had checked out your branch locally and noticed the inconsistency.

I'm not sure I'm ready to care about the size of an extra "this" keyword here and there. (My variable and class names are pretty big after all.) Stuff like that becomes less important once the lib is used in an actual app and uglified.

[Andarist]

Sure thing, as I've said - this one is super micro so I dont care about it that much either. Was just commenting what's the difference because at first I chose to include this initializer manually in the constructor. I guess mainly because this:

constructor(props) {
  super(props)
}

looks super weird :p

[bvaughn]

Agreed! 😁 It also triggers a "useless constructor" lint warning hehe.

[bvaughn]

This is neat 😄 I learned something new.

[bvaughn]

FYI this was just published as part of 1.1.0

Thanks again~

[Andarist]

cool! thanks for working on useful libs in your free time! ❤️