feat: denoise search results #751
Conversation
MikeShi42
commented
Apr 14, 2025
MikeShi42
commented
|
MikeShi42
force-pushed
the
mikeshi/v0-event-patterns
branch
from
3f14f88 to
fee91c9
Compare
|
@MikeShi42 when u get a chance, can you resolve these conflicts in the PR? I'll give it another thorough review when these are resolved. Thanks! |
MikeShi42
force-pushed
the
mikeshi/denoise-search
branch
from
8789854 to
f8cb7cc
Compare
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| setIsLive(false); | ||
| } | ||
| }, | ||
| [setIsLive], | ||
| [isLive, setIsLive], |
There was a problem hiding this comment.
This just an optimization or does it fix a bug?
There was a problem hiding this comment.
good q, I don't actually recall why this diff is here since the commit was a while back, at best vaguely recalling it's likely trying to fix an excess re-render issue.
| > | ||
| {shouldShowLiveModeHint && | ||
| analysisMode === 'results' && | ||
| denoiseResults != true && ( |
There was a problem hiding this comment.
I think a bool comparison here is good
| bodyValueExpression: patternColumn ?? '', | ||
| totalCount: undefined, | ||
| enabled: denoiseResults, | ||
| }); |
There was a problem hiding this comment.
Just a comment for later: If we notice performance issues here, we could move this to a webworker
There was a problem hiding this comment.
there's a few other issues we likely want to fix, I think it re-imports packages at times for some reason but I'm not too worried since none of these behaviors come around unless the user is activating the denoise
| import { | ||
| FetchNextPageOptions, | ||
| keepPreviousData, | ||
| QueryClient, |
There was a problem hiding this comment.
keepPreviousData also unused
