Skip to content

Adds Eslint rule to handle promises. Closes #6005#6078

Merged
waldekmastykarz merged 1 commit intopnp:mainfrom
Jwaegebaert:eslintRulePromises
Jul 5, 2024
Merged

Adds Eslint rule to handle promises. Closes #6005#6078
waldekmastykarz merged 1 commit intopnp:mainfrom
Jwaegebaert:eslintRulePromises

Conversation

@Jwaegebaert
Copy link
Copy Markdown
Contributor

@Jwaegebaert Jwaegebaert commented Jun 7, 2024

Enabling this additional rule took more effort and changed more than expected.

First, to get @typescript-eslint/no-floating-promises working, I updated the compiler option from es2020 to esnext.

With this rule enabled, quite a few tests that never really worked or tested anything valid came to light. 😄 So, a few changes were made there too. In some cases, I couldn't find a way to solve the floating promise issue, so I added // eslint-disable-next-line @typescript-eslint/no-floating-promises. If I missed something in those cases, I'm always open to suggestions!

I'm also adding the priority flag to this PR since it’s a big change that affects the entire codebase. Any changes will likely require an intensive rebase.

Closes #6005

@Jwaegebaert Jwaegebaert added the pr-priority Process this PR asap label Jun 7, 2024
@waldekmastykarz
Copy link
Copy Markdown
Member

Good catch and a great addition @Jwaegebaert. Could you check out the one conflict that we've got so that we can get this merged?

@waldekmastykarz waldekmastykarz marked this pull request as draft July 4, 2024 12:52
@Jwaegebaert Jwaegebaert force-pushed the eslintRulePromises branch from dbfcf9b to 10ee972 Compare July 4, 2024 13:47
@Jwaegebaert Jwaegebaert marked this pull request as ready for review July 4, 2024 13:50
@waldekmastykarz waldekmastykarz self-assigned this Jul 5, 2024
Copy link
Copy Markdown
Member

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

Perfect! Nothing to add. Awesome work 👏

@waldekmastykarz waldekmastykarz merged commit 10ee972 into pnp:main Jul 5, 2024
@Jwaegebaert Jwaegebaert deleted the eslintRulePromises branch August 3, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-merged pr-priority Process this PR asap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Eslint rule to handle promises

2 participants