mozilla / perfcompare

Improved Performance Comparison Tool
Mozilla Public License 2.0
39 stars 93 forks source link

Properly debounce the search function, and increment the ID #671

Closed julienw closed 4 months ago

julienw commented 4 months ago

There was 2 issues:

  1. The search function and its debounce version were being recreated at each render. As a result it wasn't debounced and a new request was being sent for each keystroke, albeit after half a second.
  2. The ID used to know if the received response is for latest search wasn't incremented at the right place. As a result some reponses weren't ignored where they should have been.

This first commit fixes both issues, by using useCallback and incrementing the search id in the right place.

The second commit is a small correctness issue where handleEscKeypress wasn't memoized. I think it wasn't doing any problem but it's more consistent with the other function handleDocumentMousedown.

Finally the 3rd commit fixes a bunch of issues with the existing test for the SearchView. The biggest issue (IMO) is that the mock was always returning all the data, because it wasn't configured in the correct order. I thought the library would order it by specificity but it's not doing it.

netlify[bot] commented 4 months ago

Deploy Preview for mozilla-perfcompare ready!

Name Link
Latest commit a5898c5987276ba0ad369c281248cf10c3598adf
Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/6662c5084f07420008cb9241
Deploy Preview https://deploy-preview-671--mozilla-perfcompare.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.80%. Comparing base (1f9f726) to head (67b42a6).

:exclamation: Current head 67b42a6 differs from pull request most recent head a5898c5

Please upload reports for the commit a5898c5 to get more accurate results.

Files Patch % Lines
src/components/Search/SearchInputAndResults.tsx 94.73% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## beta #671 +/- ## ========================================== - Coverage 91.80% 91.80% -0.01% ========================================== Files 68 68 Lines 1587 1586 -1 Branches 285 285 ========================================== - Hits 1457 1456 -1 Misses 104 104 Partials 26 26 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.