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?

yes, that's right - because it's an arrow function ... whose `this` is not the same as if the function were the regular notation (i.e. how you did `render()`) — Jaromanda X, Aug 02 '18 at 04:34
Oh, so the arrow function would remain bound to the `ProfileEditor` scope, not the `SyncedTextField`? Seems kinda strange to me, but thanks! — Aanand Kainth, Aug 02 '18 at 04:35
it's got to do with what `this` is at the time the function is defined or something - I'm horrible at explaining arrow functions :p is there any reason you can't `onChange(event) {` ... etc? — Jaromanda X, Aug 02 '18 at 04:37
No, there isn't, I was just experimenting with what the context would be, and the results really surprised me. I think it may be because `class` is internally a fancier way to provide a function with a prototype, which I saw when I logged it. — Aanand Kainth, Aug 02 '18 at 04:38

Estus Flask · Answer 1 · 2018-08-02T11:51:48.547

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 }
  ...
}

Amazing answer. Does Create React App automatically add Babel? As a side note, I didn't want to extract SyncedTextField because it managed the `ProfileEditor`'s state. — Aanand Kainth, Aug 02 '18 at 16:57
create-react-app is just CLI tool to set up https://github.com/facebook/create-react-app/tree/master/packages/react-scripts/template Webpack+Babel template. So yes, it uses Babel. I would expect this problem to be fixed in Babel 7, so you could eject the project and upgrade Babel to 7. *because it managed the ProfileEditor's state* - this isn't shown in the question. It's very likely that there are more idiomatic ways to do what you're trying to do. Consider asking a question if needed. — Estus Flask, Aug 02 '18 at 17:02

Context for nested classes in ES6 is strange

1 Answers1

Linked