Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule proposal: prefer-object-define-properties #1729

Open
Pyrolistical opened this issue Feb 11, 2022 · 6 comments
Open

Rule proposal: prefer-object-define-properties #1729

Pyrolistical opened this issue Feb 11, 2022 · 6 comments

Comments

@Pyrolistical
Copy link

Description

I didn't know Object.defineProperties existed. Would have been nice for a rule to teach me as I wrote consecutive Object.defineProperty.

Fail

Object.defineProperty(matcher, 'logger', {
  value: () => {},
  writable: true,
})
Object.defineProperty(matcher, 'builder', {
  value: (value) => value.map(({ build }) => build()),
  writable: true,
})

Pass

Object.defineProperties(matcher, {
  logger: {
    value: () => {},
    writable: true,
  },
  builder: {
    value: (value) => value.map(({ build }) => build()),
    writable: true,
  }
})
@fisker
Copy link
Collaborator

fisker commented Feb 11, 2022

If there are many descriptors, normally it's a loop, if there are not many descriptors, I don't think they make much difference. But I'm fine adding this rule, just don't think it have much value.

@fisker
Copy link
Collaborator

fisker commented Feb 11, 2022

If we decide to add this rule, we should also check Reflect.defineProperty

@sindresorhus sindresorhus changed the title Rule proposal: prefer-object-defineProperties Rule proposal: prefer-object-define-properties Feb 11, 2022
@Pyrolistical
Copy link
Author

Another reason to reject this rule, it would interact poorly with microsoft/TypeScript#41424

@sindresorhus
Copy link
Owner

Another reason to reject this rule, it would interact poorly with microsoft/TypeScript#41424

This plugin targets JavaScript. TypeScript weaknesses is not a concern. TypeScript users can just disable this rule.

@sindresorhus
Copy link
Owner

I think if this is going to be useful, it needs an auto-fix. Manually converting to Object.defineProperties would not be worth the effort.

@sindresorhus
Copy link
Owner

Accepted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants