docs: [no-floating-promises] clarify that void does not resolve promises #9949

alexandercerutti · 2024-09-06T22:52:08Z

PR Checklist

Addresses an existing open issue: fixes Docs: [no-floating-promises] voiding Promise doesn't make it handled #9947
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Reworded some points of no-floating-promises and added a warn under ignoreVoid option.

typescript-eslint · 2024-09-06T22:52:11Z

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2024-09-06T22:52:23Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`230e741`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/66f1045d47dac400083a495a
😎 Deploy Preview	https://deploy-preview-9949--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 4 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 90 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

nx-cloud · 2024-09-06T22:52:58Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 230e741. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 32 targets

Sent with 💌 from NxCloud.

kirkwaiblinger

Thanks for sending this in! Sorry for leaving so many comments - the promise docs in general, and especially this rule, deal with quite a bit of subtle nuance.

packages/eslint-plugin/docs/rules/no-floating-promises.mdx

Josh-Cena · 2024-09-08T03:26:06Z

I'll let Kirk take on this since apparently he has a lot of opinions while I have much fewer, but if either of you want to add anything to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises#error_handling, and maybe eventually link to it on this doc page, I'd be glad to review a PR over there on MDN.

codecov · 2024-09-09T11:18:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.70%. Comparing base (1c183ab) to head (230e741).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9949   +/-   ##
=======================================
  Coverage   88.70%   88.70%           
=======================================
  Files         426      426           
  Lines       14835    14835           
  Branches     4312     4312           
=======================================
  Hits        13159    13159           
  Misses       1533     1533           
  Partials      143      143

Flag	Coverage Δ
unittest	`88.70% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

alexandercerutti · 2024-09-09T12:04:41Z

@Josh-Cena I've added a link to MDN page you linked in order to improve the warning explanation.

packages/eslint-plugin/docs/rules/no-floating-promises.mdx

… ignoreVoid

alexandercerutti · 2024-09-10T06:20:00Z

@kirkwaiblinger I've written a paragraph about Promises handling and linked it to the ignoreVoid warning. Let me know what do you think about it.

Once the changes are ready, I'll merge the base branch into this (again).

packages/eslint-plugin/docs/rules/no-floating-promises.mdx

kirkwaiblinger

just need resolution on #9949 (comment)

alexandercerutti · 2024-09-17T20:47:04Z

@kirkwaiblinger done, removed. Should I remove even the warn?

kirkwaiblinger · 2024-09-18T01:16:04Z

@kirkwaiblinger done, removed. Should I remove even the warn?

No, the warn admonition is great! 👌

packages/eslint-plugin/docs/rules/no-floating-promises.mdx

kirkwaiblinger

Great! Thanks for bearing with us on this.

kirkwaiblinger · 2024-09-19T15:58:02Z

(once another member gets a chance to review, they'll merge it in, and if all goes well this will go out with our next Monday release 🙂)

JoshuaKGoldberg

LGTM! Just some small suggestions.

packages/eslint-plugin/docs/rules/no-floating-promises.mdx

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

JoshuaKGoldberg

yay!

Improved no-floating-promises documentation

5a33bbe

alexandercerutti changed the title ~~Improved no-floating-promises documentation~~ docs: Improved no-floating-promises page Sep 6, 2024

alexandercerutti changed the title ~~docs: Improved no-floating-promises page~~ docs: [no-floating-promises] improved page Sep 6, 2024

kirkwaiblinger requested changes Sep 7, 2024

View reviewed changes

kirkwaiblinger added the awaiting response Issues waiting for a reply from the OP or another party label Sep 7, 2024

Josh-Cena mentioned this pull request Sep 8, 2024

docs: [no-floating-promises] add MDN link regarding void operator #9953

Merged

3 tasks

Merge branch 'main' into docs/no-floating-promises-rewording

1fa3b8a

alexandercerutti added 4 commits September 9, 2024 13:20

Changed silencing phrase

e65bd7b

Moved rules report order to prioritize handling

a68205b

Improved ignoreVoid warning

a1c35cd

Removed handled/unhandled markers and added a bit on the next tip

a3e97b9

alexandercerutti requested a review from kirkwaiblinger September 9, 2024 12:03

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Sep 9, 2024

kirkwaiblinger reviewed Sep 9, 2024

View reviewed changes

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

alexandercerutti added 2 commits September 9, 2024 19:20

Reverted tip edit

628b753

Added a paragraph about promises handling and improved the warning in…

dc61f8f

… ignoreVoid

alexandercerutti requested a review from kirkwaiblinger September 10, 2024 06:19

Merge branch 'main' into docs/no-floating-promises-rewording

dc54247

kirkwaiblinger requested changes Sep 16, 2024

View reviewed changes

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

JoshuaKGoldberg reviewed Sep 16, 2024

View reviewed changes

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

alexandercerutti added 2 commits September 17, 2024 18:33

Moved handling Promises paragraph up

6e1d755

Reworked a bit Handling Promises rejections

59cbb55

kirkwaiblinger requested changes Sep 17, 2024

View reviewed changes

Removed Handling Promises rejection

b985141

alexandercerutti requested a review from kirkwaiblinger September 17, 2024 20:47

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Sep 17, 2024

Added link to mdn in the tip

5a96121

kirkwaiblinger reviewed Sep 18, 2024

View reviewed changes

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

Removed ref to handling promises rejection (not existing anymore)

526258e

alexandercerutti requested a review from kirkwaiblinger September 19, 2024 09:23

kirkwaiblinger previously approved these changes Sep 19, 2024

View reviewed changes

kirkwaiblinger added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Sep 19, 2024

Merge branch 'main' into docs/no-floating-promises-rewording

8be3a3b

kirkwaiblinger requested a review from JoshuaKGoldberg September 19, 2024 16:44

github-actions bot removed the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Sep 19, 2024

kirkwaiblinger requested a review from Josh-Cena September 19, 2024 16:45

JoshuaKGoldberg previously approved these changes Sep 19, 2024

View reviewed changes

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

packages/eslint-plugin/docs/rules/no-floating-promises.mdx Outdated Show resolved Hide resolved

Changed monospace comment in warning to link

bf8343d

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

alexandercerutti dismissed stale reviews from JoshuaKGoldberg and kirkwaiblinger via bf8343d September 19, 2024 17:25

Typo fix

44038ea

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

kirkwaiblinger approved these changes Sep 19, 2024

View reviewed changes

Josh-Cena approved these changes Sep 19, 2024

View reviewed changes

kirkwaiblinger changed the title ~~docs: [no-floating-promises] improved page~~ docs: [no-floating-promises] improved page Sep 19, 2024

JoshuaKGoldberg approved these changes Sep 19, 2024

View reviewed changes

Merge branch 'main' into docs/no-floating-promises-rewording

230e741

kirkwaiblinger changed the title ~~docs: [no-floating-promises] improved page~~ docs: [no-floating-promises] clarify that void does not resolve promises Sep 23, 2024

kirkwaiblinger merged commit c5dc755 into typescript-eslint:main Sep 23, 2024
65 checks passed

alexandercerutti deleted the docs/no-floating-promises-rewording branch September 23, 2024 16:33

github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: [no-floating-promises] clarify that void does not resolve promises #9949

docs: [no-floating-promises] clarify that void does not resolve promises #9949

alexandercerutti commented Sep 6, 2024

typescript-eslint bot commented Sep 6, 2024

netlify bot commented Sep 6, 2024 •

edited

Loading

nx-cloud bot commented Sep 6, 2024 •

edited

Loading

kirkwaiblinger left a comment

Josh-Cena commented Sep 8, 2024

codecov bot commented Sep 9, 2024 •

edited

Loading

alexandercerutti commented Sep 9, 2024

alexandercerutti commented Sep 10, 2024 •

edited

Loading

kirkwaiblinger left a comment

alexandercerutti commented Sep 17, 2024

kirkwaiblinger commented Sep 18, 2024

kirkwaiblinger left a comment

kirkwaiblinger commented Sep 19, 2024

JoshuaKGoldberg left a comment

JoshuaKGoldberg left a comment

docs: [no-floating-promises] clarify that void does not resolve promises #9949

docs: [no-floating-promises] clarify that void does not resolve promises #9949

Conversation

alexandercerutti commented Sep 6, 2024

PR Checklist

Overview

typescript-eslint bot commented Sep 6, 2024

netlify bot commented Sep 6, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Sep 6, 2024 • edited Loading

☁️ Nx Cloud Report

kirkwaiblinger left a comment

Choose a reason for hiding this comment

Josh-Cena commented Sep 8, 2024

codecov bot commented Sep 9, 2024 • edited Loading

Codecov Report

alexandercerutti commented Sep 9, 2024

alexandercerutti commented Sep 10, 2024 • edited Loading

kirkwaiblinger left a comment

Choose a reason for hiding this comment

alexandercerutti commented Sep 17, 2024

kirkwaiblinger commented Sep 18, 2024

kirkwaiblinger left a comment

Choose a reason for hiding this comment

kirkwaiblinger commented Sep 19, 2024

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

netlify bot commented Sep 6, 2024 •

edited

Loading

nx-cloud bot commented Sep 6, 2024 •

edited

Loading

codecov bot commented Sep 9, 2024 •

edited

Loading

alexandercerutti commented Sep 10, 2024 •

edited

Loading