-
Notifications
You must be signed in to change notification settings - Fork 885
prefer-object-spread: ignore Object.assign({}, this) #3126
prefer-object-spread: ignore Object.assign({}, this) #3126
Conversation
src/rules/preferObjectSpreadRule.ts
Outdated
* object types." | ||
*/ | ||
if (node.arguments[1] !== undefined) { | ||
if (node.arguments[1].kind === ts.SyntaxKind.ThisKeyword) { |
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.
that's not limited to the second argument. We should exit if any parameter to Object.assign
is this
similar to the check !node.arguments.some(isSpreadElement)
above
src/rules/preferObjectSpreadRule.ts
Outdated
/** | ||
* @url https://github.com/palantir/tslint/issues/3117 | ||
* Per TS2698, "spread types may only be created from | ||
* object types." |
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.
also add a TODO to remove this when typescript get's support for spread types and a link to the PR (linked in the issue comments)
@@ -1,3 +1,5 @@ | |||
const form = Object.assign({}, this); |
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.
add a test for const foo = Object.assign({}, bar, baz, this);
src/rules/preferObjectSpreadRule.ts
Outdated
* object types." | ||
* | ||
*/ | ||
if (node.arguments.some((arg) => arg.kind === ts.SyntaxKind.ThisKeyword)) { |
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.
Can you please add this to the condition ending in line 62
@@ -1,3 +1,5 @@ | |||
const form = Object.assign({}, this); | |||
const foo = Object.assign({}, bar, baz, this); |
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.
Needs another test: foo = Object.assign(bar, this);
Overview of change:
Allows for Object.assign(obj, this)