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

Prevent mistakes with Promise.resolve/reject() #47

Closed
sindresorhus opened this issue Nov 20, 2016 · 6 comments
Closed

Prevent mistakes with Promise.resolve/reject() #47

sindresorhus opened this issue Nov 20, 2016 · 6 comments

Comments

@sindresorhus
Copy link

sindresorhus commented Nov 20, 2016

I've seen this mistake a couple of times: avajs/ava#1119 (comment) Nested parens are hard to read and sometimes you wrap the wrong thing. Would be useful to have a rule the reported when you tried using more than one argument with Promise.resolve() or Promise.reject().

@jfmengels
Copy link
Contributor

That sounds like a good addition, but you could generalize that to all Promise functions.

  • 0 or 1 argument to Promise.resolve()/Promise.reject()
  • 1 argument to new Promise()
  • 1 or 2 arguments to .then()
  • 1 argument to .catch()
  • 1 argument to Promise.all()
  • etc...

@macklinu
Copy link
Contributor

macklinu commented Feb 5, 2018

I'd be interested in contributing this. Would this be a new rule, perhaps named valid-params? Any other thoughts on requirements for this rule?

@macklinu
Copy link
Contributor

macklinu commented Feb 6, 2018

I'm working on this currently and hope to have a PR up later today 😄

macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 6, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 6, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 7, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 7, 2018
@macklinu macklinu added the has pr label Feb 7, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 8, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 10, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 10, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 10, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 10, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 10, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 11, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 12, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 23, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 23, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 25, 2018
macklinu added a commit to macklinu/eslint-plugin-promise that referenced this issue Feb 25, 2018
@alexilyaev
Copy link

@macklinu Will it include 1 argument to Promise.all() check?

I've just had an issue with some code that passed 2 arguments to Promise.all instead of an array of 2 items, and it took me a long time to find it.

Will your PR cover it, or should I open a new ticket?

@macklinu
Copy link
Contributor

macklinu commented Mar 8, 2018

Yep, 1 argument to Promise.all() is covered in #85. 👍

macklinu added a commit that referenced this issue Mar 8, 2018
@macklinu
Copy link
Contributor

macklinu commented Mar 8, 2018

Just shipped v3.7.0 - please update and report any issues you find with this new rule. Thanks!

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

No branches or pull requests

5 participants