Add Pagefind searchbar to apps/web header#2075
Conversation
- Install vite-plugin-pagefind and pagefind dependencies - Configure pagefind plugin in vite.config.ts - Update build script to run pagefind after vite build - Create Search component using Pagefind UI - Add Search component to header - Import Pagefind UI CSS in styles.css Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe pull request introduces Pagefind static site search functionality to the web application. It adds Pagefind as a production and dev dependency, updates build and typecheck scripts to generate search indexes post-build, creates a new Search component that dynamically loads and initializes Pagefind UI, integrates it into the header, and refactors CI detection in the environment configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/components/search.tsx (1)
3-26: Consider adding visual loading state.The component renders an empty div while Pagefind UI loads asynchronously. Users won't see any feedback during the import and initialization, which could feel unresponsive.
Consider adding a loading state:
import { useEffect, useRef, useState } from "react"; export function Search() { const containerRef = useRef<HTMLDivElement>(null); const [isLoading, setIsLoading] = useState(true); useEffect(() => { let instance: any = null; const loadPagefind = async () => { if (!containerRef.current) return; try { const pagefindUI = await import("/pagefind/pagefind-ui.js"); instance = new pagefindUI.PagefindUI({ element: containerRef.current, showSubResults: true, showImages: false, }); setIsLoading(false); } catch { console.debug("Pagefind UI not available yet"); setIsLoading(false); } }; loadPagefind(); return () => { if (instance && typeof instance.destroy === 'function') { instance.destroy(); } }; }, []); return ( <div className="relative"> {isLoading && <div className="animate-pulse">Loading search...</div>} <div ref={containerRef} /> </div> ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/web/package.json(2 hunks)apps/web/src/components/header.tsx(2 hunks)apps/web/src/components/search.tsx(1 hunks)apps/web/src/styles.css(1 hunks)apps/web/vite.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/vite.config.ts
**/*.config.{ts,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Agent configuration should be centralized and externalized from implementation logic
Files:
apps/web/vite.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/vite.config.tsapps/web/src/components/search.tsxapps/web/src/components/header.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:24.348Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/desktop-e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:24.348Z
Learning: Applies to apps/desktop-e2e/**/*.{test,spec}.{js,ts,tsx} : Refer to `scripts/setup-desktop-e2e.sh` for end-to-end test environment setup if setup is missing
Applied to files:
apps/web/package.json
🧬 Code graph analysis (1)
apps/web/src/components/header.tsx (1)
apps/web/src/components/search.tsx (1)
Search(3-26)
🪛 GitHub Actions: web_ci
apps/web/src/styles.css
[error] 1-1: During 'vite build && pagefind --site dist/client' step, PostCSS could not load '/pagefind/pagefind-ui.css' (ENOENT).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fmt
- GitHub Check: Devin
🔇 Additional comments (6)
apps/web/package.json (2)
7-7: LGTM: Build script correctly sequences Vite build and Pagefind indexing.The build script properly runs
vite buildfirst to generate static assets, thenpagefind --site dist/clientto index the built site for search functionality.
80-84: LGTM: Pagefind dependencies added correctly.Both
pagefindCLI andvite-plugin-pagefindare appropriately added to devDependencies with reasonable version constraints.apps/web/src/components/header.tsx (1)
11-11: LGTM: Search component cleanly integrated into header.The Search component is correctly imported and rendered in the desktop navigation. The implementation is straightforward and follows the existing patterns.
Note: The Search component only appears on desktop (within the
sm:flexnav). Consider whether mobile users should also have access to search functionality in the mobile menu (lines 239-373).Also applies to: 180-180
apps/web/src/components/search.tsx (1)
11-11: The dynamic import already has appropriate error handling and gracefully degrades during development.The code at line 11 includes a try-catch block (lines 10-19) that catches import failures and logs a debug message. Since
vite-plugin-pagefindserves pagefind assets through Vite's dev server, the import will fail during development when the pagefind index hasn't been generated yet—but this is expected and handled. The error doesn't crash the application; it simply logs a debug message and renders an empty container. This is the correct pattern for optional features that are only available post-build.apps/web/vite.config.ts (1)
15-18: The pagefind configuration is correct; consider improving plugin ordering.The
assetsDirectory: "public"andoutputDirectory: "dist/client"settings are both correct—they match Vite's standard public directory and your build script target (pagefind --site dist/client). However, placing pagefind as the first plugin is suboptimal per Vite conventions. Since your build script runs indexing via CLI after the build completes (vite build && pagefind --site dist/client), consider either moving the plugin to the end of the array or explicitly settingapply: 'build'to clarify its build-time-only behavior.apps/web/src/styles.css (1)
2-2: CSS import is correct and does not cause a build failure.The
vite-plugin-pagefindplugin does not automatically inject CSS—CSS must be included manually by the developer. The@import url("/pagefind/pagefind-ui.css")at line 2 is the correct approach. CSS@import url()statements are lazy-loaded at runtime by the browser, not during the Vite build step, so this does not block the build process. Since the build script runsvite build && pagefind --site dist/client, the pagefind-ui.css file will exist at/pagefind/pagefind-ui.cssbefore the application is served. The Search component already handles dynamic loading of pagefind-ui.js with error handling, and the current CSS import pattern aligns with standard Pagefind integration. No changes are needed.Likely an incorrect or invalid review comment.
- Move CSS import from styles.css to dynamic loading in Search component - Add cleanup function to destroy PagefindUI instance on unmount - Fixes build error: ENOENT /pagefind/pagefind-ui.css during vite build Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/src/components/search.tsx (1)
21-38: Consider adding debug logging and an unmount guard around the async importTwo small improvements to make this more robust and debuggable:
Log failures in dev-friendly way
Right now thecatch {}is silent; adding aconsole.debughelps explain why search is missing when/pagefind/...404s (as expected in dev) and matches the PR’s stated behavior.Guard against work after unmount
If the component unmounts before theimport()resolves, the code can still try to create aPagefindUIinstance afterward. It’s harmless today (errors are swallowed), but a simple cancellation flag makes the pattern more future-proof.For example:
useEffect(() => { - let pagefindInstance: { destroy?: () => void } | null = null; + let pagefindInstance: { destroy?: () => void } | null = null; + let cancelled = false; const loadPagefind = async () => { - if (!containerRef.current) return; + if (!containerRef.current || cancelled) return; const linkId = "pagefind-ui-css"; if (!document.getElementById(linkId)) { const link = document.createElement("link"); link.id = linkId; link.rel = "stylesheet"; link.href = "/pagefind/pagefind-ui.css"; document.head.appendChild(link); } try { const pagefindPath = "/pagefind/pagefind-ui.js"; const pagefindUI = await import(/* @vite-ignore */ pagefindPath); + if (!containerRef.current || cancelled) return; pagefindInstance = new pagefindUI.PagefindUI({ element: containerRef.current, showSubResults: true, showImages: false, }); - } catch {} + } catch (error) { + console.debug("Pagefind UI not available yet", error); + } }; loadPagefind(); return () => { + cancelled = true; if (pagefindInstance?.destroy) { pagefindInstance.destroy(); } }; }, []);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/web/package.json(2 hunks)apps/web/src/components/search.tsx(1 hunks)apps/web/src/env.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/src/env.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/env.tsapps/web/src/components/search.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote
- GitHub Check: ci
- GitHub Check: fmt
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (2)
apps/web/src/env.ts (1)
4-4: CI flag extraction intoisCIkeeps behavior while improving clarityUsing a shared
isCIconstant forskipValidationis straightforward and preserves the previousprocess.env.CI === "true"semantics while making CI-related checks easier to reuse and reason about.Also applies to: 38-39
apps/web/src/components/search.tsx (1)
1-42: Search component correctly initializes and cleans up Pagefind UIThe hook setup looks solid: you guard on
containerRef.current, inject the CSS only once, dynamically import the UI script, and keep a handle to thePagefindUIinstance sodestroy()runs on unmount. This also addresses the earlier review note about cleanup in Strict Mode / remount scenarios.
Add Pagefind searchbar to apps/web header
Summary
Integrates Pagefind search into the web app header using
vite-plugin-pagefind. The search component is placed in the header navigation area as a temporary location for now (styling/positioning to be refined later per user request).Changes:
pagefindandvite-plugin-pagefinddependenciesdist/clientoutputUpdates since last revision
Fixed CI build failure (
ENOENT: no such file or directory, open '/pagefind/pagefind-ui.css'):@importinstyles.cssto dynamic<link>injection in the Search componentdestroy()call for the PagefindUI instanceReview & Testing Checklist for Human
pnpm -F @hypr/web build) and verify pagefind generates an index indist/client/pagefind/Recommended test plan: Run
pnpm -F @hypr/web build && pnpm -F @hypr/web serve, then open the preview and test the search bar in the header.Notes
Link to Devin run: https://app.devin.ai/sessions/38abf192afc94b029e7b8fa4173accf5
Requested by: yujonglee (@yujonglee)