Skip to content

Add frontend for search#116

Closed
Pleb5 wants to merge 1 commit into02-17-demo_1cc066bb_add_server_apifrom
02-17-demo_812dd918_add_frontend_for_search
Closed

Add frontend for search#116
Pleb5 wants to merge 1 commit into02-17-demo_1cc066bb_add_server_apifrom
02-17-demo_812dd918_add_frontend_for_search

Conversation

@Pleb5
Copy link
Copy Markdown
Owner

@Pleb5 Pleb5 commented Feb 17, 2026

No description provided.

Copy link
Copy Markdown
Owner Author

Pleb5 commented Feb 17, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Pleb5 Pleb5 mentioned this pull request Feb 17, 2026
@Pleb5 Pleb5 marked this pull request as ready for review February 17, 2026 12:31
@Pleb5 Pleb5 force-pushed the 02-17-demo_1cc066bb_add_server_api branch from 94f2524 to 1a000cd Compare February 17, 2026 12:39
@Pleb5 Pleb5 force-pushed the 02-17-demo_812dd918_add_frontend_for_search branch from c6b9f31 to a0793e6 Compare February 17, 2026 12:39
Comment on lines +9 to +26
useEffect(() => {
setLoading(true);
fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
.then(response => {
if (!response.ok) {
throw new Error('Network response was not ok');
}
return response.json();
})
.then(data => {
setTasks(data);
setLoading(false);
})
.catch(error => {
setError(error.message);
setLoading(false);
});
}, [searchQuery]); // Depend on searchQuery
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Race condition: Multiple fetch requests can be in flight simultaneously as the user types, and responses may arrive out of order. If a user types "cat" then quickly types "s" to make "cats", the response for "cat" might arrive after "cats", displaying wrong results.

Fix by implementing cleanup in useEffect:

useEffect(() => {
  const abortController = new AbortController();
  setLoading(true);
  fetch(`/search?query=${encodeURIComponent(searchQuery)}`, {
    signal: abortController.signal
  })
    .then(response => {
      if (!response.ok) throw new Error('Network response was not ok');
      return response.json();
    })
    .then(data => {
      setTasks(data);
      setLoading(false);
    })
    .catch(error => {
      if (error.name !== 'AbortError') {
        setError(error.message);
        setLoading(false);
      }
    });
  return () => abortController.abort();
}, [searchQuery]);
Suggested change
useEffect(() => {
setLoading(true);
fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
.then(response => {
if (!response.ok) {
throw new Error('Network response was not ok');
}
return response.json();
})
.then(data => {
setTasks(data);
setLoading(false);
})
.catch(error => {
setError(error.message);
setLoading(false);
});
}, [searchQuery]); // Depend on searchQuery
useEffect(() => {
const abortController = new AbortController();
setLoading(true);
fetch(`/search?query=${encodeURIComponent(searchQuery)}`, {
signal: abortController.signal
})
.then(response => {
if (!response.ok) throw new Error('Network response was not ok');
return response.json();
})
.then(data => {
setTasks(data);
setLoading(false);
})
.catch(error => {
if (error.name !== 'AbortError') {
setError(error.message);
setLoading(false);
}
});
return () => abortController.abort();
}, [searchQuery]); // Depend on searchQuery

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +22 to +25
.catch(error => {
setError(error.message);
setLoading(false);
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error state persists across requests. If a request fails and sets an error, then the user types again triggering a new successful request, the old error remains set. After the successful request completes (loading=false), line 32-34 will still display the error message instead of the results.

Fix by clearing error state when starting a new request:

useEffect(() => {
  setLoading(true);
  setError(null); // Clear previous error
  fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
  // ... rest of the code
}, [searchQuery]);

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +9 to +26
useEffect(() => {
setLoading(true);
fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
.then(response => {
if (!response.ok) {
throw new Error('Network response was not ok');
}
return response.json();
})
.then(data => {
setTasks(data);
setLoading(false);
})
.catch(error => {
setError(error.message);
setLoading(false);
});
}, [searchQuery]); // Depend on searchQuery
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Performance issue: Every single keystroke triggers an immediate API request. If a user types "javascript" (10 characters), this generates 10 separate API calls. In production, this will cause excessive server load, potential rate limiting, and poor performance.

Fix by implementing debouncing:

useEffect(() => {
  const timeoutId = setTimeout(() => {
    setLoading(true);
    fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
      .then(response => {
        if (!response.ok) throw new Error('Network response was not ok');
        return response.json();
      })
      .then(data => {
        setTasks(data);
        setLoading(false);
      })
      .catch(error => {
        setError(error.message);
        setLoading(false);
      });
  }, 300); // 300ms debounce
  
  return () => clearTimeout(timeoutId);
}, [searchQuery]);
Suggested change
useEffect(() => {
setLoading(true);
fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
.then(response => {
if (!response.ok) {
throw new Error('Network response was not ok');
}
return response.json();
})
.then(data => {
setTasks(data);
setLoading(false);
})
.catch(error => {
setError(error.message);
setLoading(false);
});
}, [searchQuery]); // Depend on searchQuery
useEffect(() => {
const timeoutId = setTimeout(() => {
setLoading(true);
fetch(`/search?query=${encodeURIComponent(searchQuery)}`)
.then(response => {
if (!response.ok) {
throw new Error('Network response was not ok');
}
return response.json();
})
.then(data => {
setTasks(data);
setLoading(false);
})
.catch(error => {
setError(error.message);
setLoading(false);
});
}, 300); // 300ms debounce
return () => clearTimeout(timeoutId);
}, [searchQuery]); // Depend on searchQuery

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@Pleb5 Pleb5 mentioned this pull request Feb 17, 2026
@Pleb5 Pleb5 deleted the branch 02-17-demo_1cc066bb_add_server_api February 18, 2026 09:50
@Pleb5 Pleb5 closed this Feb 18, 2026
@Pleb5 Pleb5 deleted the 02-17-demo_812dd918_add_frontend_for_search branch February 18, 2026 09:50
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