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

Prevent flow-strip-types/flow-comments from removing entire ClassProperty #4587

Merged
merged 1 commit into from Sep 28, 2016

Conversation

danharper
Copy link
Member

Q A
Bug fix? yes
Breaking change? no
New feature? no
Deprecations? no
Spec compliancy? no
Tests added/pass? yes
Fixed tickets #4388
License MIT
Doc PR -

ClassProperty had the Flow alias, which was causing it to be stripped, or converted to a comment.

It's the only non-Flow-specific syntax with that alias, and removing the alias doesn't break any other tests.


Input:

class X {
  foo = 2
  bar: number = 3
  baz: ?string
}

Previous flow-strip-types:

class X {}

New flow-strip-types:

class X {
  foo = 2;
  bar = 3;
}

Previous flow-comments:

class X {
  /*:: foo = 2*/
  /*:: bar: number = 3*/
  /*:: baz: ?string*/
}

New flow-comments:

class X {
  foo = 2;
  bar /*: number*/ = 3;
  baz /*: ?string*/;
}

@codecov-io
Copy link

codecov-io commented Sep 28, 2016

Current coverage is 88.91% (diff: 100%)

Merging #4587 into master will increase coverage by 0.19%

@@             master      #4587   diff @@
==========================================
  Files           195        195          
  Lines         13754      13875   +121   
  Methods        1424       1444    +20   
  Messages          0          0          
  Branches       3167       3207    +40   
==========================================
+ Hits          12203      12337   +134   
+ Misses         1551       1538    -13   
  Partials          0          0          

Powered by Codecov. Last update 5ea57d5...b081fdb

@hzoo hzoo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Sep 28, 2016
Copy link
Member

@danez danez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.

@danez danez merged commit 8709899 into babel:master Sep 28, 2016
panagosg7 pushed a commit to panagosg7/babel that referenced this pull request Jan 17, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 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 PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants