Babel rely on value returned by super() but this is not reliable with Edge

[JSteunou]

Bug Report

Current Behavior

When using some transpiled code, babel rely on _this from _this = super() but this bug on Edge is still present with latest version chakra-core/ChakraCore#4663

Input Code

• REPL or Repo link if applicable:

export class App extends React.Component {
    state = {
        fruit: 'banana'
    }

    handleClick = (event, ...rest) => {
        this.setState({fruit: 'apple'})
    }

    render() {
        const {fruit} = this.state
        return (
            <h1 onClick={this.handleClick}>I'm a {fruit}</h1>
        )
    }
}

become with preset set on targets: {esmodules: true}

class App extends react__WEBPACK_IMPORTED_MODULE_0___default.a.Component {
  constructor() {
    var _this;

    _this = super(...arguments);
    this.state = {
      fruit: 'banana'
    };

    this.handleClick = function (event) {
      _this.setState({
        fruit: 'apple'
      });
    };
  }

  render() {
    const fruit = this.state.fruit;
    return react__WEBPACK_IMPORTED_MODULE_0___default.a.createElement("h1", {
      onClick: this.handleClick,
      __source: {
        fileName: _jsxFileName,
        lineNumber: 16
      },
      __self: this
    }, "I'm a ", fruit);
  }

}

Expected behavior/code

The initial behavior is that _this should not be undefined but I think that is in the hands of ChakraCore team. Is this something babel can do? There is already this issue which is kinda similar #8019 but also this one which emphasis on the fact that _this should user the value resturned by super() #2279

Environment

• Babel version(s): v7.1.5
• Node/npm version: Node 8/npm 6
• OS: build Ubuntu 18.04, this test on Windows 10 / Edge 17
• Monorepo no
• How you are using Babel: loader

Possible Solution

Another possible solution would be to not transpile on rest/spread & default argument function when targets: {esmodules: true} or targets: {browsers: 'Edge >= 14'} because its seems they support it

[babel-bot]

Hey @JSteunou! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

[JSteunou]

I forgot to say but if the arrow function does not contains rest/spread or default argument value the transpiled code is from

export class App extends React.Component {
    state = {
        fruit: 'banana'
    }

    handleClick = (event) => {
        this.setState({fruit: 'apple'})
    }

    render() {
        const {fruit} = this.state
        return (
            <h1 onClick={this.handleClick}>I'm a {fruit}</h1>
        )
    }
}

to

class App extends react__WEBPACK_IMPORTED_MODULE_0___default.a.Component {
  constructor() {
    super(...arguments);
    this.state = {
      fruit: 'banana'
    };

    this.handleClick = event => {
      this.setState({
        fruit: 'apple'
      });
    };
  }

  render() {
    const fruit = this.state.fruit;
    return react__WEBPACK_IMPORTED_MODULE_0___default.a.createElement("h1", {
      onClick: this.handleClick,
      __source: {
        fileName: _jsxFileName,
        lineNumber: 16
      },
      __self: this
    }, "I'm a ", fruit);
  }

}

hence the emphasis on the possible solution to not transpile rest/spread and default argument for Edge

[JSteunou]

I tracked down when & why this regression and I found that this fix #8926 introduced it

Rolling back to @babel/preset-env@7.1.0 transpile to

class App extends react__WEBPACK_IMPORTED_MODULE_0___default.a.Component {
  constructor(...args) {
    super(...args);
    this.state = {
      fruit: 'banana'
    };

    this.handleClick = (event, ...rest) => {
      this.setState({
        fruit: 'apple'
      });
    };
  }

  render() {
    const fruit = this.state.fruit;
    return react__WEBPACK_IMPORTED_MODULE_0___default.a.createElement("h1", {
      onClick: this.handleClick,
      __source: {
        fileName: _jsxFileName,
        lineNumber: 16
      },
      __self: this
    }, "I'm a ", fruit);
  }

}

I think the PR #8926 is to aggressive. If I understand well, the need was for object spread with default param, not classical rest/pread nor simple default param.

Update: link correct PR

[frehner]

We just ran into this too, and reverting back to 7.1.0 fixed it. Thank you for your research and workaround, @JSteunou!

[JSteunou]

no problem, I just hope someone like @nicolo-ribaudo @loganfsmyth or @hzoo will take a look at it, because I think it can be a serious bug, and a very difficult one to analyse to babel users

[nicolo-ribaudo]

We should just output super(), _this = this instead of _this = super(). It's only 6 bytes more per class and fixes the issue without introducing any additional flag.
It would probably be an easy PR.

[JSteunou]

I'm not sure about this, because there was already bugfix around it, and _this = super() was needed. (Cannot find the history, issue around inheritance)

I think the real issue is around this PR #8926 and maybe babel-preset should have a more fine grain around this with Edge.

[joeldenning]

@JSteunou I'm considering submitting a PR for this, but I don't see how #8926 is related? I'm no expert in the babel codebase, but #8926 seems related to arrow functions only, not to classes? Is "destructuring, parameters / defaults, arrow function" somehow related to _this = super()?

[joeldenning]

Oops now I see the connection 👍

[joeldenning]

@JSteunou I just created #9060 which (hopefully) fixes this. This is my first contribution to babel, so I might be unaware of why my solution doesn't work or something, but am hoping it's alright :)

About what you said earlier regarding _this = super() being necessary, do you know which issue that was related to? The solution I implemented was the one that @nicolo-ribaudo suggested in this comment

[JSteunou]

I'm sorry I cannot get my hand on this old issue :( I was counting on @loganfsmyth memory

Maybe by searching on some file history or tests