-
Notifications
You must be signed in to change notification settings - Fork 885
Treat member variables initialized to a function as a function for ordering purposes #969
Conversation
super.visitPropertyDeclaration(node); | ||
} | ||
|
||
public visitPropertySignature(node: ts.Node) { | ||
this.checkModifiersAndSetPrevious(node, getModifiers(false, node.modifiers)); | ||
public visitPropertySignature(node: ts.PropertyDeclaration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's debatable if ts.PropertyDeclaration
is the right type here. However, this is what we use in other places for this method signature so I went with it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it is discussed here: microsoft/TypeScript#3833
PropertySignature
nodes will be visited when linting interfaces. But looking at that code snippet from the parser, ts.PropertyDeclaration
is probably safe enough to use as the typedef here. I guess we'll rely on testing :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nice find. We really only use this to access common fields like node.type
so shouldn't be a problem
Exciting! What a long standing issue. Will take a look today. |
@@ -171,7 +171,8 @@ A sample configuration file with all options is available [here](https://github. | |||
* `member-ordering` enforces member ordering. Rule options: | |||
* `public-before-private` All public members must be declared before private members. | |||
* `static-before-instance` All static members must be declared before instance members. | |||
* `variables-before-functions` All variables needs to be declared before functions. | |||
* `variables-before-functions` All member variables need to be declared before member functions. | |||
Member variables initialized to a function literal are treated as member functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In retrospect, this doesn't seem like the right option name... shouldn't it be properties-before-methods
? If you agree I'll file an issue as a breaking change for v4.x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh, it's not perfect because functions assigned with =
are properties really but we treat them as methods. (I don't think you'd call functions on the prototype "properties" but not sure.) Let me think about this, but I'm inclined just to leave it as it is because I don't see a big clarity benefit worth making a breaking change
… a function for ordering purposes
097cc35
to
6bc6e0d
Compare
Updated (via an amended commit) |
👍 |
Treat member variables initialized to a function as a function for ordering purposes
Fixes #226