Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: migration app router from pages router #180

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ name: Deploy Next.js site to Pages
on:
# Runs on pushes targeting the default branch
push:
branches: ["main"]
branches: ['main']

# Allows you to run this workflow manually from the Actions tab
workflow_dispatch:
Expand All @@ -20,7 +20,7 @@ permissions:

# Allow one concurrent deployment
concurrency:
group: "pages"
group: 'pages'
cancel-in-progress: true

jobs:
Expand All @@ -36,7 +36,7 @@ jobs:
- name: Setup Node
uses: actions/setup-node@v3
with:
node-version: "16"
node-version: '16'
cache: ${{ steps.detect-package-manager.outputs.manager }}
- name: Setup Pages
uses: actions/configure-pages@v2
Expand All @@ -60,10 +60,8 @@ jobs:
run: ${{ steps.detect-package-manager.outputs.manager }} ${{ steps.detect-package-manager.outputs.command }}
- name: Build with Next.js
run: ${{ steps.detect-package-manager.outputs.manager }} run build
- name: Static HTML export with Next.js
run: ${{ steps.detect-package-manager.outputs.runner }} next export
- name: Optimize all static images after the Next.js static export
run: ${{ steps.detect-package-manager.outputs.runner }} next-image-export-optimizer
run: ${{ steps.detect-package-manager.outputs.runner }} next-image-export-optimizer
- name: Upload artifact
uses: actions/upload-pages-artifact@v1
with:
Expand Down
16 changes: 16 additions & 0 deletions app/_components/theme-initializer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use client';

import { type ThemeOption, useTheme } from '@/hooks/useTheme';
import { useEffect } from 'react';

function ThemeInitializer() {
const { setTheme } = useTheme();
useEffect(() => {
const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
setTheme(themeOption);
}, [setTheme]);
Comment on lines +8 to +11
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for localStorage access.

The localStorage access should be wrapped in a try-catch block to handle cases where it might be unavailable (e.g., in private browsing mode).

 useEffect(() => {
-  const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
-  setTheme(themeOption);
+  try {
+    const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
+    setTheme(themeOption);
+  } catch (error) {
+    console.warn('Failed to access localStorage:', error);
+    setTheme('system');
+  }
 }, [setTheme]);
📝 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.

Suggested change
useEffect(() => {
const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
setTheme(themeOption);
}, [setTheme]);
useEffect(() => {
try {
const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
setTheme(themeOption);
} catch (error) {
console.warn('Failed to access localStorage:', error);
setTheme('system');
}
}, [setTheme]);


return null;
}
Comment on lines +6 to +14
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider hydration mismatch prevention.

To prevent hydration mismatches, consider using a mounting check before accessing localStorage.

 function ThemeInitializer() {
   const { setTheme } = useTheme();
   useEffect(() => {
+    let mounted = true;
     try {
       const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
-      setTheme(themeOption);
+      if (mounted) {
+        setTheme(themeOption);
+      }
     } catch (error) {
       console.warn('Failed to access localStorage:', error);
-      setTheme('system');
+      if (mounted) {
+        setTheme('system');
+      }
     }
+    return () => {
+      mounted = false;
+    };
   }, [setTheme]);

   return null;
 }
📝 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.

Suggested change
function ThemeInitializer() {
const { setTheme } = useTheme();
useEffect(() => {
const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
setTheme(themeOption);
}, [setTheme]);
return null;
}
function ThemeInitializer() {
const { setTheme } = useTheme();
useEffect(() => {
let mounted = true;
try {
const themeOption = (window.localStorage.getItem('theme') || 'system') as ThemeOption;
if (mounted) {
setTheme(themeOption);
}
} catch (error) {
console.warn('Failed to access localStorage:', error);
if (mounted) {
setTheme('system');
}
}
return () => {
mounted = false;
};
}, [setTheme]);
return null;
}


export default ThemeInitializer;
6 changes: 2 additions & 4 deletions pages/community.tsx → app/community/community-page.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
'use client';

import type { NextPage } from 'next';
import Head from 'next/head';
import { Button, Icon, Layout } from '@/components';
import CommunitySVG from '@/public/assets/icons/community_help.svg';

const Community: NextPage = () => {
return (
<Layout className="community_page">
<Head>
<title>Community · Yorkie</title>
</Head>
<div className="content">
<div className="img_box">
<CommunitySVG />
Expand Down
10 changes: 10 additions & 0 deletions app/community/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Metadata } from 'next';
import CommunityPage from './community-page';

export const metadata: Metadata = {
title: 'Community · Yorkie',
};

export default async function Page() {
return <CommunityPage />;
}
151 changes: 151 additions & 0 deletions app/docs/[[...slug]]/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { getSlugs, getDocsFromSlug, getDocsMetaFromSlug, getDocsOrderList } from '@/utils/mdxUtils';
import type { MDXComponents } from 'mdx/types';
import remarkGfm from 'remark-gfm';
import rehypeSlug from 'rehype-slug';
import rehypeAutolinkHeadings from 'rehype-autolink-headings';
import rehypeToc, { HtmlElementNode, ListItemNode } from '@jsdevtools/rehype-toc';
import rehypeImageMeta from '@/utils/rehypeImageMeta';
import rehypeWrapContents from '@/utils/rehypeWrapContents';
import rehypeVariables from '@/utils/rehypeVariables';
import { Button, Icon, CodeBlock, CodeBlockContentWrapper, CodeBlockHeader, Image } from '@/components';
import { CustomLink, CustomCodeBlock, Breadcrumb, Caption, ImageWrap, Alert, Blockquote } from '@/components/docs';
import SlugPage from './slug-page';
import { MDXRemote } from 'next-mdx-remote/rsc';

type Props = {
params: {
slug?: string[];
};
};

const components: MDXComponents = {
a: CustomLink,
h3: (props) => <h3 className="heading" {...props} />,
h4: (props) => <h4 className="heading" {...props} />,
h5: (props) => <h5 className="heading" {...props} />,
h6: (props) => <h6 className="heading" {...props} />,
Button,
Icon,
pre: (props) => <CustomCodeBlock {...props} />,
blockquote: (props) => <Blockquote {...props} />,
img: ({ src, alt, title, width, height }) => (
<Image src={src!} alt={alt || ''} title={title} width={width as number} height={height as number} />
),
Image,
ImageWrap,
Breadcrumb,
Caption,
Alert,
CodeBlock,
CodeBlockHeader,
CodeBlockContentWrapper,
};

export default async function DocsPage({ params }: Props) {
const slug = params.slug?.join('/') ?? 'index';

const { content, source } = getDocsFromSlug(slug);

const navList = getDocsOrderList('/docs');

const mdxContent = await MDXRemote({
source,
components,
options: {
mdxOptions: {
remarkPlugins: [remarkGfm],
rehypePlugins: [
rehypeImageMeta,
[
rehypeVariables,
{
variables: [
{ pattern: 'YORKIE_VERSION', value: process.env.NEXT_PUBLIC_YORKIE_VERSION },
{ pattern: 'YORKIE_JS_VERSION', value: process.env.NEXT_PUBLIC_YORKIE_JS_VERSION },
{ pattern: 'YORKIE_IOS_VERSION', value: process.env.NEXT_PUBLIC_YORKIE_IOS_VERSION },
{ pattern: 'YORKIE_ANDROID_VERSION', value: process.env.NEXT_PUBLIC_YORKIE_ANDROID_VERSION },
{ pattern: 'DASHBOARD_PATH', value: process.env.NEXT_PUBLIC_DASHBOARD_PATH },
{ pattern: 'API_ADDR', value: process.env.NEXT_PUBLIC_API_ADDR },
{ pattern: 'API_HOST', value: process.env.NEXT_PUBLIC_API_HOST },
{ pattern: 'API_PORT', value: process.env.NEXT_PUBLIC_API_PORT },
],
},
],
rehypeSlug,
[
rehypeAutolinkHeadings,
{
test: ['h3', 'h4', 'h5', 'h6'],
behavior: 'wrap',
},
],
[
rehypeToc,
{
position: 'afterend',
headings: ['h3', 'h4', 'h5', 'h6'],
customizeTOC: (toc: HtmlElementNode) => {
const children = toc.children || [];
const contents = (children[0] as any)?.children?.length;
if (!contents) return false;

toc.tagName = 'div';
toc.properties.className = 'pagination';
const wrapper = {
type: 'element',
tagName: 'div',
properties: { className: 'pagination_inner' },
children: [
{
type: 'element',
tagName: 'strong',
properties: { className: 'pagination_title' },
children: [{ type: 'text', value: 'On this page' }],
},
...children,
],
};
toc.children = [wrapper];
},
customizeTOCItem: (tocItem: ListItemNode, heading: HtmlElementNode) => {
tocItem.properties['data-heading'] = heading.properties.id;
},
},
],
rehypeWrapContents,
],
},
parseFrontmatter: true,
},
});

return (
<SlugPage navList={navList} content={content}>
{mdxContent}
</SlugPage>
);
}

export async function generateMetadata({ params }: Props) {
const slug = params.slug?.join('/') ?? 'index';
const { meta } = getDocsFromSlug(slug);

return {
...meta,
};
}

export const generateStaticParams = async () => {
const params = getSlugs()
.filter((slug) => {
if (process.env.NODE_ENV === 'development') return true;

const { phase } = getDocsMetaFromSlug(slug);
return phase !== 'development';
})
.map((slug) => ({
slug: slug === 'index' ? [''] : slug.split('/'),
}));

return params;
};
29 changes: 29 additions & 0 deletions app/docs/[[...slug]]/slug-page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use client';

import Head from 'next/head';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove commented Head component and use metadata.ts instead

The commented Head component is from Pages Router. In App Router, page metadata should be handled using a separate metadata.ts file or the generateMetadata function.

Remove the commented code and implement metadata using App Router conventions:

-import Head from 'next/head';

Create a new file metadata.ts in the same directory:

import { Metadata } from 'next';

export const metadata: Metadata = {
  title: 'Documentation · Yorkie',
};

Also applies to: 19-21

import { type DocsOrderList } from '@/utils/mdxUtils';
import { Layout, Navigator } from '@/components';
import ScrollPositionIndicator from '../_components/scroll-position-indicator';

export default function DocsPage({
content,
navList,
children,
}: {
content: string;
navList: DocsOrderList;
children: React.ReactNode;
}) {
return (
<Layout className="documentation_page">
{/* <Head>
<title>Documentation · Yorkie</title>
</Head> */}
<ScrollPositionIndicator content={content} />
<div className="content">
<Navigator navList={navList} />
<section className="section">{children}</section>
</div>
</Layout>
);
}
59 changes: 59 additions & 0 deletions app/docs/_components/scroll-position-indicator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'use client';

import { useState, useEffect, useCallback } from 'react';

export default function ScrollPositionIndicator({ content }: { content: string }) {
const [activeId, setActiveId] = useState<string>('');
const [headings, setHeadings] = useState<Array<Element>>([]);
const [headingTops, setHeadingTops] = useState<Array<{ id: string; top: number }>>([]);

const updateHeadingPositions = useCallback(() => {
const scrollTop = document.documentElement.scrollTop;
const headingTops = [...headings].map((heading) => {
const top = heading.getBoundingClientRect().top + scrollTop;
return {
id: heading.id,
top,
};
});
setHeadingTops(headingTops);
}, [headings]);

useEffect(() => {
setHeadings(Array.from(document.querySelectorAll('.documentation_page .heading')));
}, [content]);

Comment on lines +22 to +25
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

useEffect(() => {
if (headings.length === 0) return;

const setActiveTOCLink = () => {
const scrollTop = document.documentElement.scrollTop;
const yOffset = 150;

const currentHeading =
scrollTop < 10
? headingTops.find((headingTop) => {
return scrollTop >= headingTop.top - yOffset;
})
: [...headingTops].reverse().find((headingTop) => {
return scrollTop >= headingTop.top - yOffset;
});

setActiveId(currentHeading?.id || '');
};

window.addEventListener('scroll', setActiveTOCLink);
setActiveTOCLink();
return () => {
window.removeEventListener('scroll', setActiveTOCLink);
};
}, [headings, headingTops]);

useEffect(() => {
updateHeadingPositions();
document.querySelector(`.toc-item.is_active`)?.classList.remove('is_active');
document.querySelector(`[data-heading="${activeId}"]`)?.classList.add('is_active');
Comment on lines +54 to +55
Copy link

Choose a reason for hiding this comment

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

🛠️ 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.

}, [activeId, updateHeadingPositions]);

return null;
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use client';

import { NextPage } from 'next';
import Head from 'next/head';
import { ExampleLayout } from '@/components';
import {
Sidebar,
Expand All @@ -19,9 +20,6 @@ const CalendarExampleView: NextPage = () => {
<ExampleLayout breadcrumbTitle={exampleTitle}>
{() => (
<>
<Head>
<title>{`${exampleTitle} · Yorkie Examples`}</title>
</Head>
<Sidebar wide>
<Sidebar.Tabs defaultTab="code">
<Sidebar.Top>
Expand Down
12 changes: 12 additions & 0 deletions app/examples/calendar/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Metadata } from 'next';
import CalendarPage from './calendar-page';

const exampleTitle = 'Calendar';

export const metadata: Metadata = {
title: `${exampleTitle} · Yorkie Examples`,
};

export default async function Page() {
return <CalendarPage />;
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use client';

import { NextPage } from 'next';
import Head from 'next/head';
import { ExampleLayout } from '@/components';
import {
Sidebar,
Expand All @@ -18,9 +19,6 @@ const CodemirrorExampleView: NextPage = () => {
<ExampleLayout breadcrumbTitle={exampleTitle}>
{() => (
<>
<Head>
<title>{`${exampleTitle} · Yorkie Examples`}</title>
</Head>
<Sidebar wide>
<Sidebar.Tabs defaultTab="code">
<Sidebar.Top>
Expand Down
12 changes: 12 additions & 0 deletions app/examples/codemirror/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Metadata } from 'next';
import CodemirrorPage from './codemirror-page';

const exampleTitle = 'CodeMirror';

export const metadata: Metadata = {
title: `${exampleTitle} · Yorkie Examples`,
};

export default async function Page() {
return <CodemirrorPage />;
}
Loading
Loading