Skip to content
This repository was archived by the owner on Mar 23, 2024. It is now read-only.

requireSemicolons: check for ClassProperty#2057

Closed
hzoo wants to merge 1 commit intojscs-dev:masterfrom
hzoo:require-semi-cp
Closed

requireSemicolons: check for ClassProperty#2057
hzoo wants to merge 1 commit intojscs-dev:masterfrom
hzoo:require-semi-cp

Conversation

@hzoo
Copy link
Member

@hzoo hzoo commented Jan 7, 2016

@markelog
Copy link
Member

markelog commented Jan 7, 2016

Wow, didn't even know you could do that.
LGTM

@michaelficarra
Copy link

It's going to be hilarious when the babel people figure out this is a bug and the semicolon-free people need a semicolon remover.

@markelog
Copy link
Member

markelog commented Jan 9, 2016

Oh right, we need the same action for disallowSemicolons )

@markelog
Copy link
Member

markelog commented Jan 9, 2016

When prop without semicolon will became a valid syntax that is

@hzoo
Copy link
Member Author

hzoo commented Jan 9, 2016

Well when that happens it sounds like a good first bug/beginner friendly issue to me! 😄

@michaelficarra
Copy link

@markelog It is already valid to omit semicolons after class properties in most cases.

@markelog
Copy link
Member

markelog commented Jan 9, 2016

@michaelficarra

Really?

semicolons are required always.

@michaelficarra
Copy link

Really.

@markelog
Copy link
Member

markelog commented Jan 9, 2016

Don't see any clarification on the ASI in original link i provided, but judging by the interesting original discussion and rules of ASI you should be right.

Which means babel pr was indeed falsy (not this pr though) and that we can add same opposite logic to disallowSemicolons rule.

Thanks for playing!

@michaelficarra
Copy link

You're welcome. Don't forget about this case which I hadn't thought of at first.

@flying-sheep
Copy link

here’s the ASI clarification

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants