Add prefer-object-define-properties rule
#1843
Conversation
|
|
|
I tried to update the snapshot but it doesn't seems to work |
Shouldn't they next to each other? |
Oh right. |
|
This is how I did in other rule |
|
Putting this on draft for now |
| return ( | ||
| node | ||
| && node.type === 'CallExpression' | ||
| && node.callee.type === 'MemberExpression' |
There was a problem hiding this comment.
| && node.callee.type === 'MemberExpression' | |
| && node.callee.type === 'MemberExpression' | |
| && !node.computed |
Object[defineProperty](...)
There was a problem hiding this comment.
I can only think of something like tihs
|
I made some changes. I hope it's easier to understand the code. Mainly changes:
This not done yet.
Maybe other isssues, I'll review again later. |
| return descriptors.properties.map(property => sourceCode.getText(property)).join(',\n'); | ||
| } | ||
|
|
||
| return `...(${sourceCode.getText(descriptors)})`; |
There was a problem hiding this comment.
@Slowlife01 We are adding () to all descriptors, can you exclude some of them with this function https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/rules/utils/should-add-parentheses-to-spread-element-argument.js?
There was a problem hiding this comment.
Need add tests, and should do isParenthesized check first.
|
|
||
| const MESSAGE_ID = 'prefer-object-define-properties'; | ||
| const messages = { | ||
| [MESSAGE_ID]: 'Prefer `{{replacement}}` over multiple `{{value}}`.', |
There was a problem hiding this comment.
This message may cause confusing, since the "secondCallExpression" can be Object.defineProperties, we need improve it.
| 🔧 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).* | ||
| <!-- /RULE_NOTICE --> | ||
|
|
||
| When defining more than one properties, [Object.defineProperties](https://mdn.io/Object.defineProperties) should be preferred. |
There was a problem hiding this comment.
| When defining more than one properties, [Object.defineProperties](https://mdn.io/Object.defineProperties) should be preferred. | |
| When defining more than one property, [Object.defineProperties](https://mdn.io/Object.defineProperties) should be preferred. |
|
I won't be able to finish this now |
Added
prefer-object-define-propertiesrule, resolves #1729Might need more test cases and better description
And unsure if fixer can be improved (formatting the output etc)