Skip to content

Enable "object-literal-shorthand" lint rule#16987

Merged
1 commit merged into
masterfrom
object-literal-shorthand
Jul 7, 2017
Merged

Enable "object-literal-shorthand" lint rule#16987
1 commit merged into
masterfrom
object-literal-shorthand

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jul 6, 2017

Replace { foo: foo } with { foo } and { foo: function() {} } with { foo() {} }.

@ghost ghost force-pushed the object-literal-shorthand branch from 1a4a1a8 to 962b70e Compare July 6, 2017 20:18
Copy link
Copy Markdown
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Seems like an improvement. There are some more parameters that could be renamed to compact further, such as here, but this is already nice.

Comment thread src/compiler/sys.ts
resolvePath: function(path: string): string {
return _path.resolve(path);
},
resolvePath: path => _path.resolve(path),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

resolvePath: _path.resolve,

Copy link
Copy Markdown
Member

@RyanCavanaugh RyanCavanaugh Jul 7, 2017

Choose a reason for hiding this comment

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

👎 - that's a possible behavioral change in multiple ways (different this reference, different behavior when more than one argument is supplied). Very dangerous if someone is currently calling e.g. [a, b].map(sys.resolvePath), it will throw an exception where one wasn't before.

@ghost ghost merged commit e7dc2a6 into master Jul 7, 2017
@ghost ghost deleted the object-literal-shorthand branch July 7, 2017 14:27
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
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