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

requireSemicolons: check for ClassProperty #2057

Closed
wants to merge 1 commit into from

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.

hzoo added a commit to hzoo/node-jscs that referenced this pull request Jan 27, 2016
hzoo added a commit to hzoo/node-jscs that referenced this pull request Jan 27, 2016
@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.

None yet

4 participants