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

fix(esl-utils): debounced unhandled rejection #1839

Merged
merged 8 commits into from
Aug 8, 2023

Conversation

fshovchko
Copy link
Contributor

The scope of this PR is to fix the issue described in #1801, and to refactor code from a function to a class

closes #1801

@fshovchko fshovchko added the bug Something isn't working label Jul 28, 2023
@fshovchko fshovchko requested review from a team, dshovchko, ala-n, julia-murashko, abarmina, NastaLeo, Natalie-Smirnova and HenadzV and removed request for a team July 28, 2023 14:11
@ala-n ala-n added this to the ⚡ESL: 4.10.0 Minor release milestone Jul 28, 2023
Copy link
Collaborator

@ala-n ala-n left a comment

Choose a reason for hiding this comment

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

Please consider an alternative approach:

  • do not change debounce (most likely, at all). A similar issue may appear in other async decorators
  • Make the Deferred utility improved with a lazy <instanceof Deferred>.promise instead

The original request to make a class was related to Defered but not a denounced.

Please also provide tests to make sure we have no problem (ideally, first of all, make a unit test cases and make sure they are correct but not successful, and then provide a fix)

@fshovchko fshovchko requested a review from ala-n July 29, 2023 14:27
@fshovchko fshovchko requested a review from ala-n July 29, 2023 17:50
@ala-n ala-n added the require squash No action from PR author required. Marks that current PR will be merged with squash label Aug 4, 2023
@ala-n ala-n self-assigned this Aug 4, 2023
@ala-n ala-n added the needs review The PR is waiting for review label Aug 4, 2023
@codeclimate
Copy link

codeclimate bot commented Aug 8, 2023

Code Climate has analyzed commit 4fb5a0a and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 49.4% (0.1% change).

View more on Code Climate.

@ala-n ala-n force-pushed the main branch 6 times, most recently from 1f18a8a to 6506db7 Compare August 8, 2023 15:19
@ala-n ala-n merged commit 5e111ba into main Aug 8, 2023
6 checks passed
@ala-n ala-n deleted the fix/debounced-uncaught-reject branch August 8, 2023 16:00
@github-actions github-actions bot locked and limited conversation to collaborators Aug 8, 2023
@ala-n ala-n added the released label Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working needs review The PR is waiting for review released require squash No action from PR author required. Marks that current PR will be merged with squash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 in esl-utils]: 'Uncaught in promise' after debounce.cancel() call
7 participants