X Tutup
The Wayback Machine - https://web.archive.org/web/20201116043810/https://github.com/babel/babel/issues/10039
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 · 9 comments
Open

Should not remove class fields when using flow #10039

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

Comments

@nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo 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
Copy link

@pajaydev pajaydev commented May 29, 2019

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

@existentialism
Copy link
Member

@existentialism existentialism commented May 29, 2019

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

@pajaydev
Copy link

@pajaydev pajaydev commented May 29, 2019

@existentialism Cool. Thanks

@lidoravitan
Copy link
Contributor

@lidoravitan lidoravitan commented Jun 18, 2019

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

@pajaydev
Copy link

@pajaydev pajaydev commented Jun 18, 2019

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

@lidoravitan
Copy link
Contributor

@lidoravitan lidoravitan commented Jun 19, 2019

@pajaydev cool thanks for the response

@pajaydev
Copy link

@pajaydev pajaydev 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 ?.

@pajaydev
Copy link

@pajaydev pajaydev commented Jul 2, 2019

@nicolo-ribaudo I fixed the test. Thanks for your help. I appreciate it.

@nicolo-ribaudo nicolo-ribaudo mentioned this issue Nov 21, 2019
11 of 27 tasks complete
facebook-github-bot added a commit to facebook/flow that referenced this issue Feb 27, 2020
Summary:
Flow implemented class fields while they were very early-stage, behind the `esproposal.class_instance_fields` and `esproposal.class_static_fields` flowconfig options.

In current versions of the proposal, fields without initializers now default to `undefined`: [[design](https://github.com/tc39/proposal-class-fields#fields-without-initializers-are-set-to-undefined)] [[spec](https://tc39.es/proposal-class-fields/#sec-define-field)]

Babel < 8 stripped uninitialized fields, which is not spec-compliant. Babel 8 changes to leaving the field and just removing the annotation, as you'd rightly expect, but making it impossible* to have type-only fields. [[babel issue](babel/babel#10039)] [[babel 8 notes](babel/babel#10746)]

This diff introduces a `declare` syntax to provide a solution for type-only fields:

```
class C {
  declare foo: string;
  bar: string;
}

// becomes
class C {
  bar;
}
```

As far as Flow is concerned, both `foo` and `bar` are treated as before, as if they are `string`:

```
const x = new C;
(x.foo: string);
(x.bar: string);
```

Fixes #6811

[* technically you can use Flow comment syntax -- `class C { /*:: prop: string; */ }` but we don't want to require that]

Reviewed By: samwgoldman

Differential Revision: D20088452

fbshipit-source-id: f00299539c9387a55102e9a4e554a3b3bcb3498e
facebook-github-bot added a commit to facebook/flow that referenced this issue Mar 3, 2020
Summary:
Flow implemented class fields while they were very early-stage, behind the `esproposal.class_instance_fields` and `esproposal.class_static_fields` flowconfig options.

In current versions of the proposal, fields without initializers now default to `undefined`: [[design](https://github.com/tc39/proposal-class-fields#fields-without-initializers-are-set-to-undefined)] [[spec](https://tc39.es/proposal-class-fields/#sec-define-field)]

Babel < 8 stripped uninitialized fields, which is not spec-compliant. Babel 8 changes to leaving the field and just removing the annotation, as you'd rightly expect, but making it impossible* to have type-only fields. [[babel issue](babel/babel#10039)] [[babel 8 notes](babel/babel#10746)]

This diff introduces a `declare` syntax to provide a solution for type-only fields:

```
class C {
  declare foo: string;
  bar: string;
}

// becomes
class C {
  bar;
}
```

As far as Flow is concerned, both `foo` and `bar` are treated as before, as if they are `string`:

```
const x = new C;
(x.foo: string);
(x.bar: string);
```

Fixes #6811

[* technically you can use Flow comment syntax -- `class C { /*:: prop: string; */ }` but we don't want to require that]

Reviewed By: samwgoldman

Differential Revision: D20088452

fbshipit-source-id: f6102215d5cf13d91c073affd3adf85ce7d97a04
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.
X Tutup