Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

preset-env with async arrow function class properties produces broken code on Edge #8019

Closed
taion opened this issue May 24, 2018 · 9 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@taion
Copy link
Contributor

taion commented May 24, 2018

Bug Report

Current Behavior
See the code below. This code works as expected on Chrome (the instance of Bar) is logged, but it does not work on Edge. On Edge, undefined is logged.

This is because super() apparently returns undefined on Edge when the superclass is not actually an ES2015 class.

In my example, this comes up in practice when I have things extending React.Component.

Input Code
https://babeljs.io/repl/#?babili=false&browsers=Edge%3E%3D14%2C%20Chrome%20%3E%3D%2060&build=&builtIns=usage&code_lz=PTAEEkFsEMHMEsB2BTUAXAFvAzqHpp0AnaRbAB3gBtkATUAYyum2wDoAoAMwFdEG08APaJQAMSFCAFAEpQAbwC-HDkxa4AQtCKhkADzTJEtXBKEKOoRiOxoiPAUKKyLVq9h7lkzmQG5LoMoBXJKgALygLmEAfAqK_gEARtrhBNgAnvyRcjGubgw2QjRsVEKwUpg4fgHxHEEoAO6gWj5syT6-QA&debug=true&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=true&fileSize=false&lineWrap=false&presets=stage-2%2Cenv&prettier=false&targets=&version=6.26.0&envVersion=1.6.2

Expected behavior/code
A clear and concise description of what you expected to happen (or code).

Babel Configuration (.babelrc, package.json, cli command)

Per REPL.

Environment

  • Babel version(s): v6.26.0 in REPL, but I actually saw this with a recent v7 beta
  • Node/npm version: Node 8/npm 5
  • OS: OS X 10.12.6
  • Monorepo: No
  • How you are using Babel: webpack, CLI, and REPL

Possible Solution
@loganfsmyth suggested transpiling to (super(...), _this = this) instead of _this = super(...).

@babel-bot
Copy link
Collaborator

Hey @taion! 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.

@loganfsmyth
Copy link
Member

As mentioned on Slack, sounds like chakra-core/ChakraCore#4663 so we may want to have a special case for class support on Edge or something to fall back to non-native classes.

@taion
Copy link
Contributor Author

taion commented May 24, 2018

And for additional reference, I'm seeing this on the most recent Edge available on SauceLabs:

Microsoft Edge 42.17134.1.0
Microsoft EdgeHTML 17.17134

@macrozone
Copy link

is there any workaround for this? This looks like an easy fix, but the bug has a heavy impact.

@nicolo-ribaudo
Copy link
Member

For now you could force classes to be transformed.

@vitorbal
Copy link
Contributor

Would transpiling to (super(...), _this = this), as mentioned in the issue description, be an acceptable solution here?

Or is the suggested fix to force classes to be transformed and wait for the ChakraCore fix to land?

@mrm007
Copy link

mrm007 commented Feb 26, 2019

Checked with latest Babel (7.3.4) and it seems to be fixed. REPL link (you need to add @babel/proposal-plugin-class-properties).

Input:

// Imagine this is a transpiled class.
function Foo() {}

class Bar extends Foo {
  constructor() {
    super();
  }

  bar = async () => {
    console.log(this);
  };
}

new Bar().bar();

Output:

// Imagine this is a transpiled class.
function Foo() {}

class Bar extends Foo {
  constructor() {
    var _this;

    super();      // important part is here
    _this = this; // important part is here

    _defineProperty(this, "bar",
    // etc.
  }
}

new Bar().bar();

As you can see, _this = super(...) doesn't happen any more.

@taion
Copy link
Contributor Author

taion commented Feb 26, 2019

nice!

@taion taion closed this as completed Feb 26, 2019
@taion
Copy link
Contributor Author

taion commented Feb 26, 2019

This appears to have worked since 7.0.0, at least. Very nice.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

7 participants