Skip to content

Add debounce for generating swap routes#621

Closed
kkpagaev wants to merge 2 commits intosoroswap:mainfrom
kkpagaev:fix/add-debounce-for-swap-routes
Closed

Add debounce for generating swap routes#621
kkpagaev wants to merge 2 commits intosoroswap:mainfrom
kkpagaev:fix/add-debounce-for-swap-routes

Conversation

@kkpagaev
Copy link

Fix too many request when using inputs in swap form

@vercel
Copy link

vercel bot commented Apr 28, 2025

@kkpagaev is attempting to deploy a commit to the PaltaLabs OU Team on Vercel.

A member of the Team first needs to authorize it.

@MattPoblete MattPoblete requested a review from Copilot April 29, 2025 13:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds a debounce mechanism for generating swap routes to minimize the frequency of API requests when the swap form inputs change. Key changes include:

  • Importing and using lodash-es's debounce function.
  • Wrapping the generateRoute function with debounce via useCallback.
  • Adjusting the formatting of assetIn/assetOut properties for clarity.

};

return { generateRoute, resetRouterSdkCache, maxHops };
const generateRouteDebounce = useCallback(debounce(generateRoute, 500), [
Copy link

Copilot AI Apr 29, 2025

Choose a reason for hiding this comment

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

Consider including 'generateRoute' in the dependency array of useCallback to ensure that changes to its implementation are captured.

Copilot uses AI. Check for mistakes.
@MattPoblete
Copy link
Collaborator

This fix actually do more requests than the deployed app (372 vs 231), please fix
Captura de pantalla 2025-04-29 a la(s) 11 23 30 a m
Captura de pantalla 2025-04-29 a la(s) 11 22 19 a m

@kkpagaev
Copy link
Author

@MattPoblete Hi, we have been emailing Esteban on upwork for a few days now and haven't heard back from him.

It would be great if he would reply.

I'm duplicating the last text here:

I just wanted to clarify something, as there might be a small misunderstanding. We’ve resolved the “Too Many Simultaneous Requests” issue as outlined in the assessment document.
However, during the review, a different issue was mentioned. We’re aware of that one too, but it wasn’t part of the specific scope we initially agreed on.

Initially you wanted to see how we work, to evaluate our approach, on the solution of the first task, I would like to get your feedback on this.

We’re ready to move forward and assist further, just let us know if everything is on track and what the next steps are.

Looking forward to your feedback!

@vercel
Copy link

vercel bot commented Apr 30, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 30, 2025 8:48pm

@esteblock
Copy link
Member

I also dont understand what this PR is solving.
The current situation is when I change an amount in the form I get
image

And with this PR I get
image

...
And whrn I change asset:
Current situation:
image

with this PR
image

@esteblock
Copy link
Member

Also, when forcing an output (exact out), when I change the value
Current situation
image

This PR
image

@kkpagaev
Copy link
Author

kkpagaev commented May 1, 2025

In this PR, we addressed the issue where:

  • An API request was triggered on every input change.
  • After our changes, this no longer happens because we added a debounce. Now, there's a short delay before the API is called - we wait until the user finishes typing before making the request.

The issues you’ve described in conversation in this PR do not directly relate to our test task:
"Too Many Simultaneous Requests (Inputting values triggers too many RPC/API calls. Causes lag and API)".

  1. The fact that the page makes around 4 requests on load - we’ve seen that as well and are ready to work on it within a separate task.
  2. On asset selection (that mentioned here - Add debounce for generating swap routes #621 (comment)) is also outside the scope of this particular test task.

Additionally, based on your screenshots, we only see the results of the initial page load - not the behavior when interacting with the input field.

We've prepared two short videos demonstrating the behavior before and after our changes.
In both videos, we entered three zeroes into the swap amount input. You can observe the corresponding network activity in the browser's network tab.

Screencast.From.2025-05-01.13-19-04.mp4

This shows the behavior before any changes were applied. As you can see, entering three zeroes triggers three separate requests to the /split endpoint.

new.mp4

This shows the behavior after the changes were implemented. Performing the same action now results in only a single request to the /split endpoint.

Please review our PR again and confirm that the reported issue has indeed been resolved.

Looking forward to your feedback - thank you!

@MattPoblete @esteblock

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants