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

Enhancement: [no-floating-promises] add ignorePattern #7006

Closed
4 tasks done
kkmuffme opened this issue May 7, 2023 · 1 comment
Closed
4 tasks done

Enhancement: [no-floating-promises] add ignorePattern #7006

kkmuffme opened this issue May 7, 2023 · 1 comment
Labels
duplicate This issue or pull request already exists enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@kkmuffme
Copy link

kkmuffme commented May 7, 2023

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/no-floating-promises/

Description

When moving to TS/upgrading legacy code you often have e.g. jQuery.ajax() calls which now return a promise, but 99% of people write their success/error callbacks directly in the param object.

This leads to tons of eslint-disable-next-line comments.

An option for the rule like ignorePattern (which exists in eslint for various rules) which would exclude certain patterns from reporting an error would be great

Fail

no change

Pass

jQuery.ajax({
	method: 'POST',
	url: 'example.com',
	data: {
		foo: 'bar',
	},
	complete: function() {
		doSomething();
	}
});

Additional Info

No response

@kkmuffme kkmuffme added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels May 7, 2023
@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented May 7, 2023

99% of people write their success/error callbacks directly in the param object.

The rule is generally right to warn on that pattern. There's often no guarantee available in the type system that those calls don't themselves throw an error:

jQuery.ajax({
	complete: doSomething,
	method: 'GET',
	url: 'example.com',
});

function doSomething() {
  if (somethingRiskyThatOccasionallyFails()) {
    throw new Error("oh no");
  }
}

...which is why complete methods and similar on jQuery Ajax calls are not how I would recommend using them. Even the jQuery.ajax() docs don't include an explicit example of them, as they're not the primary recommended way of using the functions.

moving to TS/upgrading legacy code

In general, we try not to add options for old/legacy patterns that the community has moved away from. We'd want to see an example of a case where a "floating" Promise is a good way of writing the code. jQuery's complete is not convincing.


That being said, we have seem more convincing cases, such as #5231. I'll re-file this issue with a more broad context and links to older issues. [edit: that's #7008]

(apologies for hijacking your issue - I've had bad experiences before from discussions on reasonable feature requests getting sidetracked by non-optimal starting suggestions)

@JoshuaKGoldberg JoshuaKGoldberg added duplicate This issue or pull request already exists and removed triage Waiting for maintainers to take a look labels May 7, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
duplicate This issue or pull request already exists enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

2 participants