Conversation
|
@DSanich is attempting to deploy a commit to the NodeTec Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds client-side search UI: new SearchPanel and SearchView components with hydration-aware initialization, relay-backed infinite pagination, local search filtering, author profile resolution, per-item actions (download, broadcast, view JSON), a new /search page, and a Search entry in the sidebar. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant SearchComp as SearchPanel/SearchView
participant Relay as Relay (Feed)
participant Profiles as Profiles Service
participant UI as UI Layer
User->>Browser: open /search or open SearchPanel
Browser->>SearchComp: mount (hydration check)
SearchComp->>Relay: fetch page 1 (useInfiniteQuery)
Relay-->>SearchComp: paginated blog events
SearchComp->>Profiles: request author profiles for visible items
Profiles-->>SearchComp: author avatars/names
SearchComp-->>Browser: render filtered list
User->>SearchComp: scroll -> in-view trigger
SearchComp->>Relay: fetch next page
Relay-->>SearchComp: additional blogs
SearchComp-->>Browser: append items
User->>SearchComp: item action (download/broadcast/view JSON)
SearchComp->>Relay: publish/broadcast (if chosen)
Relay-->>SearchComp: success/error
SearchComp->>UI: show toast / open JSON dialog
UI-->>User: feedback / dialog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@ChristianChiarulli I would be glad to receive your review 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/sidebar/SearchPanel.tsx`:
- Around line 73-79: The query is firing on mount even with an empty search, so
update the useInfiniteQuery call (queryKey ['search-feed', activeRelay], queryFn
fetchBlogs, getNextPageParam) to include the search term in the gating: add a
check that searchQuery is non-empty to the enabled option (e.g., enabled:
isHydrated && !!activeRelay && searchQuery.trim().length > 0) so the query only
runs after the user types; this uses the existing useInfiniteQuery and
fetchBlogs symbols and will cause React Query to refetch automatically when
searchQuery becomes non-empty.
🧹 Nitpick comments (1)
src/components/sidebar/SearchPanel.tsx (1)
95-96: Deduplicate pubkeys before profile fetch.
Multiple blogs from the same author will repeatedly enqueue the same pubkey. De-duping reduces unnecessary profile lookups.♻️ Suggested change
- const pubkeys = filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : []; + const pubkeys = filteredBlogs.length > 0 + ? Array.from(new Set(filteredBlogs.map((blog) => blog.pubkey))) + : [];
| } = useInfiniteQuery({ | ||
| queryKey: ['search-feed', activeRelay], | ||
| queryFn: ({ pageParam }) => fetchBlogs({ limit: 50, until: pageParam, relay: activeRelay }), | ||
| initialPageParam: undefined as number | undefined, | ||
| getNextPageParam: (lastPage) => lastPage.nextCursor, | ||
| enabled: isHydrated && !!activeRelay, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "SearchPanel.tsx" -type fRepository: nodetec/notestack
Length of output: 101
🏁 Script executed:
cat -n ./src/components/sidebar/SearchPanel.tsxRepository: nodetec/notestack
Length of output: 16544
🌐 Web query:
TanStack React Query useInfiniteQuery enabled flag refetch behavior documentation
💡 Result:
useInfiniteQuerysupports the sameenabledoption asuseQuery(its options are “identical to theuseQueryhook” aside from infinite-specific additions). [1]- When
enabled: false:- it won’t auto-fetch on mount
- it won’t auto-refetch in the background
- it will ignore
queryClient.invalidateQueries()/queryClient.refetchQueries()that would normally trigger a refetch - you can still manually fetch via the
refetchfunction returned from the hook. [2]
- When an infinite query refetch happens (once it’s enabled and becomes stale / is refetched), TanStack Query refetches the pages (“each group”) sequentially starting from the first page to avoid stale cursors/duplicates. [3]
- If you’re worried about refetching many pages, you can use
maxPagesso a refetch only refetches up to that many cached pages (also sequentially). [3]
Sources:
[1] TanStack Query v5 useInfiniteQuery reference (options identical to useQuery) (tanstack.com)
[2] TanStack Query “Disabling/Pausing Queries” guide (enabled: false behavior) (tanstack.dev)
[3] TanStack Query v5 “Infinite Queries” guide (sequential refetch; maxPages) (tanstack.com)
Citations:
- 1: https://tanstack.com/query/v5/docs/react/reference/useInfiniteQuery
- 2: https://tanstack.dev/query/latest/docs/framework/react/guides/disabling-queries
- 3: https://tanstack.com/query/v5/docs/framework/react/guides/infinite-queries
Gate the query by search term to avoid unnecessary relay requests on mount.
Currently, the query fetches 50+ items immediately when the component mounts, even though the UI prompts the user to enter a search term first. This wastes relay traffic. Since filteredBlogs already returns an empty array when searchQuery is empty (lines 84–85), add a search term check to the enabled condition to prevent the initial fetch.
When the user types their first character, TanStack React Query will automatically refetch since enabled toggles from false to true.
💡 Suggested change
} = useInfiniteQuery({
queryKey: ['search-feed', activeRelay],
queryFn: ({ pageParam }) => fetchBlogs({ limit: 50, until: pageParam, relay: activeRelay }),
initialPageParam: undefined as number | undefined,
getNextPageParam: (lastPage) => lastPage.nextCursor,
- enabled: isHydrated && !!activeRelay,
+ enabled: isHydrated && !!activeRelay && !!searchQuery.trim(),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } = useInfiniteQuery({ | |
| queryKey: ['search-feed', activeRelay], | |
| queryFn: ({ pageParam }) => fetchBlogs({ limit: 50, until: pageParam, relay: activeRelay }), | |
| initialPageParam: undefined as number | undefined, | |
| getNextPageParam: (lastPage) => lastPage.nextCursor, | |
| enabled: isHydrated && !!activeRelay, | |
| }); | |
| } = useInfiniteQuery({ | |
| queryKey: ['search-feed', activeRelay], | |
| queryFn: ({ pageParam }) => fetchBlogs({ limit: 50, until: pageParam, relay: activeRelay }), | |
| initialPageParam: undefined as number | undefined, | |
| getNextPageParam: (lastPage) => lastPage.nextCursor, | |
| enabled: isHydrated && !!activeRelay && !!searchQuery.trim(), | |
| }); |
🤖 Prompt for AI Agents
In `@src/components/sidebar/SearchPanel.tsx` around lines 73 - 79, The query is
firing on mount even with an empty search, so update the useInfiniteQuery call
(queryKey ['search-feed', activeRelay], queryFn fetchBlogs, getNextPageParam) to
include the search term in the gating: add a check that searchQuery is non-empty
to the enabled option (e.g., enabled: isHydrated && !!activeRelay &&
searchQuery.trim().length > 0) so the query only runs after the user types; this
uses the existing useInfiniteQuery and fetchBlogs symbols and will cause React
Query to refetch automatically when searchQuery becomes non-empty.
|
hey i haven't really been paying attention to PRs lately, you might have to update it to work with the new layout. Also I'm not sure how efficient search really is on nostr at the moment to the point that i hesitate to call it search. I haven't read it over yet but how can you search through titles and summaries if they aren't single letter tags? |
|
@ChristianChiarulli hello! Thanks for your message! Yes, you're right about the limitations. My current implementation is more like filtering already loaded articles. Since title and summary aren't indexed as searchable tags in Nostr, and NIP-50 isn't widely supported, this provides a way to filter through the articles visible in the feed. I can either:
I also made some corrections according to the new design in the commit: c36c65f |
DSanich
force-pushed
the
feat/154-implement-search
branch
from
bc9091d to
d17d968
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/sidebar/SearchPanel.tsx`:
- Around line 80-92: The current filteredBlogs useMemo is ineffective because
allBlogs (created by flatMap) is a new array each render; move the flattening
into the memo so that useMemo depends on the stable React Query result (data)
and searchQuery instead of allBlogs. Modify the useMemo for filteredBlogs to
compute const allBlogs = data?.pages.flatMap(...) inside the memo and list
[data, searchQuery] as dependencies, preserving the existing search logic (trim,
toLowerCase checks against blog.title and blog.summary) and keep the
filteredBlogs variable name and useMemo wrapper.
- Around line 204-208: The "No articles found" message appears even when more
pages may exist; update the JSX in SearchPanel.tsx (the block using searchQuery,
isLoading, and filteredBlogs) to consider hasNextPage: if filteredBlogs is empty
and hasNextPage is true, either trigger loading of more pages (call the existing
pagination function such as fetchNextPage/loadMore) or show a softened message
like "No loaded articles match \"{searchQuery}\" — more results may be
available, load more" instead of the definitive "No articles found"; if
hasNextPage is false keep the original definitive message. Ensure you reference
and use the existing pagination flag/function (hasNextPage and the project's
load-more function) rather than adding new network logic.
🧹 Nitpick comments (3)
src/components/sidebar/SearchPanel.tsx (3)
94-95: Memoizepubkeysto avoid unnecessary profile refetches.
pubkeysis a new array reference on every render, which likely triggers re-fetches or re-subscriptions insideuseProfiles. Wrap it inuseMemokeyed onfilteredBlogs.Suggested fix
- const pubkeys = filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : []; + const pubkeys = useMemo( + () => (filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : []), + [filteredBlogs], + );
230-230: Usecursor-pointeron interactive elements.Both the blog item (line 230) and the author span (line 248) use
cursor-defaultdespite being clickable (role="button"). This makes them feel non-interactive. Switch tocursor-pointer.Also applies to: 248-248
143-146: Consider naming this panel "Filter" rather than "Search" per the PR discussion.The PR discussion notes that this is client-side filtering of loaded articles, not a true search. The maintainer raised concerns about calling it "search." The heading on line 151 says "Search" and the placeholder says "Search articles..." — consider renaming to "Filter" to set accurate user expectations.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/sidebar/SearchView.tsx`:
- Around line 191-201: The two result-messaging branches can both render because
the "no results" check uses !isLoading but doesn't exclude error state; update
the conditional that renders the "No articles found matching ..." block (which
uses searchQuery, isLoading, and filteredBlogs) to also require !isError (or
otherwise short-circuit when isError is true) so only the error message from the
isError branch (which uses searchQuery and isError) is shown when a load error
occurs.
🧹 Nitpick comments (6)
src/app/search/page.tsx (1)
6-6:SearchViewlives undersidebar/but is the main content of a dedicated page.The component at
src/components/sidebar/SearchView.tsxis rendered as the primary page body here, not as a sidebar panel. Consider relocating it to a more appropriate directory (e.g.,src/components/search/SearchView.tsx) to better reflect its role and keep thesidebar/directory focused on sidebar-specific components. As per coding guidelines, "UI and editor surface code should be organized insrc/components/, witheditor/,sidebar/, and sharedui/atoms subdirectories."src/components/sidebar/SearchView.tsx (5)
74-86:useMemoforfilteredBlogsis effectively a no-op —allBlogsis a new array reference every render.
allBlogson line 74 is derived viaflatMapwhich produces a new array on each render, so theuseMemodependency[allBlogs, searchQuery]always sees a changed reference and recomputes unconditionally.Stabilize the input by memoizing
allBlogsitself, or usedata(the react-query result object, which is referentially stable when unchanged) as the dependency:♻️ Proposed fix
- const allBlogs = data?.pages.flatMap((page) => page.blogs) ?? []; - - const filteredBlogs = useMemo(() => { - if (!searchQuery.trim()) { - return []; - } - const query = searchQuery.toLowerCase(); - return allBlogs.filter((blog) => { - const titleMatch = blog.title.toLowerCase().includes(query); - const summaryMatch = blog.summary?.toLowerCase().includes(query); - return titleMatch || summaryMatch; - }); - }, [allBlogs, searchQuery]); + const allBlogs = useMemo( + () => data?.pages.flatMap((page) => page.blogs) ?? [], + [data], + ); + + const filteredBlogs = useMemo(() => { + if (!searchQuery.trim()) { + return []; + } + const query = searchQuery.toLowerCase(); + return allBlogs.filter((blog) => { + const titleMatch = blog.title.toLowerCase().includes(query); + const summaryMatch = blog.summary?.toLowerCase().includes(query); + return titleMatch || summaryMatch; + }); + }, [allBlogs, searchQuery]);
88-89:pubkeysarray recreated every render triggers unnecessary downstream work.Because
filteredBlogsis unstable (see above), thepubkeysarray is also a fresh reference each render. After stabilizingallBlogs/filteredBlogs, also memoizepubkeysto avoid passing a new array touseProfileson every render:♻️ Proposed fix
- const pubkeys = filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : []; + const pubkeys = useMemo( + () => (filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : []), + [filteredBlogs], + );
57-72: Data is fetched eagerly regardless of whether the user has typed a query.The infinite query fires as soon as
isHydrated && !!activeRelay, pre-loading 50 articles even before the user types anything. This may be an intentional preload strategy (to make results feel instant), but it's worth noting that the user may never search — and each page fetches 50 items, which is relatively large. If this is intentional, consider adding a brief comment documenting the design choice; otherwise, gating on!!searchQuery.trim()inenabledwould defer the cost.
30-41: Utility functions could be shared.
formatDateandtruncateNpublook like general-purpose helpers. IfSearchPanel.tsx(or other components) define similar helpers, consider moving them to a shared utility module (e.g.,src/lib/utils/format.ts).
43-341: Extract shared feed logic into a custom hook to eliminate duplication.Both
SearchPanel.tsxandSearchView.tsxduplicate the infinite-loading feed, search filtering, profile resolution, and event handlers (handleBroadcast,handleViewJson). The identical utility functions (formatDate,truncateNpub) should also be centralized. The primary differences are navigation strategies (callbacks vs. router-based) and styling, which can be parameterized. Consider creating a shareduseSearchFeedhook or similar to manage query logic, filtering, and handlers, allowing both components to focus on layout and navigation specifics.
|
I think a better way to implement search might be with https://antiprimal.net/ I just got counts working with it and it seems pretty solid |
@ChristianChiarulli Ok, thanks! I will do everything in my power. |
|
@ChristianChiarulli I've been thinking a lot and trying to implement a solution, but in practice, the NIP-50 offered by Antiprimal only allows searching by standard Nostr events, such as, for example, kind 1 (short notes). Long notes (articles) used in the application aren't eligible for this search; their kind is 30023. Relay doesn't support this. So, the current solution of filtering already cached articles is the only viable one. What do you think about this? |
DSanich
force-pushed
the
feat/154-implement-search
branch
from
c36c65f to
bd1ad77
Compare
DSanich
force-pushed
the
feat/154-implement-search
branch
from
bd1ad77 to
6b93222
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/sidebar/SearchPanel.tsx`:
- Around line 94-95: The pubkeys array is recreated every render causing
useProfiles to refetch; wrap its creation in useMemo and deduplicate entries so
the reference stays stable when contents don't change. Replace the direct const
pubkeys = ... with a memoized version like const pubkeys = useMemo(() =>
Array.from(new Set(filteredBlogs.map(b => b.pubkey))), [filteredBlogs]) so
useProfiles(pubkeys, relays) only reruns when the actual pubkey list changes.
🧹 Nitpick comments (2)
src/components/sidebar/SearchPanel.tsx (2)
49-344: Substantial duplication withSearchView.tsx— extract shared logic.Per the AI summary and relevant snippets,
SearchView.tsx(lines 42–341) implements nearly identical functionality: same infinite query setup, same filtering logic, same per-item actions (download, broadcast, view JSON), same profile resolution, and same item rendering. The two components share ~250+ lines of duplicated code.Consider extracting:
- A shared
useSearchFeedhook encapsulating the infinite query, filtering, and profile resolution.- A shared
BlogListItemcomponent for the per-item rendering (avatar, title, summary, thumbnail, actions dropdown).This would keep
SearchPanel(sidebar overlay) andSearchView(page body) as thin layout wrappers.
54-54:unknown | nullsimplifies tounknownsincenullis a subset ofunknown.Suggested fix
- const [jsonEvent, setJsonEvent] = useState<unknown | null>(null); + const [jsonEvent, setJsonEvent] = useState<unknown>(null);
| const pubkeys = filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : []; | ||
| const { isLoading: isLoadingProfiles, isFetching: isFetchingProfiles, getProfile } = useProfiles(pubkeys, relays); |
There was a problem hiding this comment.
pubkeys array is unstable — triggers useProfiles refetch every render.
Because filteredBlogs recomputes on every render (due to the allBlogs issue above), this pubkeys array is also a new reference each time, which will cause useProfiles to re-run its query logic unnecessarily. Once you fix the useMemo dependency (use data instead of allBlogs), this stabilizes transitively. However, you should also memoize pubkeys itself to avoid triggering profile fetches when only the array reference changes but the content is the same.
Suggested fix
- const pubkeys = filteredBlogs.length > 0 ? filteredBlogs.map((blog) => blog.pubkey) : [];
+ const pubkeys = useMemo(
+ () => (filteredBlogs.length > 0 ? [...new Set(filteredBlogs.map((b) => b.pubkey))] : []),
+ [filteredBlogs],
+ );This also deduplicates pubkeys, avoiding redundant profile fetches for authors with multiple articles.
🤖 Prompt for AI Agents
In `@src/components/sidebar/SearchPanel.tsx` around lines 94 - 95, The pubkeys
array is recreated every render causing useProfiles to refetch; wrap its
creation in useMemo and deduplicate entries so the reference stays stable when
contents don't change. Replace the direct const pubkeys = ... with a memoized
version like const pubkeys = useMemo(() => Array.from(new
Set(filteredBlogs.map(b => b.pubkey))), [filteredBlogs]) so useProfiles(pubkeys,
relays) only reruns when the actual pubkey list changes.
2 participants
Add a dedicated search panel to find articles by title and summary.
Changes:
fix #154
Summary by CodeRabbit