fix: migration app router from pages router #180
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
app/_components/theme-initializer.tsxOops! Something went wrong! :( ESLint: 8.26.0 TypeError [ERR_INVALID_ARG_TYPE]: Error while loading rule '@next/next/no-html-link-for-pages': The "path" argument must be of type string. Received undefined WalkthroughThis pull request represents a comprehensive migration of the Yorkie homepage from Next.js Pages Router to the App Router (version 13+). The changes span across multiple files, involving structural modifications to the application's routing, metadata management, and component rendering. Key transformations include replacing Changes
Assessment against linked issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (19)
components/Tabs/Tab.tsx (1)
Line range hint
22-29: Enhance accessibility attributesWhile the component implements basic ARIA attributes, it should include additional accessibility features for better screen reader support.
<button data-active={isActive || undefined} type="button" role="tab" id={ctx.getTabId(value)} + aria-selected={isActive} + aria-controls={ctx.getTabPanelId(value)} + tabIndex={isActive ? 0 : -1} onClick={activateTab} {...others} >components/Accordion/AccordionControl.tsx (2)
Line range hint
8-10: Consider extending interface with ARIA attributesThe AccordionControl props interface should include ARIA-specific properties for better accessibility.
export interface AccordionControlProps extends React.ComponentPropsWithoutRef<'button'> { children?: React.ReactNode; + 'aria-expanded'?: boolean; + 'aria-controls'?: string; }
Line range hint
20-33: Add essential ARIA attributes for accessibilityThe accordion button needs proper ARIA attributes to be fully accessible.
<button {...restProps} ref={ref} className={classNames('accordion_btn', { is_active: isActive }, className)} onClick={() => { ctx.onChange(value); }} type="button" data-active={isActive || undefined} + aria-expanded={isActive} + aria-controls={`accordion-content-${value}`} > {iconPosition === 'left' && icon} {children} {iconPosition === 'right' && icon} </button>hooks/useOutsideClick.ts (1)
Line range hint
11-12: Improve type safety by avoiding 'any' typesThe hook uses 'any' type for refs which could lead to runtime errors. Consider using proper TypeScript types.
-export const useOutsideClick = (ref: any, callback: (event: MouseEvent) => void, ...excludedRefs: any[]): void => { +export const useOutsideClick = ( + ref: React.RefObject<HTMLElement>, + callback: (event: MouseEvent) => void, + ...excludedRefs: React.RefObject<HTMLElement>[] +): void => {components/Layout/ExampleLayout.tsx (2)
Line range hint
32-54: Missing view type switching UI.The component accepts different view types but doesn't provide UI controls to switch between them. Consider implementing a view type selector in the header navigation.
<nav className="nav"> <h1 className="logo"> <Link href="/"> <LogoGnbSVG /> </Link> <span className="blind">Yorkie</span> </h1> <span className="nav_text"> <Link href="/examples">Examples</Link> </span> <span className="nav_text">{breadcrumbTitle}</span> + <div className="view-controls"> + {viewTypes.map((type) => ( + <button + key={type} + onClick={() => setViewType(type)} + className={viewType === type ? 'active' : ''} + > + {exampleViewTypeMap[type].label} + </button> + ))} + </div> </nav>
Implement view switching UI or remove unused view-related code
The verification reveals that while the component:
- Accepts
viewTypesprop and uses it inwebtoons-page.tsx- Has state management for
viewTypewithuseState- Passes the
viewTypeto children- Has a header section where view switching UI could be implemented
However:
- The
exampleViewTypeMapis defined but never used- There's no UI implementation for switching between view types in the header
- The view switching functionality is incomplete despite the infrastructure being in place
You should either:
- Implement the view switching UI using the defined
exampleViewTypeMapandviewTypes- Or remove the unused
exampleViewTypeMapif view switching is not needed🔗 Analysis chain
Line range hint
8-15: Remove or utilize unused constants and props.The
exampleViewTypeMapandviewTypesprop are defined but not used in the component. Either implement the view switching functionality or remove these unused elements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if viewTypes or exampleViewTypeMap are used elsewhere rg "exampleViewTypeMap" --type ts rg "viewTypes.*=.*\[.*\]" --type tsLength of output: 377
Script:
#!/bin/bash # Let's check the implementation of ExampleLayout to see if these props are actually used ast-grep --pattern 'const ExampleLayout = ({ $$$, viewTypes = $_,$$$}) => { $$$ }' # Also check for any view type related UI elements or functions rg "viewType" -A 5 --type ts components/Layout/ExampleLayout.tsxLength of output: 724
Script:
#!/bin/bash # Let's check if there's any UI implementation for view switching in the component rg -B 5 -A 10 "header_example" --type ts components/Layout/ExampleLayout.tsx # Also check if exampleViewTypeMap is used within the header ast-grep --pattern 'header className="header_example"> $$$ </header>'Length of output: 659
app/community/community-page.tsx (1)
Line range hint
1-42: Add metadata configuration for the community page.With the migration to App Router, page metadata should be configured using the new metadata API instead of the removed Head component.
Add a metadata configuration file:
+ // app/community/page.tsx + import type { Metadata } from 'next' + + export const metadata: Metadata = { + title: 'Community · Yorkie', + description: 'Join our community and get help', + }components/Popover/Popover.tsx (1)
Line range hint
63-77: Consider adding ARIA attributes for accessibility.The popover should be properly labeled for screen readers.
return ( <PopoverContextProvider value={{ opened: _opened, targetRef, dropdownRef, onToggle: _onToggle, onOpen: _onOpen, onClose: _onClose, + ariaLabel, + ariaExpanded: _opened }} > {children} </PopoverContextProvider> );components/Accordion/Accordion.tsx (1)
Line range hint
31-92: Add ARIA attributes for accessibilityWhile the implementation is solid, it's missing essential accessibility attributes.
return ( <AccordionContextProvider value={{ isItemActive, onChange: handleItemChange, iconPosition, icon, }} > - <ul className="accordion_list">{children}</ul> + <div + role="region" + aria-label="Accordion" + className="accordion_list" + > + {children} + </div> </AccordionContextProvider> );components/Layout/ThemeDropdown.tsx (2)
Line range hint
35-47: Simplify event handling logicThe click handler could be simplified and made more maintainable.
-onClick={(e) => { - const target = (e.target as Element).closest('.dropdown_item'); - if (!target) return; - const option = target.getAttribute('data-option') as ThemeOption; - if (themeOption !== option) { - setThemeOption(option); - setTheme(option); - } - setDropdownOpened(false); -}} +onClick={(e) => { + const option = (e.target as Element).closest<HTMLElement>('[data-option]')?.dataset.option as ThemeOption; + if (!option) return; + + if (themeOption !== option) { + setThemeOption(option); + setTheme(option); + } + setDropdownOpened(false); +}}
Line range hint
19-33: Add keyboard navigation supportThe theme dropdown should support keyboard navigation for accessibility.
<Popover.Target> - <button type="button" className="btn btn_small"> + <button + type="button" + className="btn btn_small" + aria-haspopup="true" + aria-expanded={dropdownOpened} + aria-label="Theme selector" + > <span className="filter_title">Theme:</span> <span className="text">{themeOption.replace(/^[a-z]/, (char) => char.toUpperCase())}</span> <Icon type="arrow" /> </button> </Popover.Target>app/examples/tldraw/tldraw-page.tsx (1)
Line range hint
56-62: Validate environment variables and add type safetyWith Next.js App Router, consider adding runtime validation for environment variables and improving type safety.
+const validateEnvVars = () => { + if (!process.env.NEXT_PUBLIC_API_ADDR) { + throw new Error('NEXT_PUBLIC_API_ADDR is not defined'); + } + if (!process.env.NEXT_PUBLIC_EXAMPLES_API_KEY) { + throw new Error('NEXT_PUBLIC_EXAMPLES_API_KEY is not defined'); + } +}; const TldrawExampleView: NextPage = () => { + validateEnvVars(); + return ( // ... existing code ... <BasicExampleView - rpcAddr={process.env.NEXT_PUBLIC_API_ADDR || ''} - apiKey={process.env.NEXT_PUBLIC_EXAMPLES_API_KEY || ''} + rpcAddr={process.env.NEXT_PUBLIC_API_ADDR} + apiKey={process.env.NEXT_PUBLIC_EXAMPLES_API_KEY} documentKey={exampleKey} iframeURL={EXAMPLE_PREVIEW_URL + exampleKey} userMaxCount={30}components/exampleView/Sidebar/Sidebar.tsx (1)
Line range hint
63-82: Improve accessibility for interactive elementsThe share button and GitHub link need accessibility improvements.
<div className={classNames('btn_box', { full_width: !ctx.wide })}> <a href={codeURL} className="btn gray600" target="_blank" rel="noreferrer" + aria-label="View source code on GitHub" > GitHub </a> {shareButton && ( <a - href="#" + href="#share" + onClick={(e) => { + e.preventDefault(); + // Add share functionality + }} className="btn gray900 btn_share" + aria-label="Share to invite others" > - Share to invite other + Share to invite others </a> )} </div>components/exampleView/BasicView/ProjectCodes.tsx (2)
Line range hint
42-54: Add error handling to file click handlerThe
onClickFilehandler should include error handling for cases where the file might not exist or be inaccessible.Consider this enhancement:
const onClickFile = useCallback( (targetFile: string) => { + if (!targetFile) { + console.error('Invalid file path'); + return; + } setFileInfo((prev) => { const [directoryInfo, openedFileInfo] = setFileOpen(prev, targetFile); + if (!openedFileInfo) { + console.error(`Failed to open file: ${targetFile}`); + return prev; + } setActiveFileInfo(openedFileInfo); return directoryInfo; }); }, [setFileInfo], );
Line range hint
56-150: Reduce code duplication in folder rendering logicThe folder rendering logic is duplicated between the main component and
SubFolderCodes. Consider extracting common functionality into a shared component.Consider creating a shared
FolderItemcomponent:type FolderItemProps = { item: CodeInfo; onClickFile: (path: string) => void; isOpen: boolean; onToggleFolder: () => void; }; function FolderItem({ item, onClickFile, isOpen, onToggleFolder }: FolderItemProps) { return ( <li className={classNames('folder_item', { is_active: item.isFile && item.isOpen })}> <button type="button" className="btn_folder" onClick={() => { if (item.isFile) { onClickFile(item.path); } else { onToggleFolder(); } }} > {item.isFile ? ( <Icon type="file" /> ) : isOpen ? ( <Icon type="folderOpen" /> ) : ( <Icon type="folderClose" /> )} <span className="name">{item.name}</span> </button> {!item.isFile && isOpen && <SubFolderCodes fileList={item.children} onClickFile={onClickFile} />} </li> ); }components/exampleView/BasicView/BasicExampleView.tsx (2)
Line range hint
32-57: Improve error handling in Yorkie client initializationThe Yorkie client initialization lacks proper error handling and cleanup logic.
Consider this enhancement:
useEffect(() => { let unsubscribeDoc: Function; + let client: yorkie.Client; const activate = async () => { - const client = new yorkie.Client(rpcAddr, { apiKey }); - await client.activate(); - const doc = new yorkie.Document(documentKey); - await client.attach(doc); + try { + client = new yorkie.Client(rpcAddr, { apiKey }); + await client.activate(); + const doc = new yorkie.Document(documentKey); + await client.attach(doc); - setDocChangeInfos((prev) => [...prev, { type: 'initialize', content: 'Connection has been established!' }]); + setDocChangeInfos((prev) => [...prev, { type: 'initialize', content: 'Connection has been established!' }]); + unsubscribeDoc = doc.subscribe((event) => { + if (event.type === 'remote-change') { + const { operations } = event.value; + for (const op of operations) { + setDocChangeInfos((prev) => [...prev, { type: 'update', content: op.path }]); + } + } + }); + } catch (error) { + console.error('Failed to initialize Yorkie client:', error); + setDocChangeInfos((prev) => [...prev, { type: 'initialize', content: 'Failed to establish connection!' }]); + } }; activate(); return () => { - if (unsubscribeDoc) unsubscribeDoc(); + const cleanup = async () => { + if (unsubscribeDoc) unsubscribeDoc(); + if (client) { + try { + await client.deactivate(); + } catch (error) { + console.error('Failed to cleanup Yorkie client:', error); + } + } + }; + cleanup(); }; }, [rpcAddr, documentKey, apiKey]);
Line range hint
89-108: Improve user management logicThe user management logic could be more robust with validation and error handling.
Consider this enhancement:
const addUser = useCallback(() => { + if (!Array.isArray(userList)) { + console.error('Invalid userList state'); + return; + } + if (userList.length === userMaxCount) { - alert("You can't add more users"); + console.warn(`Maximum user count (${userMaxCount}) reached`); + return; + } + + const nextUserId = userList.length + 1; + if (nextUserId > userMaxCount) { + console.warn(`Cannot add user ${nextUserId}: exceeds maximum count`); return; } - if (userList.length === userList[userList.length - 1]) { - setUserList((prev) => [...prev, userList.length + 1]); + + // Find the first available user ID + const usedIds = new Set(userList); + let newId = 1; + while (usedIds.has(newId) && newId <= userMaxCount) { + newId++; + } + + if (newId > userMaxCount) { + console.warn('No available user IDs'); return; } - for (let i = 0; i < userList.length; i++) { - if (userList[i] !== i + 1) { - setUserList((prev) => [...prev.slice(0, i), i + 1, ...prev.slice(i)]); - return; - } - } + + setUserList((prev) => [...prev, newId]); }, [setUserList, userList, userMaxCount]);components/exampleView/DualView.tsx (2)
Line range hint
45-46: Security concern: Hardcoded iframe URLThe iframe URL is hardcoded and could pose security risks.
Consider:
- Using environment variables for the URL
- Adding sandbox attributes
- Implementing Content Security Policy (CSP)
- <iframe title="Example" frameBorder={0} src="https://yorkie.dev" width="100%" height="100%"></iframe> + <iframe + title="Example" + frameBorder={0} + src={process.env.NEXT_PUBLIC_EXAMPLE_URL} + sandbox="allow-scripts allow-same-origin" + width="100%" + height="100%" + ></iframe>
Line range hint
54-67: Improve state management and type safetyThe pin list state management could be more robust with proper TypeScript types.
Consider this enhancement:
+type UserId = 'user1' | 'user2' | 'user3' | 'user4'; + export function DualView() { - const [pinList, setPinList] = useState<Array<string>>(['user1']); + const [pinList, setPinList] = useState<Array<UserId>>(['user1']); - const handlePin = (user: string) => { + const handlePin = (user: UserId) => { if (pinList.includes(user)) { setPinList(pinList.filter((item) => item !== user)); return; } if (pinList.length === 2) { + console.warn('Maximum number of pins reached'); return; } setPinList([...pinList, user]); };
🧹 Nitpick comments (44)
next.config.js (1)
14-16: Environment variables correctly typed as stringsGood change converting boolean values to strings, as environment variables are always strings in Node.js. However, consider adding runtime validation for these values.
Consider adding validation:
const nextConfig = { env: { + // Validate quality is between 0-100 + nextImageExportOptimizer_quality: + process.env.NEXT_IMAGE_QUALITY || '75', // ... other env vars }, }package.json (1)
Line range hint
1-53: Consider stricter version managementThe extensive use of caret ranges (
^) across dependencies could lead to unexpected updates. For a production website, consider:
- Using exact versions or tilde ranges (
~) for more predictable builds- Adding
enginesfield to enforce Node.js version requirementAdd the following to package.json:
{ + "engines": { + "node": ">=18.0.0" + }, "name": "homepage", ... }components/Layout/Header.tsx (1)
Line range hint
35-40: Consider using startsWith for more robust path matchingThe exact equality comparison with pathname might be fragile for nested routes. Consider using
startsWithfor more robust path matching.-<li className={`gnb_item ${pathname == '/products' ? 'is_active' : ''}`}> +<li className={`gnb_item ${pathname?.startsWith('/products') ? 'is_active' : ''}`}>components/Layout/MobileGnbDropdown.tsx (1)
37-37: Consider extracting path matching logicThe pathname checking logic is repeated throughout the component. Consider extracting it into a utility function for better maintainability.
+const isActivePath = (pathname: string | null, path: string, exact = false) => { + if (!pathname) return false; + const cleanPath = pathname.split('#')[0]; + return exact ? cleanPath === path : cleanPath.startsWith(path); +}; -className={classNames('dropdown_menu', { is_active: pathname?.split('#')[0] === '/products' })} +className={classNames('dropdown_menu', { is_active: isActivePath(pathname, '/products', true) })}Also applies to: 64-64, 74-74, 84-84, 94-94, 104-104, 114-114, 124-124, 134-134, 144-144, 156-156, 164-164, 176-176, 188-188, 198-198
app/docs/[[...slug]]/page.tsx (1)
87-90: Improve type safety when accessing TOC childrenCasting
children[0]toanycan mask type errors and lead to runtime issues. To enhance type safety, consider properly typing thetoc.childrenarray and handling cases wherechildren[0]or itschildrenproperty may be undefined.app/page.tsx (1)
8-8: Remove unnecessaryasynckeyword from thePagecomponentThe
Pagecomponent does not perform any asynchronous operations. Theasynckeyword can be removed to improve clarity and avoid confusion.Apply this diff:
-export default async function Page() { +export default function Page() { return <HomePage />; }app/products/page.tsx (1)
8-8: Remove unnecessaryasynckeyword from thePagecomponentSince the
Pagecomponent does not perform any asynchronous operations, it's unnecessary to declare it asasync. Removing theasynckeyword can enhance readability.Apply this diff:
-export default async function Page() { +export default function Page() { return <ProductPage />; }app/examples/page.tsx (1)
8-10: Consider adding error boundary for client components.While the implementation is correct, consider wrapping the client component with an error boundary to handle potential rendering errors gracefully.
export default async function Page() { - return <ExamplesPage />; + return ( + <ErrorBoundary fallback={<ErrorFallback />}> + <ExamplesPage /> + </ErrorBoundary> + ); }app/not-found.tsx (1)
4-6: Consider enhancing SEO metadata for the 404 page.While the basic metadata is correct, consider adding more SEO-friendly metadata properties for better search engine handling of the 404 page.
export const metadata: Metadata = { title: 'Page not found · Yorkie', + description: 'The page you are looking for could not be found.', + robots: 'noindex, follow', };app/examples/webtoons/page.tsx (1)
8-10: Consider removing unnecessary async keywordThe
Pagecomponent doesn't appear to perform any asynchronous operations. Unless there are plans to add data fetching, the async keyword can be removed.-export default async function Page() { +export default function Page() { return <WebtoonsPage />; }app/examples/quill/page.tsx (1)
4-8: LGTM! Good use of constant for title.Extracting the example title into a constant improves maintainability. However, for simple string concatenation, consider using string concatenation instead of template literal.
- title: `${exampleTitle} · Yorkie Examples`, + title: exampleTitle + ' · Yorkie Examples',app/examples/tldraw/page.tsx (2)
10-12: Consider removing unnecessaryasynckeywordThe
Pagecomponent doesn't contain any asynchronous operations, making theasynckeyword unnecessary.-export default async function Page() { +export default function Page() { return <TldrawPage />; }
6-8: Consider enhancing metadata for SEOWhile the title is properly set, consider adding description and other metadata fields for better SEO.
export const metadata: Metadata = { title: `${exampleTitle} · Yorkie Examples`, + description: 'Interactive Tldraw example powered by Yorkie', + openGraph: { + title: `${exampleTitle} · Yorkie Examples`, + description: 'Interactive Tldraw example powered by Yorkie', + }, };app/examples/calendar/page.tsx (1)
10-12: Consider removing unnecessaryasynckeyword.The Page component is marked as async but doesn't perform any asynchronous operations. Consider simplifying to a regular component:
-export default async function Page() { +export default function Page() { return <CalendarPage />; }app/examples/simultaneous-cursors/page.tsx (1)
1-12: Consider creating a shared page template for examples.Since all example pages follow the same pattern, consider creating a shared template or higher-order component to reduce duplication and standardize the implementation:
// app/examples/_components/create-example-page.tsx import { Metadata } from 'next'; interface ExamplePageProps { title: string; Component: React.ComponentType; } export function createExamplePage({ title, Component }: ExamplePageProps) { const metadata: Metadata = { title: `${title} · Yorkie Examples`, }; function Page() { return <Component />; } return { metadata, Page }; } // Usage in example pages: // app/examples/calendar/page.tsx import { createExamplePage } from '../_components/create-example-page'; import CalendarPage from './calendar-page'; export const { metadata, Page } = createExamplePage({ title: 'Calendar', Component: CalendarPage, }); export default Page;This approach would:
- Reduce code duplication
- Ensure consistent metadata formatting
- Make it easier to modify the page structure for all examples at once
- Simplify the creation of new example pages
components/Tabs/Tabs.context.ts (2)
Line range hint
1-16: LGTM! Consider adding JSDoc comments.The context implementation is well-structured with proper typing and error handling. The 'use client' directive is correctly placed for Next.js App Router compatibility.
Consider adding JSDoc comments to document the context interface and its methods:
+/** + * Context for managing tab state and interactions + */ interface TabsContext { + /** Current active tab value */ value: TabsValue; + /** Callback for tab change events */ onTabChange(value: TabsValue): void; + /** Generates unique ID for tab elements */ getTabId(value: string): string; + /** Generates unique ID for panel elements */ getPanelId(value: string): string; }
Line range hint
7-12: Consider adding type safety for tab values.To improve type safety and prevent potential runtime errors, consider using a union type or enum for possible tab values instead of the generic
TabsValuetype.export type TabsValue = 'tab1' | 'tab2' | 'tab3'; // Or use an enumcomponents/Accordion/Accordion.context.ts (1)
Line range hint
1-16: LGTM! Consider improving icon prop typing.The context implementation is well-structured and follows the same robust pattern as the Tabs context.
Consider using a more specific type for the icon prop:
interface AccordionContext { iconPosition: AccordionIconPosition; - icon: React.ReactNode; + icon: React.ReactElement | null; onChange(value: string): void; isItemActive(value: string): boolean; }This change would ensure that only valid React elements (or null) can be passed as icons, preventing potential runtime issues with invalid values.
utils/createSafeContext.tsx (1)
Line range hint
5-25: Consider enhancing error messages and adding display names.The implementation is well-structured and type-safe. Consider these improvements:
- Add more context to error messages
- Add display names for better debugging
export function createSafeContext<ContextValue>(errorMessage: string) { const Context = createContext<ContextValue | null>(null); + Context.displayName = 'SafeContext'; const useSafeContext = () => { const ctx = useContext(Context); if (ctx === null) { - throw new Error(errorMessage); + throw new Error(`Context Provider is missing: ${errorMessage}`); } return ctx; }; const Provider = ({ children, value }: { value: ContextValue; children: React.ReactNode }) => ( <Context.Provider value={value}>{children}</Context.Provider> ); + Provider.displayName = 'SafeContext.Provider'; return [Provider, useSafeContext] as const; }components/Tabs/TabsPanel.tsx (1)
Line range hint
14-29: Consider enhancing accessibility and style handling.While the implementation is solid, consider these improvements:
- Add more ARIA attributes for better accessibility
- Move inline styles to CSS classes for better maintainability
export const TabsPanel = forwardRef<HTMLDivElement, TabsPanelProps>(({ value, children, ...others }, ref) => { const ctx = useTabsContext(); const active = ctx.value === value; return ( <div {...others} ref={ref} role="tabpanel" + aria-labelledby={ctx.getTabId(value)} + aria-hidden={!active} id={ctx.getPanelId(value)} - style={{ display: !active ? 'none' : undefined }} + className={`tabs-panel ${!active ? 'tabs-panel--hidden' : ''}`} > {children} </div> ); });Add corresponding CSS:
.tabs-panel--hidden { display: none; }app/docs/[[...slug]]/slug-page.tsx (1)
8-16: Consider extracting props interfaceFor better type safety and reusability, consider extracting the props interface.
interface DocsPageProps { content: string; navList: DocsOrderList; children: React.ReactNode; } export default function DocsPage({ content, navList, children }: DocsPageProps)hooks/useUncontrolled.ts (2)
Line range hint
18-24: Improve type safety and default value handlingThe current implementation could benefit from more explicit type safety and default value handling.
export function useUncontrolled<T>({ value, defaultValue, finalValue, - onChange = () => {}, + onChange, }: UseUncontrolledInput<T>): [T, (value: T) => void, boolean] { - const [uncontrolledValue, setUncontrolledValue] = useState(defaultValue !== undefined ? defaultValue : finalValue); + const [uncontrolledValue, setUncontrolledValue] = useState<T>(() => { + if (defaultValue !== undefined) return defaultValue; + if (finalValue !== undefined) return finalValue; + throw new Error('Either defaultValue or finalValue must be provided'); + });
Line range hint
25-27: Add error handling for onChange callbackThe current implementation doesn't handle potential errors in the onChange callback.
const handleUncontrolledChange = (val: T) => { setUncontrolledValue(val); - onChange?.(val); + try { + onChange?.(val); + } catch (error) { + console.error('Error in onChange handler:', error); + } };hooks/useClipboard.ts (2)
Line range hint
6-9: Consider adding type safety for the timeout parameterWhile the implementation is solid, we could enhance type safety for the timeout parameter.
-export function useClipboard({ timeout = 2000 } = {}) { +interface UseClipboardOptions { + timeout?: number; +} +export function useClipboard({ timeout = 2000 }: UseClipboardOptions = {}) {
Line range hint
17-27: Consider adding fallback for older browsersWhile the clipboard API check is good, consider adding a fallback using document.execCommand for broader browser support.
const copy = (valueToCopy: string) => { if ('clipboard' in navigator) { navigator.clipboard .writeText(valueToCopy) .then(() => handleCopyResult(true)) .catch((err) => setError(err)); } else { + try { + const textArea = document.createElement('textarea'); + textArea.value = valueToCopy; + document.body.appendChild(textArea); + textArea.select(); + document.execCommand('copy'); + document.body.removeChild(textArea); + handleCopyResult(true); + } catch (err) { + setError(err instanceof Error ? err : new Error('Copy failed')); + } - setError(new Error('useClipboard: navigator.clipboard is not supported')); } };hooks/useOutsideClick.ts (1)
Line range hint
16-29: Consider performance optimization for event handlingThe current implementation checks every click. Consider using event delegation and possibly debouncing for better performance.
useEffect(() => { + const debounceTimeout = 100; + let timeoutId: NodeJS.Timeout; + const handleClickOutside = (event: MouseEvent) => { + if (timeoutId) { + clearTimeout(timeoutId); + } + + timeoutId = setTimeout(() => { const target = event.target as HTMLElement; if (ref.current?.contains(target)) { return; } for (const excludedRef of excludedRefs) { if (excludedRef.current?.contains(target)) { return; } } callBackRef.current && callBackRef.current(event); + }, debounceTimeout); }; document.addEventListener('click', handleClickOutside); return (): void => { document.removeEventListener('click', handleClickOutside); + if (timeoutId) { + clearTimeout(timeoutId); + } }; }, [ref, callback, excludedRefs]);components/exampleView/Sidebar/SidebarTabs.tsx (1)
Line range hint
8-31: Consider adding error boundaries and memoization.While the component structure is well-organized, consider these improvements:
- Wrap the component with an error boundary to gracefully handle runtime errors
- Memoize the context value to prevent unnecessary re-renders
+ import { useMemo } from 'react'; export function SidebarTabs({ defaultTab, onTabChange, children, }: { defaultTab: string; children: React.ReactNode; onTabChange?(value: TabsValue): void; }) { const [activeTab, setActiveTab] = useState<TabsValue>(defaultTab); + const contextValue = useMemo(() => ({ activeTab }), [activeTab]); return ( <SidebarTabsContextProvider - value={{ - activeTab, - }} + value={contextValue} >app/not-found-page.tsx (1)
Line range hint
7-45: Consider adding metadata for the 404 page.While the Head component was correctly removed, consider adding metadata using the App Router's metadata API.
// Add to the same file or in a separate metadata.ts export const metadata = { title: 'Page not found · Yorkie', description: 'The page you are looking for might be removed or is temporarily unavailable.', };components/Popover/Popover.tsx (1)
Line range hint
9-23: Enhance type safety and accessibility.While the implementation is solid, consider these improvements:
+ import { AriaAttributes } from 'react'; + + interface PopoverProps extends AriaAttributes { + opened?: boolean; + defaultOpened?: boolean; + children: ReactNode; + closeOnClickOutside?: boolean; + onChange?(opened: boolean): void; + onOpen?: () => void; + onClose?: () => void; + } + - export function Popover({ + export function Popover({ + 'aria-label': ariaLabel, + 'aria-expanded': ariaExpanded, opened, defaultOpened, children, closeOnClickOutside = true, onChange, onOpen, onClose, - }: { - opened?: boolean; - defaultOpened?: boolean; - children: ReactNode; - closeOnClickOutside?: boolean; - onChange?(opened: boolean): void; - onOpen?: () => void; - onClose?: () => void; - }) + }: PopoverProps)app/examples/codemirror/codemirror-page.tsx (1)
Line range hint
1-50: Consider implementing error boundaries for client componentsSince this is now a client component, consider implementing error boundaries to gracefully handle runtime errors in the CodeMirror integration.
+'use client'; + +import { Component, ErrorInfo, ReactNode } from 'react'; + +class ErrorBoundary extends Component< + { children: ReactNode }, + { hasError: boolean } +> { + constructor(props: { children: ReactNode }) { + super(props); + this.state = { hasError: false }; + } + + static getDerivedStateFromError(_: Error) { + return { hasError: true }; + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo) { + console.error('CodeMirror error:', error, errorInfo); + } + + render() { + if (this.state.hasError) { + return <div>Something went wrong loading the editor.</div>; + } + return this.props.children; + } +} + const CodemirrorExampleView: NextPage = () => { return ( + <ErrorBoundary> <ExampleLayout breadcrumbTitle={exampleTitle}> {/* ... existing code ... */} </ExampleLayout> + </ErrorBoundary> ); };app/examples/calendar/calendar-page.tsx (1)
Line range hint
42-47: Consider adding environment variable validationWhile the environment variable fallbacks are good, consider adding runtime validation to ensure the required configuration is present.
+const validateEnvVars = () => { + const required = { + NEXT_PUBLIC_API_ADDR: process.env.NEXT_PUBLIC_API_ADDR, + NEXT_PUBLIC_EXAMPLES_API_KEY: process.env.NEXT_PUBLIC_EXAMPLES_API_KEY, + }; + + const missing = Object.entries(required) + .filter(([_, value]) => !value) + .map(([key]) => key); + + if (missing.length > 0) { + console.error(`Missing required environment variables: ${missing.join(', ')}`); + } +}; + +// Call this in a useEffect or before rendering +validateEnvVars();app/layout.tsx (1)
49-58: Consider moving Google Analytics to a separate componentThe Google Analytics script setup could be moved to a dedicated component for better maintainability and reusability.
Create a new component
app/_components/analytics.tsx:'use client'; export default function Analytics() { return ( <> <Script src="https://www.googletagmanager.com/gtag/js?id=G-7KXWLDH8CH" strategy="afterInteractive" /> <Script id="google-analytics" strategy="afterInteractive"> {` window.dataLayer = window.dataLayer || []; function gtag(){window.dataLayer.push(arguments);} gtag('js', new Date()); gtag('config', 'G-7KXWLDH8CH'); `} </Script> </> ); }app/examples/quill/quill-page.tsx (1)
Line range hint
13-14: Consider moving example configuration to a separate fileThe example configuration constants could be moved to a separate file for better maintainability.
Create
app/examples/quill/config.ts:export const EXAMPLE_CONFIG = { key: 'vanilla-quill', title: 'Quill', } as const;components/Accordion/Accordion.tsx (1)
Line range hint
10-29: Consider enhancing type safety and documentationThe type system is well-structured, but could benefit from some improvements:
- Consider making
AccordionValuetype exported for reuse- Add JSDoc comments for
AccordionIconPositiontype+/** Position of the accordion icon relative to the content */ export type AccordionIconPosition = 'left' | 'right'; +/** Value type for accordion state based on multiple selection mode */ +export type AccordionValue<Multiple extends boolean> = Multiple extends true ? string[] : string | null;app/examples/kanban/kanban-page.tsx (2)
Line range hint
16-17: Address TODO comment regarding document structureThe TODO comment indicates pending work on the document structure view.
Would you like me to help implement the document structure view or create a GitHub issue to track this task?
Line range hint
22-24: Improve environment check implementationConsider moving the environment check to a constant or configuration file for better maintainability.
+const isDevelopment = process.env.NODE_ENV === 'development'; + <Sidebar.TabsList> <Sidebar.TabsTab value="code">Code</Sidebar.TabsTab> - {process.env.NODE_ENV === 'development' && ( + {isDevelopment && ( <Sidebar.TabsTab value="documentStructure">Document Structure</Sidebar.TabsTab> )} </Sidebar.TabsList>components/Layout/ThemeDropdown.tsx (1)
Line range hint
12-17: Optimize localStorage access and theme initializationConsider using a single useEffect for initialization and adding error handling for localStorage.
useEffect(() => { - const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption; - setThemeOption(themeOption); + try { + const storedTheme = window.localStorage.getItem('theme'); + if (storedTheme && ['system', 'light', 'dark'].includes(storedTheme)) { + setThemeOption(storedTheme as ThemeOption); + } + } catch (error) { + console.error('Failed to access localStorage:', error); + } }, [setTheme]);app/examples/todomvc/todomvc-page.tsx (1)
Line range hint
1-61: Consider implementing error boundaries for client componentsSince this is now a client component, consider implementing error boundaries to gracefully handle runtime errors.
+'use client'; + +import { ErrorBoundary } from '@/components/ErrorBoundary'; + const TodoListExampleView: NextPage = () => { return ( + <ErrorBoundary fallback={<div>Something went wrong. Please try again.</div>}> <ExampleLayout breadcrumbTitle={exampleTitle}> {() => ( // ... existing code ... )} </ExampleLayout> + </ErrorBoundary> ); };components/exampleView/Sidebar/Sidebar.tsx (1)
Line range hint
18-28: Optimize useEffect dependency and state initializationThe current effect setup can be simplified by using the default state initialization.
-const [isOpened, setIsOpened] = useState<boolean>(defaultOpened); +const [isOpened, setIsOpened] = useState(() => defaultOpened); -useEffect(() => { - setIsOpened(defaultOpened); -}, [defaultOpened]);app/examples/webtoons/webtoons-page.tsx (1)
1-2: Consider extracting shared example page logicThis component shares significant code with
multi-cursor-page.tsx. Consider creating a shared higher-order component or custom hook.Example implementation of a shared hook:
// hooks/useExamplePage.ts export function useExamplePage(defaultTab = 'about') { const [activeTab, setActiveTab] = useState<string | null>(defaultTab); return { activeTab, setActiveTab, // Add other shared logic here }; }components/exampleView/BasicView/ProjectCodes.tsx (1)
Line range hint
15-40: Consider enhancing type safety and state managementThe current implementation could benefit from:
- Using TypeScript's discriminated unions for better type safety
- Consolidating related state into a single reducer
Consider this refactor:
- const [fileInfo, setFileInfo] = useState<DirectoryInfo>({ - isFile: false, - name: '', - path: '', - children: [], - }); - const [activeFileInfo, setActiveFileInfo] = useState<FileInfo | null>( - activeFile ? getFileInfo(files, activeFile) : null, - ); - const [isFolderOpen, setIsFolderOpen] = useState<boolean>(true); + type State = { + fileInfo: DirectoryInfo; + activeFile: FileInfo | null; + folderStates: Record<string, boolean>; + }; + + const [state, dispatch] = useReducer(reducer, { + fileInfo: { + isFile: false, + name: '', + path: '', + children: [], + }, + activeFile: activeFile ? getFileInfo(files, activeFile) : null, + folderStates: {}, + });components/exampleView/DualView.tsx (1)
Line range hint
156-157: Improve accessibility: Non-English text in aria-labelThe "blind" class contains non-English text which should be translated for accessibility.
- <span className="blind">화면 추가하기</span> + <span className="blind">Add screen</span>app/products/product-page.tsx (1)
179-185: Consider using Next.js Link component for external linksFor consistency with Next.js best practices, consider using the Link component for external links with the appropriate configuration.
- <a - href="https://github.com/yorkie-team/yorkie/blob/main/design/garbage-collection.md" - className="link" - > - Garbage Collection - </a> + <Link + href="https://github.com/yorkie-team/yorkie/blob/main/design/garbage-collection.md" + className="link" + target="_blank" + rel="noopener noreferrer" + > + Garbage Collection + </Link>app/home-page.tsx (1)
Line range hint
184-212: Consider adding ARIA labels for better accessibility.The CodeBlockWrapper implementation looks good, but the code copy button could benefit from accessibility improvements.
Consider adding ARIA labels to improve accessibility:
<CodeBlockHeader.RightBox> - <CodeBlockHeader.CopyButton value={activeFeatureCode.info.code} /> + <CodeBlockHeader.CopyButton + value={activeFeatureCode.info.code} + aria-label={`Copy ${activeFeatureCode.info.title} code`} + /> </CodeBlockHeader.RightBox>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (66)
.github/workflows/deploy.yml(4 hunks)app/_components/theme-initializer.tsx(1 hunks)app/community/community-page.tsx(1 hunks)app/community/page.tsx(1 hunks)app/docs/[[...slug]]/page.tsx(1 hunks)app/docs/[[...slug]]/slug-page.tsx(1 hunks)app/docs/_components/scroll-position-indicator.tsx(1 hunks)app/examples/calendar/calendar-page.tsx(1 hunks)app/examples/calendar/page.tsx(1 hunks)app/examples/codemirror/codemirror-page.tsx(1 hunks)app/examples/codemirror/page.tsx(1 hunks)app/examples/example-page.tsx(1 hunks)app/examples/kanban/kanban-page.tsx(1 hunks)app/examples/kanban/page.tsx(1 hunks)app/examples/multi-cursor/multi-cursor-page.tsx(1 hunks)app/examples/multi-cursor/page.tsx(1 hunks)app/examples/page.tsx(1 hunks)app/examples/profile-stack/page.tsx(1 hunks)app/examples/profile-stack/profile-stack-page.tsx(1 hunks)app/examples/quill/page.tsx(1 hunks)app/examples/quill/quill-page.tsx(1 hunks)app/examples/simultaneous-cursors/page.tsx(1 hunks)app/examples/simultaneous-cursors/simultaneous-cursors-page.tsx(1 hunks)app/examples/tldraw/page.tsx(1 hunks)app/examples/tldraw/tldraw-page.tsx(1 hunks)app/examples/todomvc/page.tsx(1 hunks)app/examples/todomvc/todomvc-page.tsx(1 hunks)app/examples/webtoons/page.tsx(1 hunks)app/examples/webtoons/webtoons-page.tsx(1 hunks)app/home-page.tsx(3 hunks)app/layout.tsx(1 hunks)app/not-found-page.tsx(1 hunks)app/not-found.tsx(1 hunks)app/page.tsx(1 hunks)app/products/page.tsx(1 hunks)app/products/product-page.tsx(4 hunks)components/Accordion/Accordion.context.ts(1 hunks)components/Accordion/Accordion.tsx(1 hunks)components/Accordion/AccordionControl.tsx(1 hunks)components/CodeBlock/CodeBlock.tsx(2 hunks)components/Layout/ExampleLayout.tsx(1 hunks)components/Layout/Header.tsx(2 hunks)components/Layout/Layout.tsx(1 hunks)components/Layout/MobileGnbDropdown.tsx(15 hunks)components/Layout/ThemeDropdown.tsx(1 hunks)components/Navigator.tsx(2 hunks)components/Popover/Popover.tsx(1 hunks)components/Tabs/Tab.tsx(1 hunks)components/Tabs/Tabs.context.ts(1 hunks)components/Tabs/TabsPanel.tsx(1 hunks)components/exampleView/BasicView/BasicExampleView.tsx(1 hunks)components/exampleView/BasicView/ProjectCodes.tsx(1 hunks)components/exampleView/DualView.tsx(1 hunks)components/exampleView/Sidebar/Sidebar.tsx(1 hunks)components/exampleView/Sidebar/SidebarTabs.tsx(1 hunks)docs/sample.mdx(2 hunks)hooks/useClipboard.ts(1 hunks)hooks/useOutsideClick.ts(1 hunks)hooks/useUncontrolled.ts(1 hunks)next.config.js(2 hunks)package.json(2 hunks)pages/_app.tsx(0 hunks)pages/docs/[[...slug]].tsx(0 hunks)tsconfig.json(2 hunks)utils/createSafeContext.tsx(1 hunks)utils/mdxUtils.ts(2 hunks)
💤 Files with no reviewable changes (2)
- pages/_app.tsx
- pages/docs/[[...slug]].tsx
✅ Files skipped from review due to trivial changes (3)
- components/Layout/Layout.tsx
- .github/workflows/deploy.yml
- app/examples/codemirror/page.tsx
🔇 Additional comments (54)
next.config.js (2)
21-36: Well-structured SVG handling configuration
The new SVG configuration provides a flexible approach:
- SVGs can be imported as URLs using
?urlquery - SVGs are transformed to React components by default
- Clear separation of concerns with distinct rules
33-33: Verify @svgr/webpack dependency
Ensure that @svgr/webpack is properly listed in the project's dependencies.
✅ Verification successful
@svgr/webpack dependency is properly configured
The dependency is correctly listed in package.json as "@svgr/webpack": "^6.5.0" under the dependencies section.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for @svgr/webpack dependency
# Check in package.json
echo "Checking package.json for @svgr/webpack..."
if [ -f package.json ]; then
grep -A 5 -B 5 "@svgr/webpack" package.json
fi
# Check if it's installed
echo "Checking for physical presence of @svgr/webpack..."
fd -t d "svgr" node_modules
Length of output: 636
package.json (2)
23-25: Verify Next.js 14 migration requirements
The upgrade to Next.js 14 requires several compatibility checks:
- Ensure Node.js version is ≥ 18 (as mentioned in PR requirements)
- Verify that all App Router migration steps are completed
- Check for deprecated APIs and features
✅ Verification successful
Next.js 14 upgrade appears compatible with current setup
The verification shows:
- App Router is in use (
app/directory present) - No deprecated APIs detected in the codebase
- Configuration is export-optimized and compatible with Next.js 14
- Static export setup with proper image optimization configuration
While Node.js engine requirements aren't explicitly set, the Next.js 14 setup appears correct with:
- Static export configuration (
output: 'export') - Modern features enabled (
reactStrictMode: true,swcMinify: true) - Proper image optimization setup for static exports
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if package-lock.json exists and verify engine requirements
if [ -f "package-lock.json" ]; then
echo "Checking Node.js engine requirements..."
cat package-lock.json | jq '.packages[""].engines'
fi
# Search for potentially deprecated Next.js APIs
echo "Checking for deprecated APIs..."
rg -l "getInitialProps|getServerSideProps|getStaticProps|getStaticPaths" --type js --type tsx
Length of output: 374
Script:
#!/bin/bash
# Check package.json for Node.js engine requirements
echo "Checking Node.js engine requirements in package.json..."
cat package.json | jq '.engines'
# Search for potentially deprecated Next.js APIs with correct file extensions
echo "Checking for deprecated APIs..."
rg -l "getInitialProps|getServerSideProps|getStaticProps|getStaticPaths" -g "*.{js,jsx,ts,tsx}"
# Check for App Router migration status
echo "Checking App Router structure..."
fd -t d "app" --max-depth 2
fd -t d "pages" --max-depth 2
# Look for next.config.js to check configuration
echo "Checking Next.js configuration..."
if [ -f "next.config.js" ]; then
cat next.config.js
fi
Length of output: 1903
32-34: Review breaking changes in major version updates
Several dependencies have been upgraded to new major versions, which may introduce breaking changes:
- rehype-autolink-headings (7.x)
- rehype-slug (6.x)
- remark-gfm (4.x)
- @mdx-js/loader (3.x)
- eslint-config-next (15.x)
Consider:
- Updating the documentation to reflect any new plugin configurations
- Testing MDX content rendering with the new versions
- Running ESLint with the new config to address any new rules
Also applies to: 39-39, 47-47
✅ Verification successful
Major version updates appear safe to proceed
The codebase inspection shows the MDX configuration is well-structured and the rehype/remark plugins are used in a way that's compatible with their latest major versions:
- rehype-autolink-headings is used with supported options (test, behavior)
- rehype-slug is used without custom options
- remark-gfm is used without custom options
- Custom rehype plugins (rehypeWrapContents, rehypeVariables, rehypeImageMeta) use standard unist/rehype patterns that are stable across versions
The implementation in app/docs/[[...slug]]/page.tsx follows current best practices for MDX processing with Next.js.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for MDX-related imports that might need updates
echo "Checking MDX usage patterns..."
rg -l "import.*from.*@mdx-js" --type js --type tsx
# Look for potential rehype/remark plugin configurations
echo "Checking rehype/remark configurations..."
rg -l "rehype.*config|remark.*config" --type js --type tsx
Length of output: 331
Script:
#!/bin/bash
# Check for MDX-related imports and configurations with correct file types
echo "Checking MDX usage patterns..."
rg -l "import.*from.*@mdx-js"
echo "Checking rehype/remark configurations..."
rg -l "rehype|remark"
# Check next.config.js specifically as it commonly contains MDX config
echo "Checking next.config.js content..."
if [ -f "next.config.js" ]; then
cat next.config.js
fi
# Look for any markdown or mdx files that might be affected
echo "Checking for MDX/MD files..."
fd -e mdx -e md
Length of output: 2458
Script:
#!/bin/bash
# Check the MDX configuration in the docs page component
echo "Checking MDX configuration in app/docs/[[...slug]]/page.tsx..."
cat "app/docs/[[...slug]]/page.tsx"
# Check custom rehype plugins implementation
echo "Checking custom rehype implementations..."
cat utils/rehypeWrapContents.ts
cat utils/rehypeVariables.ts
cat utils/rehypeImageMeta.ts
Length of output: 9731
components/Navigator.tsx (2)
1-1: LGTM: Correct client-side component declaration
The 'use client' directive is correctly placed at the top of the file, marking this as a client component which is necessary for using browser APIs and React hooks.
4-4: LGTM: Proper migration to App Router navigation APIs
The migration from useRouter to usePathname follows the correct patterns for Next.js App Router:
- Import from 'next/navigation' instead of 'next/router'
- Using pathname directly instead of router.asPath
- Proper null safety with optional chaining
Also applies to: 18-18, 23-23, 27-27
components/Layout/Header.tsx (2)
1-1: LGTM: Correct client-side component declaration
The 'use client' directive is correctly placed at the top of the file.
Line range hint 57-67: Verify dashboard environment variables
Please ensure that NEXT_PUBLIC_DASHBOARD_PATH is properly set in both development and production environments.
✅ Verification successful
Environment variable NEXT_PUBLIC_DASHBOARD_PATH is properly configured
The environment variable is correctly set to '/dashboard' in the .env file. Since it's prefixed with NEXT_PUBLIC_, it will be available during both build time and runtime in Next.js applications, making it suitable for use in both development and production environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if NEXT_PUBLIC_DASHBOARD_PATH is defined in environment files
echo "Checking environment variables..."
fd -t f "^\.env" -x cat {} \; | grep "NEXT_PUBLIC_DASHBOARD_PATH"
Length of output: 353
Script:
#!/bin/bash
# Check for environment variables in all files including hidden ones
echo "Checking environment files (including hidden)..."
fd -H -t f "^\.env" -x cat {} \; | grep "NEXT_PUBLIC_DASHBOARD_PATH"
# Also check for any example env files
echo -e "\nChecking example environment files..."
fd -t f "env.*example" -x cat {} \; | grep "NEXT_PUBLIC_DASHBOARD_PATH"
# Search for this env variable usage in the codebase
echo -e "\nChecking usage in the codebase..."
rg "NEXT_PUBLIC_DASHBOARD_PATH" --type-add 'config:*.{js,ts,json}' -t config
Length of output: 544
components/Layout/MobileGnbDropdown.tsx (1)
Line range hint 12-20: LGTM: Proper cleanup of window event listener
The resize event listener is correctly added and cleaned up using the useEffect hook.
app/examples/page.tsx (1)
1-1: LGTM! Metadata implementation follows Next.js App Router best practices.
The metadata export is correctly typed and follows the recommended pattern for App Router pages.
Also applies to: 4-6
app/community/page.tsx (1)
1-10: LGTM! Consistent implementation with other pages.
The implementation correctly follows Next.js App Router patterns and maintains consistency with other pages in the application.
app/not-found.tsx (1)
8-10: LGTM! Correct implementation of Next.js not-found page.
The implementation follows Next.js App Router conventions for custom 404 pages.
app/examples/webtoons/page.tsx (1)
4-6: LGTM! Metadata implementation follows Next.js 14 best practices.
The metadata export is correctly implemented using the new App Router metadata API.
app/examples/multi-cursor/page.tsx (2)
4-6: LGTM! Consistent metadata implementation across pages.
The metadata implementation follows the same pattern as other pages, maintaining good consistency.
8-10: Consider removing unnecessary async keyword
Similar to other page components, this component doesn't perform any async operations.
app/examples/quill/page.tsx (1)
10-12: Consider removing unnecessary async keyword
As with other page components, this component doesn't perform any async operations.
app/examples/todomvc/page.tsx (1)
1-12: Implementation matches the pattern - same improvements apply
The implementation follows the same pattern as the tldraw example. The same improvements regarding async keyword removal and metadata enhancement would apply here.
app/examples/kanban/page.tsx (1)
1-12: Implementation follows established pattern
The implementation maintains consistency with other example pages, which is good for maintainability.
Let's verify the migration completeness:
✅ Verification successful
Migration pattern is correctly implemented
The verification confirms that:
- All page.tsx files consistently implement the metadata pattern
- Client components (including kanban-page.tsx) properly use the 'use client' directive
- The pages directory only exists under styles/, which is unrelated to the router migration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify migration from pages to app router
# Check for any remaining pages directory or components
echo "Checking for remaining pages directory..."
fd -t d "pages$"
# Check for proper usage of 'use client' directive in client components
echo "Checking for client components without 'use client' directive..."
fd -e tsx -e jsx -E "page.tsx" . app/examples/ --exec grep -L "use client"
# Verify all example pages follow the same pattern
echo "Verifying consistent page structure across examples..."
fd "page.tsx$" app/examples/ --exec grep -L "export const metadata: Metadata"
Length of output: 1004
Script:
#!/bin/bash
# Let's verify the actual content of these page components to understand the pattern
# Check the content of kanban-page.tsx
echo "Content of kanban-page.tsx:"
cat app/examples/kanban/kanban-page.tsx
# Check a few other page components for comparison
echo -e "\nContent of example-page.tsx (base example):"
cat app/examples/example-page.tsx
# Check if these are client components
echo -e "\nChecking for 'use client' directive in these files:"
grep -l "use client" app/examples/kanban/kanban-page.tsx app/examples/example-page.tsx
# Verify the structure of page.tsx files
echo -e "\nChecking structure of page.tsx files:"
find app/examples -name "page.tsx" -exec grep -l "export const metadata: Metadata" {} \;
Length of output: 14341
app/examples/calendar/page.tsx (1)
6-8: LGTM! Proper implementation of Next.js metadata API.
The metadata implementation correctly follows the App Router conventions for defining page metadata.
app/examples/profile-stack/page.tsx (1)
10-12: Consider removing unnecessary async keyword.
The Page component is marked as async but doesn't perform any asynchronous operations.
app/examples/simultaneous-cursors/page.tsx (1)
10-12: Consider removing unnecessary async keyword.
The Page component is marked as async but doesn't perform any asynchronous operations.
utils/createSafeContext.tsx (1)
1-2: LGTM! Correctly placed 'use client' directive.
The directive is properly placed at the top of the file, ensuring client-side rendering compatibility with Next.js App Router.
components/Tabs/TabsPanel.tsx (2)
1-2: LGTM! Correctly placed 'use client' directive.
The directive is properly placed at the top of the file, ensuring client-side rendering compatibility with Next.js App Router.
Line range hint 6-12: LGTM! Well-documented props interface.
The props interface is clear and properly documented with JSDoc comments.
tsconfig.json (2)
27-31: LGTM! Required Next.js plugin added.
The Next.js plugin is correctly added, which is required for App Router functionality.
33-41: Verify TypeScript configuration for Next.js App Router.
The includes and excludes are properly configured. Let's verify the configuration against Next.js documentation.
✅ Verification successful
TypeScript configuration is correctly set up for Next.js App Router
The TypeScript configuration is properly configured according to Next.js documentation:
- Confirmed Next.js 14 project with App Router (
app/directory present) - All required compiler options are set correctly including the Next.js plugin
- Includes and excludes are properly configured with:
- TypeScript files (
**/*.ts,**/*.tsx) - Next.js type declarations (
next-env.d.ts,.next/types/**/*.ts) - Proper exclusion of
node_modules
- TypeScript files (
The absence of .next/types directory is expected as it's generated during build time.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TypeScript configuration against Next.js requirements
# Check if package.json has correct Next.js version
echo "Checking Next.js version..."
jq '.dependencies.next' package.json
# Check if all required Next.js types are included
echo "Checking for Next.js type files..."
find .next/types -type f -name "*.ts" | grep -v "node_modules"
Length of output: 301
Script:
#!/bin/bash
# Let's verify the TypeScript configuration more thoroughly
# Check if this is a Next.js project by looking for next.config
echo "Checking for Next.js config..."
fd "next.config"
# Check for App Router structure
echo "Checking for App Router structure..."
fd -t d "app$"
# Check for TypeScript configuration
echo "Checking TypeScript configuration..."
cat tsconfig.json
# Check for type definitions
echo "Checking for type definition files..."
fd -e d.ts
Length of output: 1132
hooks/useClipboard.ts (1)
1-2: LGTM! Correctly marked as client component
The 'use client' directive is correctly placed at the top of the file, which is necessary for client-side functionality like clipboard operations in Next.js App Router.
hooks/useOutsideClick.ts (1)
1-2: LGTM! Correctly marked as client component
The 'use client' directive is correctly placed at the top of the file, which is necessary for DOM operations in Next.js App Router.
components/exampleView/Sidebar/SidebarTabs.tsx (1)
1-2: LGTM! Correct usage of 'use client' directive.
The component is properly marked as a client component, which is necessary for components using hooks and browser APIs.
components/CodeBlock/CodeBlock.tsx (2)
46-52: Component exports follow React best practices.
The refactoring from namespace pattern to direct exports improves code maintainability and tree-shaking capabilities.
54-55: Verify the impact of removing namespace pattern.
The removal of the namespace pattern (CodeBlock.Wrapper, CodeBlock.ContentWrapper) might affect existing imports.
✅ Verification successful
No impact from removing namespace pattern
The only occurrences of CodeBlock.Wrapper and CodeBlock.ContentWrapper are in the commented-out lines being removed. There are no active usages of this namespace pattern in the codebase, so removing these lines is safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of the old namespace pattern
rg "CodeBlock\.(Wrapper|ContentWrapper)" --type ts --type tsx
Length of output: 91
Script:
#!/bin/bash
# Search for any remaining usage of the old namespace pattern, using ripgrep without file type filters
rg "CodeBlock\.(Wrapper|ContentWrapper)"
Length of output: 190
app/community/community-page.tsx (1)
1-1: Correctly marked as client component.
The 'use client' directive is necessary as this component uses client-side features (event handlers, state).
app/examples/profile-stack/profile-stack-page.tsx (2)
1-1: LGTM: Correct placement of 'use client' directive
The 'use client' directive is correctly placed at the top of the file, which is necessary for client-side interactivity in the App Router architecture.
Line range hint 1-50: Verify metadata configuration for this route
While the component looks good, ensure that there's a corresponding page.tsx file in the same directory that exports the metadata configuration for this route, as it's no longer handled by the removed Head component.
✅ Verification successful
Metadata configuration is properly set up for this route
The metadata configuration is correctly implemented in the page.tsx file with the appropriate title using the example title variable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if metadata is properly configured for this route
echo "Checking for page.tsx and metadata configuration..."
fd --type f "page.tsx" "app/examples/profile-stack" --exec cat {} \; | grep -A 5 "export const metadata"
Length of output: 374
app/examples/codemirror/codemirror-page.tsx (1)
Line range hint 28-35: LGTM: Proper security attributes for external links
The external links correctly implement security best practices with target="_blank" and rel="noreferrer".
app/examples/calendar/calendar-page.tsx (1)
Line range hint 1-50: Verify consistent implementation across example pages
The migration pattern should be consistent across all example pages. Let's verify this consistency.
✅ Verification successful
The initial results show consistent usage of 'use client' directive and environment variables across example pages. Let's verify the component structure and imports to ensure complete consistency in the implementation pattern.
Implementation is consistent across example pages
The verification shows consistent implementation patterns across example pages:
- All example pages use the 'use client' directive
- Consistent usage of
NextPagetype andExampleLayoutcomponent - Common imports from '@/components/exampleView'
- Consistent environment variable usage where needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent implementation across example pages
echo "Checking for consistent 'use client' directive usage..."
fd --type f ".*-page.tsx$" "app/examples" --exec grep -l "^'use client';" {} \;
echo "Checking for consistent environment variable usage..."
fd --type f ".*-page.tsx$" "app/examples" --exec grep -l "process.env.NEXT_PUBLIC_" {} \;
Length of output: 1255
Script:
#!/bin/bash
# Let's check the structure and imports of example pages to ensure consistency
echo "Checking NextPage type and ExampleLayout import usage..."
fd --type f ".*-page.tsx$" "app/examples" --exec grep -A 2 "NextPage" {} \;
echo "\nChecking common component imports..."
fd --type f ".*-page.tsx$" "app/examples" --exec grep -A 5 "import.*from '@/components/exampleView'" {} \;
Length of output: 3557
app/layout.tsx (1)
8-8:
Verify NEXT_PUBLIC_SITE_URL environment variable
The metadataBase URL is using a non-null assertion (!), which could cause runtime errors if the environment variable is not set.
Consider adding a fallback URL:
- metadataBase: new URL(process.env.NEXT_PUBLIC_SITE_URL!),
+ metadataBase: new URL(process.env.NEXT_PUBLIC_SITE_URL || 'https://yorkie.dev'),app/examples/quill/quill-page.tsx (1)
1-1: LGTM: Correct use of 'use client' directive
The directive is properly placed at the top of the file, marking this as a client component which is necessary for interactive features.
app/examples/simultaneous-cursors/simultaneous-cursors-page.tsx (2)
Line range hint 13-14: Consider moving example configuration to a separate file
Similar to the Quill example, the configuration constants could be moved to a separate file.
Line range hint 52-57: Verify environment variables for BasicExampleView
The component uses environment variables without proper fallbacks which could cause issues in development.
Consider adding meaningful fallbacks or error messages:
- rpcAddr={process.env.NEXT_PUBLIC_API_ADDR || ''}
- apiKey={process.env.NEXT_PUBLIC_EXAMPLES_API_KEY || ''}
+ rpcAddr={process.env.NEXT_PUBLIC_API_ADDR || 'http://localhost:8080'}
+ apiKey={process.env.NEXT_PUBLIC_EXAMPLES_API_KEY || (() => {
+ throw new Error('NEXT_PUBLIC_EXAMPLES_API_KEY is required');
+ })()}✅ Verification successful
Environment variables are properly configured with meaningful defaults
The verification shows that both environment variables are well-defined:
NEXT_PUBLIC_API_ADDRdefaults tohttp://localhost:8080for developmentNEXT_PUBLIC_EXAMPLES_API_KEYhas a valid default valuecedaovjuioqlk4pjqn6g
The empty string fallbacks (|| '') are acceptable here since the environment variables are guaranteed to have values from the .env file in both production and development environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if required environment variables are defined
echo "Checking for required environment variables:"
fd -t f "^\.env" -x cat {} \; | grep -E "NEXT_PUBLIC_API_ADDR|NEXT_PUBLIC_EXAMPLES_API_KEY"
Length of output: 403
Script:
#!/bin/bash
# Check for environment variable usage in the codebase
echo "Checking for environment variable usage:"
rg "NEXT_PUBLIC_API_ADDR|NEXT_PUBLIC_EXAMPLES_API_KEY" -A 2 -B 2
# Check for .env files including hidden ones
echo -e "\nChecking .env files:"
fd -H -t f "^\.env" -x cat {}
Length of output: 7044
components/Accordion/Accordion.tsx (1)
1-2: LGTM: Proper client component declaration
The 'use client' directive is correctly placed at the top of the file, marking this as a client component for Next.js App Router.
app/examples/todomvc/todomvc-page.tsx (1)
1-2: Verify metadata handling in the App Router structure
The 'use client' directive is correctly added, but with the removal of the Head component, we need to ensure that the page metadata is properly handled in a corresponding page.tsx file using Next.js 13+ metadata API.
✅ Verification successful
Let me check the content of the page.tsx file to verify the metadata implementation.
Metadata is properly handled in the App Router structure
The metadata has been correctly implemented in app/examples/todomvc/page.tsx using the Next.js App Router metadata API, with the title properly set as TodoMVC · Yorkie Examples. The removal of the Head component from todomvc-page.tsx is justified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there's a corresponding page.tsx file with metadata
fd "page.tsx$" "app/examples/todomvc"
# Check metadata implementation pattern across the app
ast-grep --pattern 'export const metadata = {
$$$
}'
Length of output: 162
Script:
#!/bin/bash
# Check content of page.tsx
cat app/examples/todomvc/page.tsx
# Also check if there's any metadata in layout files
fd "layout.tsx$" "app/examples/todomvc" -x cat {}
Length of output: 349
components/exampleView/Sidebar/Sidebar.tsx (1)
1-1: Verify client boundary optimization
The 'use client' directive at the component level might be too broad. Consider splitting into client and server components for better performance.
✅ Verification successful
The 'use client' directive is appropriately placed for this component
The Sidebar component requires client-side functionality as evidenced by:
- Uses React hooks (useState, useEffect)
- Contains interactive elements (onClick handlers)
- Manages client-side state through SidebarContextProvider
- Includes interactive child components (SidebarTabs)
- Handles dynamic UI updates (toggle functionality)
The component's responsibilities are inherently client-side, and splitting it into server/client boundaries would add unnecessary complexity without performance benefits.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check component splitting patterns across the app
fd "^page\.tsx$|^layout\.tsx$" "app" -x cat {} \; | grep -B 5 -A 5 "use client"
Length of output: 82
Script:
#!/bin/bash
# Let's check the component structure and client/server patterns more comprehensively
echo "=== Checking Sidebar component and its imports ==="
cat components/exampleView/Sidebar/Sidebar.tsx
echo -e "\n=== Checking for interactive elements in Sidebar ==="
rg "onClick|onChange|onSubmit|useState|useEffect" components/exampleView/Sidebar/Sidebar.tsx
echo -e "\n=== Checking for other client directives in the component directory ==="
fd . components/exampleView/Sidebar -x grep -l "use client" {}
echo -e "\n=== Checking component's dependencies ==="
rg "import.*from" components/exampleView/Sidebar/Sidebar.tsx
Length of output: 4020
utils/mdxUtils.ts (1)
85-85: Verify MDX content handling in App Router
The addition of the source property suggests changes in MDX rendering. Let's verify the MDX processing chain.
Also applies to: 90-102
✅ Verification successful
Let me gather more information about the MDX implementation to verify the changes.
Based on the results, I can now generate the final response:
MDX processing is properly integrated with App Router
The source property is correctly used within the MDX processing chain:
- It's read from the filesystem in
getDocsFromSlug - Properly passed to
MDXRemotein the App Router page component - Configured with appropriate remark/rehype plugins for processing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check MDX processing implementation in App Router
# Look for MDX component implementations and configurations
# Check for MDX configuration
fd -e ts -e tsx -e js -e jsx --exec rg -l "MDX|mdx"
# Check for MDX imports and usage
rg -A 3 "import.*['\"].*\.mdx['\"]"
# Look for MDX processing setup
rg "compileMDX|serialize|remarkPlugins|rehypePlugins"
Length of output: 1577
Script:
#!/bin/bash
# Check the MDX utils implementation and its usage
rg -A 10 "source.*string" utils/mdxUtils.ts
# Check MDX processing in the app router
rg -A 10 "getDocFromParams|getDoc" app/docs/
# Look for MDX-related types and interfaces
ast-grep --pattern 'interface $_ {
$$$
source: string;
$$$
}'
Length of output: 3313
app/examples/multi-cursor/multi-cursor-page.tsx (2)
1-2: Verify metadata handling for App Router
The removal of the Head component requires corresponding metadata configuration in a server component (likely page.tsx).
✅ Verification successful
Let me verify the metadata implementation by checking its content.
Metadata handling is properly configured for App Router
The metadata is correctly implemented in app/examples/multi-cursor/page.tsx with the appropriate title configuration, making the removal of the Head component in the client component valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check metadata implementation in App Router
# Look for metadata configuration in the corresponding page.tsx
# Check for metadata exports in the same directory
rg -l "export const metadata" "app/examples/multi-cursor/"
# Verify metadata type imports
rg "import.*Metadata.*from 'next'" "app/examples/multi-cursor/"
Length of output: 231
Script:
#!/bin/bash
# Check the metadata implementation in page.tsx
rg -A 5 "export const metadata" "app/examples/multi-cursor/page.tsx"
Length of output: 220
1-2: Confirm client component boundary is necessary
The component uses React hooks (useState), justifying the 'use client' directive. However, let's verify if any child components could be server components.
✅ Verification successful
'use client' directive is necessary for this component
This component requires the 'use client' directive as it uses:
- React's
useStatehook for managing theactiveTabstate - Interactive elements through the Sidebar component with event handlers (
onTabChange) - Conditional rendering based on client-side state
- Child components that also use client-side features (as evidenced by the previous search showing client features in DualView, Sidebar, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential server components
# Look for components that don't use client-side features
# Check imported components for client-side features
rg -l "useState|useEffect|onClick|onChange" "components/exampleView/"
Length of output: 299
Script:
#!/bin/bash
# Let's check the actual content of the multi-cursor-page.tsx to understand its dependencies and client-side features
cat "app/examples/multi-cursor/multi-cursor-page.tsx"
Length of output: 3133
app/examples/webtoons/webtoons-page.tsx (1)
1-2: Verify consistent metadata pattern across example pages
Let's ensure all example pages follow the same metadata pattern after the App Router migration.
components/exampleView/BasicView/ProjectCodes.tsx (1)
1-2: LGTM: Client-side directive properly added
The 'use client' directive is correctly placed at the top of the file, which is necessary for client-side rendering in Next.js App Router.
app/examples/example-page.tsx (2)
1-1: LGTM: Client component directive properly added
The 'use client' directive is correctly placed at the top of the file, marking this as a client component for Next.js App Router.
1-7: Verify metadata configuration for SEO
The Head component has been removed as part of the App Router migration. Ensure that page metadata is properly configured in a separate metadata configuration file.
✅ Verification successful
Metadata is properly configured for SEO
The page metadata is correctly configured in app/examples/page.tsx using the Next.js App Router convention with an exported metadata object that sets the page title. No issues found with the SEO configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if metadata configuration exists for the examples page
fd "layout.tsx|page.tsx" app/examples/ --exec grep -l "export const metadata"
Length of output: 426
Script:
#!/bin/bash
# Check the metadata configuration in example-page.tsx
rg "metadata" app/examples/page.tsx -A 5
# Check if there's a layout.tsx in examples directory that might contain metadata
rg "metadata" app/examples/layout.tsx -A 5 2>/dev/null || echo "No layout.tsx found"
Length of output: 268
app/products/product-page.tsx (2)
1-1: LGTM: Client component directive properly added
The 'use client' directive is correctly placed at the top of the file, marking this as a client component for Next.js App Router.
Line range hint 90-126: LGTM: CodeBlock implementation updated
The CodeBlock implementation has been properly updated to use the new wrapper component structure, maintaining the existing functionality while adapting to the new architecture.
docs/sample.mdx (1)
Line range hint 201-226: LGTM: CodeBlock wrapper implementation is consistent
The CodeBlockContentWrapper implementation is consistent with the changes made in other files and properly maintains the MDX structure.
app/home-page.tsx (1)
1-6: LGTM! Client-side setup is correct.
The 'use client' directive is properly placed, and the imports are correctly structured for the App Router migration.
Let's verify all client-side dependencies are properly marked:
✅ Verification successful
Based on the output, I can see that all components using client-side hooks (useState, useEffect, onClick, onPointer) have the 'use client' directive properly set. The output shows that all relevant files have the directive, including:
- The file under review (
app/home-page.tsx) - All components that use React hooks or event handlers
- All context providers and UI components with interactivity
All client-side components are properly marked with 'use client' directive
The migration to Next.js App Router has been done correctly with respect to client/server component boundaries.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other components that might need 'use client' directive
rg -l "useState|useEffect|onClick|onPointer" --type tsx --type jsx | xargs rg -L "'use client'"
Length of output: 23171
| @@ -1,5 +1,6 @@ | |||
| /** @type {import('next').NextConfig} */ | |||
| const nextConfig = { | |||
| output: 'export', | |||
There was a problem hiding this comment.
💡 Codebase verification
Static export configuration needs adjustment due to dynamic routes
The output: 'export' setting is incompatible with the current codebase structure. Found a catch-all dynamic route at app/docs/[[...slug]]/ which is not supported with static exports. You'll need to either:
- Remove the dynamic route and use static routes instead
- Or remove the
output: 'export'configuration if dynamic routing is required
🔗 Analysis chain
Verify static export compatibility with App Router features
The output: 'export' setting enables static HTML exports but has some limitations with App Router features. Please ensure that:
- No Server Components are using dynamic features (e.g., cookies, headers, dynamic routes)
- No middleware is being used
- No Route Handlers are present
Let me gather more information about the codebase to verify if there are any server components or dynamic features that might be incompatible with static export.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential incompatible features with static export
# Check for dynamic route segments
echo "Checking for dynamic route segments..."
fd --type f --extension tsx --extension ts --extension js --extension jsx . app/ --exec grep -l '\[\[.*\]\]'
# Check for route handlers
echo "Checking for route handlers..."
fd 'route\.(ts|js|tsx|jsx)$' app/
# Check for middleware
echo "Checking for middleware file..."
fd '^middleware\.(ts|js)$'
Length of output: 406
Script:
#!/bin/bash
# Check for server components and dynamic features
# Look for 'use server' directives
echo "Checking for server components..."
rg "'use server'" -l
# Look for dynamic data fetching patterns
echo "Checking for dynamic data fetching..."
rg "fetch\(.*\)" -l
# Look for cookie usage
echo "Checking for cookie usage..."
rg "cookies" -l
# Look for headers usage
echo "Checking for headers usage..."
rg "headers\(\)" -l
# Check for dynamic route patterns (alternative approach)
echo "Checking for dynamic routes..."
fd --type d '\[.*\]' app/
# Check for API routes
echo "Checking for API routes..."
fd --type d 'api' app/
Length of output: 593
| @@ -5,8 +5,7 @@ | |||
| "scripts": { | |||
| "dev": "next dev", | |||
| "build": "next build && next-sitemap", | |||
| "export": "next build && next-sitemap && next export", | |||
| "start": "next start", | |||
| "start": "npx serve@latest out", | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin the serve package version for stability
Using npx serve@latest could potentially introduce breaking changes in the future. Consider pinning to a specific version for better stability and predictability.
- "start": "npx serve@latest out",
+ "start": "npx [email protected] out",📝 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.
| "start": "npx serve@latest out", | |
| "start": "npx serve@14.2.1 out", |
| @@ -1,13 +1,13 @@ | |||
| import { useState, useEffect } from 'react'; | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing 'use client' directive in MobileGnbDropdown.tsx
The component MobileGnbDropdown.tsx uses client-side React hooks (useState, useEffect, usePathname) but lacks the required 'use client' directive. This needs to be added at the top of the file to properly mark it as a client component in the Next.js App Router architecture.
- Add
'use client';as the first line incomponents/Layout/MobileGnbDropdown.tsx
🔗 Analysis chain
Verify App Router migration completeness
Let's verify that all necessary files have been migrated to the App Router pattern.
Let me gather more information to verify if there are any client components missing the 'use client' directive.
Let me try one more search with corrected file type flags:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining usage of next/router
echo "Checking for remaining next/router imports..."
rg "from 'next/router'" -l
# Check for proper 'use client' declarations in client components
echo "Checking for client components without 'use client' directive..."
rg "usePathname|useRouter|useState|useEffect" -l | xargs rg "^'use client'" -L
Length of output: 1272
Script:
#!/bin/bash
# Check specifically for MobileGnbDropdown.tsx since it uses useState and useEffect
echo "Checking MobileGnbDropdown.tsx for 'use client' directive..."
rg "^'use client'" components/Layout/MobileGnbDropdown.tsx
# List all files that use React hooks but weren't caught in the previous search
echo "Checking for additional files with React hooks..."
rg "import.*\{.*(?:useState|useEffect|useRouter|usePathname).*\}" --type ts --type tsx | grep -v "^components/Layout/Header\|^components/Layout/ExampleLayout\|^components/Layout/ThemeDropdown\|^components/exampleView\|^components/Navigator\|^hooks\|^app"
Length of output: 597
Script:
#!/bin/bash
# Check for any client-side functionality in MobileGnbDropdown.tsx
echo "Checking content of MobileGnbDropdown.tsx..."
cat components/Layout/MobileGnbDropdown.tsx
# Search for additional React hooks usage with correct ripgrep syntax
echo "Checking for additional files with React hooks..."
rg "import.*\{.*(useState|useEffect|useRouter|usePathname).*\}" --type-add 'tsx:*.tsx' --type tsx
Length of output: 10128
| document.querySelector(`.toc-item.is_active`)?.classList.remove('is_active'); | ||
| document.querySelector(`[data-heading="${activeId}"]`)?.classList.add('is_active'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid direct DOM manipulation; use React state or refs instead
Directly manipulating the DOM with document.querySelector can lead to inconsistencies with React's virtual DOM and potential bugs. Consider using React refs or state to manage the active class on TOC items.
| useEffect(() => { | ||
| setHeadings(Array.from(document.querySelectorAll('.documentation_page .heading'))); | ||
| }, [content]); | ||
|
|
There was a problem hiding this comment.
Ensure heading positions are updated when headings change
Currently, updateHeadingPositions is called only when activeId or updateHeadingPositions change. However, if headings change, the headingTops need to be recalculated to ensure accurate scroll position tracking. Consider invoking updateHeadingPositions whenever headings change.
Apply this diff to call updateHeadingPositions when headings change:
useEffect(() => {
+ updateHeadingPositions();
}, [activeId, updateHeadingPositions]);Alternatively, adjust the dependencies:
-}, [activeId, updateHeadingPositions]);
+}, [activeId, headings, updateHeadingPositions]);Committable suggestion skipped: line range outside the PR's diff.
| 'use client'; | ||
|
|
||
| import type { NextPage } from 'next'; | ||
| import Head from 'next/head'; | ||
| import { Button, Icon, Layout } from '@/components'; |
There was a problem hiding this comment.
Update file naming and types for Next.js App Router.
- The file should be renamed to
not-found.tsx(without-page) as per Next.js 13+ conventions NextPagetype is not necessary in the App Router
- import type { NextPage } from 'next';
import { Button, Icon, Layout } from '@/components';
import Error404SVG from '@/public/assets/icons/error_404.svg';
- const Custom404: NextPage = () => {
+ export default function NotFound() {📝 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.
| 'use client'; | |
| import type { NextPage } from 'next'; | |
| import Head from 'next/head'; | |
| import { Button, Icon, Layout } from '@/components'; | |
| 'use client'; | |
| import { Button, Icon, Layout } from '@/components'; | |
| import Error404SVG from '@/public/assets/icons/error_404.svg'; | |
| export default function NotFound() { |
| images: [{ url: `/og.png` }], | ||
| url: '', |
There was a problem hiding this comment.
Fix OpenGraph image and URL paths
The OpenGraph image path is not using the prefix utility, and the URL is empty.
- images: [{ url: `/og.png` }],
- url: '',
+ images: [{ url: `${prefix}/og.png` }],
+ url: process.env.NEXT_PUBLIC_SITE_URL || 'https://yorkie.dev',Committable suggestion skipped: line range outside the PR's diff.
| card: 'summary_large_image', | ||
| site: '@team_yorkie', | ||
| title: 'Yorkie - Bring collaboration to your app', | ||
| images: [`/og.png`], |
There was a problem hiding this comment.
Fix Twitter image path
The Twitter card image path is not using the prefix utility.
- images: [`/og.png`],
+ images: [`${prefix}/og.png`],Committable suggestion skipped: line range outside the PR's diff.
| 'use client'; | ||
|
|
||
| import { NextPage } from 'next'; | ||
| import Head from 'next/head'; | ||
| import { ExampleLayout, CodeBlock } from '@/components'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update NextPage type usage for App Router
The NextPage type from 'next' is primarily for Pages Router. For App Router, consider using React.FC or removing the type annotation as it's not necessary.
-import { NextPage } from 'next';
-const KanbanExampleView: NextPage = () => {
+const KanbanExampleView = () => {📝 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.
| 'use client'; | |
| import { NextPage } from 'next'; | |
| import Head from 'next/head'; | |
| import { ExampleLayout, CodeBlock } from '@/components'; | |
| 'use client'; | |
| import { ExampleLayout, CodeBlock } from '@/components'; |
| @@ -82,10 +82,12 @@ export type DocsMeta = { | |||
| type Docs = { | |||
| content: string; | |||
| meta: DocsMeta; | |||
| source: string; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize file reading operations
The function performs two separate file reads for the same file:
mdxSource = fs.readFileSync(docsFilePath, 'utf-8')source = fs.readFileSync(docsFilePath)
This can be optimized to a single read operation.
export const getDocsFromSlug = (slug: string): Docs => {
const docsFilePath = path.join(DOCS_PATH, `${slug}.mdx`);
- const mdxSource = fs.readFileSync(docsFilePath, 'utf-8');
- const source = fs.readFileSync(docsFilePath);
+ const source = fs.readFileSync(docsFilePath, 'utf-8');
- const { content, data } = matter(source);
+ const { content, data } = matter(source);
return {
content,
meta: {
title: data.title || slug,
order: data.order || 0,
phase: data.phase || '',
},
- source: mdxSource,
+ source,
};
};Also applies to: 90-102
What this PR does / why we need it?
migration hompage's next.js code to 14 version(page >> app)
Any background context you want to provide?
related
In order for this PR to merge, node version must be 18 or higher, so I think this PR should be merged first and then merged.
What are the relevant tickets?
Fixes #96
Checklist
Summary by CodeRabbit
Release Notes
New Features
ThemeInitializercomponent to manage user theme preferences.DocsPagecomponent for rendering documentation with enhanced Markdown processing.Popovercomponent for managing popover UI elements.useClipboardhook for copying text to the clipboard.Improvements
usePathnamefor routing.Accordioncomponent with better state management and rendering options.Bug Fixes
Headcomponents from several pages to streamline metadata management.Chores