Feature/search bar by N3k1ta · Pull Request #11 · exil-solidare/nextjs-frontend

N3k1ta · 2024-12-12T21:32:13Z

Optimize search functionality and add spiner to SearchBar

uramanovich · 2024-12-17T07:06:44Z


 const nextConfig: NextConfig = {
-  /* config options here */
+  async rewrites() {


Why do we need this? Initially danswer plan to be public and we can just use public URL

Sorry, this was something I tried to set up initially, and I forgot to remove it later.

uramanovich · 2024-12-17T07:07:19Z

+
+import SearchBar from "./SearchBar";
+
+export default function Header() {


nit: let's use arrow functions over export default to be consistent over codebase

Okay, I'll update this and in SearchBar.tsx as well for consistency in the code.

uramanovich · 2024-12-17T07:08:11Z

@@ -0,0 +1,15 @@
+import { useEffect, useState } from "react";
+
+export function useDebounce<T>(value: T, delay: number): T {


Do you know what T means here?

Yes, is a reference type for our function. It's the same type that we should have on the return. We are doing this for more reusability in our code.

uramanovich · 2024-12-17T07:09:07Z

+    setError(null);
+
+    try {
+      const response = await fetch(


Let's abstract the logic of usage danswer.
What do you think about moving it to the new service like I did for notion? You can check notion.service

I moved the search logic to the fetchDanswerData function in the srs/services folder as you requested. However, I couldn't find notion.service, so I implemented everything based on my understanding.

uramanovich · 2024-12-17T07:09:48Z

+      }));
+
+      setSearchResults(results);
+    } catch (err: any) {


By default all errors in typescript is unknown and it's a better type rather than any. Do you know why?

Suggested change

} catch (err: any) {

} catch (err) {

I had thought about this before. I know that we should avoid using the 'any' type because we won't have control over the object type that we get. But somehow I left it as is. I'll redo it now.

uramanovich · 2024-12-17T07:11:29Z

+  const [isLoading, setIsLoading] = useState(false);
+  const [error, setError] = useState<string | null>(null);
+
+  const debouncedQuery = useDebounce(searchQuery, 500);


Let's not use magic numbers. Let's move this to a variable outside of hook (because it static) and made it const

FYI: A magic number/string is a normal string or number that doesn't have a descriptive name and stands as a symbol.

I added a constant:
const DEBOUNCE_DELAY = 500;
outside the hook.

uramanovich · 2024-12-17T07:11:59Z

+        throw new Error(`No results found for "${searchQuery}"`);
+      }
+      const data = await response.json();
+      const results = searchResultSchema.parse(data).documents.map((doc) => ({


Move this logic of mapping in helper as it could simplify logic of hook

Also I suggest to do maping and parsing in 2 different steps, this simplify reading of the code

I created a file named mapDocuments.ts and placed it in the /helpers folder.
I implemented the .map function there with the proper types and added this helper function to useSearch.ts.

uramanovich · 2024-12-17T07:13:03Z

+    }
+  };
+
+  if (debouncedQuery !== previousQuery.current) {


Why do we need this if we already debounce query?

Regarding your comment, we have this logic inside if (debouncedQuery !== previousQuery.current) because, previously, we used useEffect for triggering the search when the query changed, but it was requested to remove it. By moving this logic here, we ensure that the search only triggers when the debounced query changes, and we still avoid unnecessary renders that would occur if we used useEffect. It's essentially a manual debounce implementation, which ensures better performance and avoids unnecessary re-renders while keeping the debounce logic. I hope that clears it up!

uramanovich · 2024-12-17T07:15:28Z

+
+  return {
+    searchQuery,
+    setSearchQuery,


I suggest to you now to share "setSeachQuery" and "setSearchResults" but instead share logical functions like "onClear" and "onQueryChange". In the end when you use this hook you need:

get query for search

perform change of query

receive results

have possibility to clear results

… service and helper functions; updated Zod schema

N3k1ta added 4 commits December 10, 2024 00:52

Added search bar component

3b99134

Fixed input alignment with search icon

18f1a1a

Added reset for "setError" to "Clear" button

8bce145

Optimize search functionality and add spiner to SearchBar

1601553

uramanovich reviewed Dec 17, 2024

View reviewed changes

Refactor search logic: moved data fetching and processing to separate…

0c4192e

… service and helper functions; updated Zod schema


		import SearchBar from "./SearchBar";

		export default function Header() {

		@@ -0,0 +1,15 @@
		import { useEffect, useState } from "react";

		export function useDebounce<T>(value: T, delay: number): T {

Uh oh!

Conversation

N3k1ta commented Dec 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants