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(condo): DOMA-10006 add throttling #5126

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

toplenboren
Copy link
Member

No description provided.

Copy link

sonarcloud bot commented Aug 20, 2024

@toplenboren toplenboren merged commit a8acc42 into main Aug 20, 2024
11 of 12 checks passed
@toplenboren toplenboren deleted the toplenboren-hotfix-recipients-counter branch August 20, 2024 18:06
@@ -237,11 +238,13 @@ export const RecipientCounter: React.FC<RecipientCounterProps> = ({ newsItemScop
},
)

const throttledGetCounters = useCallback(throttle(getCounters, 1500), [throttle])
Copy link
Member

Choose a reason for hiding this comment

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

Why getCounters not in deps array? To avoid callback rerender?

Copy link
Member

@SavelevMatthew SavelevMatthew Aug 20, 2024

Choose a reason for hiding this comment

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

btw, throttle is imported function, its not necessary to put it in deps

Copy link
Member

Choose a reason for hiding this comment

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

And why 1500? I'm almost sure we can reproduce error consistently with this value

@@ -266,6 +269,9 @@ export const RecipientCounter: React.FC<RecipientCounterProps> = ({ newsItemScop

const isLoadingCounters = isCountersLoading || !counters

console.log('Recipient counter', { isLoadingCounters, isCountersLoading, counters })
Copy link
Member

Choose a reason for hiding this comment

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

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants