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

Enable logical assignment by default in @babel/parser #11860

Closed
JLHwung opened this issue Jul 21, 2020 · 6 comments · Fixed by #11869
Closed

Enable logical assignment by default in @babel/parser #11860

JLHwung opened this issue Jul 21, 2020 · 6 comments · Fixed by #11869
Assignees
Labels
good first issue i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser

Comments

@JLHwung
Copy link
Contributor

JLHwung commented Jul 21, 2020

Feature Request

Logical Assignment (&&=, ||=, ??=) is Stage 4!

This means that it should be enabled by default in @babel/parser 🎉

It should be done similarly to this commit: c3388ea

  1. Remove all the this.hasPlugin and this.expectPlugin checks related to logicalAssignment in packages/babel-parser/src
  2. Remove all the usages of logicalAssignment in @babel/parser's tests
  3. Probably there is a test to ensure that the plugin is required; it can be removed.
  4. Move the tests from the experimental folder to es2021, create one if it doesn't exist
  5. PR! 🎉

If it is the first time that you contribute to 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 (or make build whenever you change a file)
  7. Add a test if needed (only input.js; output.js will be automatically generated)
  8. Update the code!
  9. yarn jest babel-parser 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
    • If you prefer, you can run OVERWRITE=true yarn jest babel-parser and they will be automatically updated.
  10. If it is working, run yarn jest to run all the tests
  11. Run git push and open a PR!
@babel-bot
Copy link
Collaborator

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

@pustovalov
Copy link
Contributor

I would like to give it a try

@JLHwung
Copy link
Contributor Author

JLHwung commented Jul 21, 2020

@pustovalov Thanks for showing up. A good first issue targets to first-time contributors. That said, if it is not claimed by a new contributor in the next 24 hours, it will be yours!

@vahnag
Copy link
Contributor

vahnag commented Jul 21, 2020

I would like to take this. Will have it ready within 24 hours if that works.

@JLHwung
Copy link
Contributor Author

JLHwung commented Jul 21, 2020

@vahnag It's yours!

@existentialism
Copy link
Member

@vahnag no rush!

@JLHwung JLHwung closed this as completed Jul 30, 2020
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 30, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants