Skip to content

Commit

Permalink
feat!: promise/avoid-new
Browse files Browse the repository at this point in the history
  • Loading branch information
@mightyiam
mightyiam committed Dec 14, 2024
1 parent c4e135c commit 727d02b
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/plugin-usage/promise.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const usage: PluginUsage = {
pluginName: 'promise',
plugin,
rules: {
'avoid-new': ['error'],
'param-names': ['error'],
},
}
Expand Down
1 change: 1 addition & 0 deletions src/test/_expected-exported-value.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export const expectedPromiseRules: Record<
string,
TSESLint.SharedConfig.RuleEntry
> = {
'promise/avoid-new': ['error'],
'promise/param-names': ['error'],
}

Expand Down
1 change: 0 additions & 1 deletion src/test/_rules_to_consider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ export const rulesToConsider: Record<string, string[]> = {
'n/prefer-promises/fs',
],
promise: [
'promise/avoid-new',
'promise/catch-or-return',
'promise/no-callback-in-promise',
'promise/no-multiple-resolved',
Expand Down

9 comments on commit 727d02b

@falco467
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule seems to be narrowly aimed at common errors in legacy node-js development.
When developing for a modern browser it seems to mainly report false positives, since it indiscriminately reports every use of new Promise() as an error.

@mightyiam
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that legitimate uses of new Promise are rare. May I see yours, please?

@falco467
Copy link

@falco467 falco467 commented on 727d02b Dec 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mightyiam my code uses await new Promise() to run code which can be cancelled by the user.
Maybe you know of a better way to do this?

let cancelFunc = null

async function doCancellableStuff() {
  try {
    await ...
    let d = await new Promise(async (reolve, reject) => {
      cancelFunc = reject
      resolve(pollData())
    }
    doSomethingWith(d)
  } catch(...)
}

@mightyiam
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I see an await inside a regular fn there.

@falco467
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, a slip while simplifying the code. updated now.

@mightyiam
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the Promise constructor executor is supposed to be an async fn.

@falco467
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. It can be, but it is dangerous to so so. What alternative would you recommend for the problem of providing an option to cancel a long running async function with a user action?

@mightyiam
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, may I see an example, please?

@mightyiam
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, would you mind using the discussions feature instead?

Please sign in to comment.