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 promise return (POC) #2489

Closed
wants to merge 1 commit into from

Conversation

fshovchko
Copy link
Contributor

@fshovchko fshovchko commented Jul 4, 2024

Closes: #1954
Request was to handle the following code

  class Test {
    @decorate(debounce, 100) // TS: type are incompatible
    protected async update(): Promise<void> {
       // ... await
    }
  }

In order to do so, debouncedSubject now needs to return the promise

@fshovchko fshovchko added bug Something isn't working enhancement Improvements and additions to code under discussion needs review The PR is waiting for review labels Jul 4, 2024
@fshovchko fshovchko requested a review from a team July 4, 2024 12:24
@fshovchko fshovchko self-assigned this Jul 4, 2024
@fshovchko fshovchko requested review from dshovchko, yadamskaya and NastaLeo and removed request for a team July 4, 2024 12:24
Copy link

codeclimate bot commented Jul 4, 2024

Code Climate has analyzed commit a054834 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 65.3% (0.0% change).

View more on Code Climate.

@ala-n ala-n changed the base branch from main to main-beta July 8, 2024 08:14
@ala-n ala-n changed the base branch from main-beta to main July 8, 2024 08:21
@ala-n ala-n changed the base branch from main to main-beta July 8, 2024 08:21
@ala-n ala-n added this to the ⚡ESL: 5.0.0 Major release milestone Jul 8, 2024
@ala-n ala-n requested a review from a team July 8, 2024 08:21
@@ -37,7 +41,9 @@ export function debounce<F extends AnyToAnyFnSignature>(fn: F, wait = 10, thisAr
deferred && deferred.resolve(result);
deferred = null;
}, wait);
return deferred.promise;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we change the actual return type?
It was made by request to make lazy creation of promise as the user does not need it in 95% of cases.

@fshovchko fshovchko closed this Jul 8, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement Improvements and additions to code needs review The PR is waiting for review under discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 in esl-utils]: debounce method is incompatible with async void functions typescript 5
2 participants