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

Should not remove class fields when using flow #10039

Open
nicolo-ribaudo opened this issue May 29, 2019 · 8 comments

Comments

Projects
None yet
4 participants
@nicolo-ribaudo
Copy link
Member

commented May 29, 2019

If the flow plugin runs before the class properties one (or if the class properties plugin isn't used), we remove uninitialize class fields:

class Foo { x: string }

currently becomes

class Foo {}

while it should be

class Foo { x }

If someone still wants the old behavior, they can either use flow comments (as offically recommended), or use the ignoreUninitialized option which will soon be added to the class properties plugin (#9141).


Is there anyone who wants to contribute for the first time to Babel? 🙂

NOTE ⚠️⚠️⚠️ Since this is a breaking change, we won't merge the PR for a long time (before Babel 8), and it might seem that we are ignoring it.

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 know other possible contributors 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. Change the wrong output.js files in the tests/fixture folder of @babel/plugin-transform-flow-strip-types (if there are any)
  8. Update the code!
  9. yarn jest [name-of-the-package-to-test] 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 tu run all the tests
  11. Run git push and open a PR!
@pajaydev

This comment has been minimized.

Copy link

commented May 29, 2019

@nicolo-ribaudo Thanks. I can try this out.

@existentialism

This comment has been minimized.

Copy link
Member

commented May 29, 2019

@ajay2507 it's yours! feel free to hit us up if you have any questions!

@pajaydev

This comment has been minimized.

Copy link

commented May 29, 2019

@existentialism Cool. Thanks

@lidoravitan

This comment has been minimized.

Copy link

commented Jun 18, 2019

@existentialism @pajaydev is there some news with this issue? does still need help? thanks

@pajaydev

This comment has been minimized.

Copy link

commented Jun 18, 2019

@lidoravitan I have done it, Some test are failing. I need to create a PR and get reviewed.

@lidoravitan

This comment has been minimized.

Copy link

commented Jun 19, 2019

@pajaydev cool thanks for the response

@pajaydev

This comment has been minimized.

Copy link

commented Jun 24, 2019

@existentialism @nicolo-ribaudo I created this PR #10120 . this test babel-plugin-transform-flow-strip-types/test/fixtures/regression/class-prop-types is failing, Shall I remove the transform-classes from babel-plugin-transform-flow-strip-types/test/fixtures/regression/class-prop-types/options.json ?.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.