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

Private Name in the left ForOfStatement is not transformed #11272

Open
JLHwung opened this issue Mar 16, 2020 · 6 comments
Open

Private Name in the left ForOfStatement is not transformed #11272

JLHwung opened this issue Mar 16, 2020 · 6 comments

Comments

@JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 16, 2020

Bug Report

  • I would like to work on a fix!

Current Behavior
The following snippet, when transformed via preset-env and stage-3 preset, will throw

Property left of ForOfStatement expected node to be of a type ["VariableDeclaration","LVal"] but instead got "CallExpression"

Input Code

class C {
  #p;
  m() {
    for (this.#p of []);
  }
}

Expected behavior/code
It should be transformed into something like

// helpers is omissioned
var C = /*#__PURE__*/function () {
  function C() {
    _classCallCheck(this, C);

    _p.set(this, {
      writable: true,
      value: void 0
    });
  }

  _createClass(C, [{
    key: "m",
    value: function m() {
      for (var _i = 0, _arr = []; _i < _arr.length; _i++) {
        i = _arr[_i];
        _classPrivateFieldSet(this, _p, i);
      };
    }
  }]);

  return C;
}();

Possible Solution

As Justin noted, we can fix it by detecting such patterns in @babel/helper-member-expression-to-functions, so babel will apply destructureSet private name handler, which will return a member expression -- a valid LHS.

If you don't know how to clone Babel, follow these steps: (you need to have make and yarn available on your machine).

  1. Write a comment there to let other possible contributors know that you are working on this bug.
  2. Fork the repo
  3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
  4. Run yarn && make bootstrap
  5. Wait
  6. Run make watch
  7. Add a test (only input.js; output.js will be automatically generated)
  8. Update the code!
  9. yarn jest babel-plugin-proposal-class-properties to run the tests
    • If some test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
  10. If it is working, run make test to run all the tests
  11. Run git push and open a PR!

Additional context/Screenshots
This issue is likely due to plugin ordering: the private-class-properties runs before for-of-transforms after this.#p is transformed to helper calls which is not a valid LHS, the for-of transforms throws on unsupported for-of left.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

@babel-bot babel-bot commented Mar 16, 2020

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

@jridgewell

This comment has been minimized.

Copy link
Member

@jridgewell jridgewell commented Mar 16, 2020

Looks like we just need to use the destructureSet transform when inside of the init. This could be a GFI.

@reznord

This comment has been minimized.

Copy link
Contributor

@reznord reznord commented Mar 16, 2020

I would like to pick this one up.

@jridgewell

This comment has been minimized.

Copy link
Member

@jridgewell jridgewell commented Mar 23, 2020

How's this going @reznord?

@reznord

This comment has been minimized.

Copy link
Contributor

@reznord reznord commented Mar 23, 2020

Hey, had a rough last week. I’ll ge getting on this issue today. Sorry for the delay

@jridgewell

This comment has been minimized.

Copy link
Member

@jridgewell jridgewell commented Mar 23, 2020

No problem! This isn't to make you feel guilty, I just want to avoid situation where you're stuck and haven't asked for help. Take the time you need, especially with all circumstances going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.