Context for nested classes in ES6 is strange

Question

I have the following nested class structure:

import React, {Component} from 'react';
import {TextField} from '@material-ui/core';
import './ProfileEditor.css';

export default class ProfileEditor extends Component {
    SyncedTextField = class SyncedTextField extends Component {
        onChange = event => {
            console.log(this);
        };

        render() {
            return (
                <TextField
                    {...this.props}
                    onChange={this.onChange}/>
            );
        }
    };

    render() {
        return (
            <form className={"ProfileEditor"}>
                <this.SyncedTextField/>
            </form>
        );
    }
}

When the code is bundled by Webpack, and run in Firefox, it runs this.onChange correctly, but the outputted this refers the the context of the ProfileEditor class instead.

This is excessively strange because in the JSX, when I refer to "this" it points to SyncedTextField correctly, but in the onChange method, it points to ProfileEditor.

I did add some properties to ProfileEditor to sanity check myself, and the properties showed up as declared in ProfileEditor, even when a conflicting definition was provided in SyncedTextField.

Can someone please tell me how I can avoid this issue, and what may be causing it?

Answer 1

Incorrect behaviour may be specific to browser development tools. But in this case it's caused by how transpiler works. There is a bug in Babel 6 class fields (which are stage 3 proposal) transform implementation.

The example compiled with Babel outputs ProfileEditor as this in onChange.

Here's SyncedTextField constructor from Babel output:

function SyncedTextField() {
  var _ref2;

  var _temp2, _this2, _ret2;

  _classCallCheck(this, SyncedTextField);

  for (
    var _len2 = arguments.length, args = Array(_len2), _key2 = 0;
    _key2 < _len2;
    _key2++
  ) {
    args[_key2] = arguments[_key2];
  }

  return (
    (_ret2 = ((_temp2 = ((_this2 = _possibleConstructorReturn(
      this,
      (_ref2 =
        SyncedTextField.__proto__ ||
        Object.getPrototypeOf(SyncedTextField)).call.apply(
        _ref2,
        [this].concat(args)
      )
    )),
    _this2)),
    (_this2.onChange = function(event) {
      console.log(_this); // SHOULD BE _this2
    }),
    _temp2)),
    _possibleConstructorReturn(_this2, _ret2)
  );
}

Notice that transpilers create _this, _this2, etc. temporary variables to provide lexical this in arrow functions but Babel uses wrong variable.

onChange = ... class field is syntactic sugar for:

  constructor(...args) {
    super(...args);

    this.onChange = event => {
        console.log(this);
    };
  }

When the example is changed from class fields to constructor code, it outputs SyncedTextField.

The same example compiled with TypeScript (used by Stackblitz by default in React template) works as expected and outputs SyncedTextField as this in onChange.

Since classes are rarely defined this way, Babel bug is usually not applicable.

SyncedTextField = class SyncedTextField extends Component {...} is an antipattern. There is no reason to nest class expression like that. It is inefficient because it is evaluated on each ProfileEditor instantiation. It should be class declaration, can be used as <SyncedTextField/> this way.

Even if SyncedTextField should be defined as a property of ProfileEditor component for testability or extensibility reasons, it's preferable to make it prototype property:

class SyncedTextField extends Component {...}

class ProfileEditor extends Component {
  get SyncedTextField() { return SyncedTextField }
  ...
}