-
Notifications
You must be signed in to change notification settings - Fork 200
fix: search and clear search behaviour in search query component #2668
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
base: main
Are you sure you want to change the base?
fix: search and clear search behaviour in search query component #2668
Conversation
Console (appwrite/console)Project ID: Sites (1)
Tip Realtime gives you live updates for users, storage, functions, and databases |
WalkthroughThis change refactors the synchronization logic in the search query component by introducing additional state variables ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/components/searchQuery.svelte (1)
60-86: Well-structured two-effect approach to break the feedback loop.The separation of concerns is clear:
- Effect 1 (lines 61-70): Handles external URL changes by syncing to input
- Effect 2 (lines 73-86): Handles user typing by syncing to URL via debounce
The key to preventing the feedback loop is updating both
inputValueandpreviousInputValuetogether in Effect 1 (lines 66-67), so Effect 2's guard conditioninputValue !== previousInputValueevaluates to false when the change originated from a URL update.Optional consideration: Effect 1 reads
inputValueon line 65, creating a dependency that causes this effect to run on every keystroke (though theurlSearch !== previousUrlSearchguard prevents any action). If performance becomes a concern, you could restructure to avoid this dependency:$effect(() => { const urlSearch = page.url.searchParams.get('search') ?? ''; if (urlSearch !== previousUrlSearch) { + const currentInput = inputValue; // read once, but still creates dependency previousUrlSearch = urlSearch; - if (urlSearch !== inputValue) { + if (urlSearch !== currentInput) { inputValue = urlSearch; previousInputValue = urlSearch; } } });However, given the guard condition, the current implementation is correct and the overhead is negligible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/components/searchQuery.svelte(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/lib/components/searchQuery.svelte
src/lib/components/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure
Files:
src/lib/components/searchQuery.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/lib/components/searchQuery.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/lib/components/searchQuery.svelte
🔇 Additional comments (1)
src/lib/components/searchQuery.svelte (1)
26-29: Good state initialization pattern.Capturing the initial URL state and using three tracking variables to distinguish user input changes from URL-driven changes is a clean approach to prevent the feedback loop described in the PR.

What does this PR do?
Fixes a bug in the
SearchQuerycomponent where the search input field and "Clear Search" button were not working correctly.The Problem:
Root Cause:
The component used a single
$effectthat watched bothinputValueandpage.url.searchParams, which created a feedback loop. When the URL changed (e.g., Clear Search removed?search=), the effect would detect a mismatch and callrunSearch(inputValue)again, re-adding the old search value to the URL.The Solution:
searchparameter changes externally (Clear Search, navigation), update the input valueBefore Fix:
After Fix:
Related PRs and Issues
#2656
Have you read the Contributing Guidelines on issues?
Yes, I have read the Contributing Guidelines.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.