Skip to content

Conversation

@jaysomani
Copy link
Contributor

@jaysomani jaysomani commented Dec 3, 2025

What does this PR do?

Fixes a bug in the SearchQuery component where the search input field and "Clear Search" button were not working correctly.

The Problem:

  • When users typed in the search field, sometimes the input would become unresponsive or not update properly
  • When users clicked the "Clear Search" button, the search would briefly clear but then immediately re-apply the old search term, causing a "flash" effect where results would appear and then disappear again
  • The search input value would not sync properly when the URL changed externally (e.g., via Clear Search button or browser navigation)

Root Cause:
The component used a single $effect that watched both inputValue and page.url.searchParams, which created a feedback loop. When the URL changed (e.g., Clear Search removed ?search=), the effect would detect a mismatch and call runSearch(inputValue) again, re-adding the old search value to the URL.

The Solution:

  • Separated the sync logic into two distinct effects with proper tracking:
    1. URL → Input sync: When the URL search parameter changes externally (Clear Search, navigation), update the input value
    2. Input → URL sync: When the user types, update the URL (with debounce)
  • Added previous value tracking to distinguish between user-initiated changes and external URL changes, preventing the feedback loop

Before Fix:

  • ❌ Clear Search would flash results then disappear
  • ❌ Search input sometimes became unresponsive
  • ❌ Old search value would re-appear after clearing

After Fix:

  • ✅ Clear Search works correctly and stays cleared
  • ✅ Search input is fully responsive
  • ✅ No feedback loops or flashing results

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

  • Bug Fixes
    • Improved search input synchronization to prevent unintended update loops and ensure consistent behavior between typed queries and URL parameters.

✏️ Tip: You can customize this high-level summary in your review settings.

@appwrite
Copy link

appwrite bot commented Dec 3, 2025

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Failed Failed Authorize Preview URL QR Code

Tip

Realtime gives you live updates for users, storage, functions, and databases

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

This change refactors the synchronization logic in the search query component by introducing additional state variables (initialSearch, inputValue, previousInputValue, previousUrlSearch) to track input and URL states separately. The original single synchronization effect is replaced with two distinct effects: one for URL-to-input synchronization and one for input-to-URL synchronization. Guard conditions are added to prevent bidirectional feedback loops between the URL and input state, while maintaining existing debounce cleanup behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • State management complexity: The introduction of multiple tracking variables (previousInputValue, previousUrlSearch) requires careful verification that all state transitions are correct and consistent.
  • Synchronization logic: The separation into two distinct effects with conditional guards needs thorough review to ensure feedback loops are genuinely prevented and no edge cases are missed.
  • Interaction between effects: Special attention should be paid to how the two effects interact and whether the order of execution could cause issues under various user interactions and URL changes.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: search and clear search behaviour in search query component' directly aligns with the PR's main objective of fixing search and clear search behavior bugs in the SearchQuery component.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 inputValue and previousInputValue together in Effect 1 (lines 66-67), so Effect 2's guard condition inputValue !== previousInputValue evaluates to false when the change originated from a URL update.

Optional consideration: Effect 1 reads inputValue on line 65, creating a dependency that causes this effect to run on every keystroke (though the urlSearch !== previousUrlSearch guard 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

📥 Commits

Reviewing files that changed from the base of the PR and between 20874f5 and 93708e7.

📒 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.

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.

1 participant