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.

Sign up for GitHub

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

Jump to bottom

chore: migrate next to app router #32

Open
wants to merge 18 commits into
base: next
Choose a base branch
from
Open

chore: migrate next to app router #32

wants to merge 18 commits into from

Conversation

RyukTheCoder
Copy link
Collaborator

@RyukTheCoder RyukTheCoder commented Oct 5, 2024 •
edited
Loading

Summary

This pull request migrates the Next.js project from the traditional Pages Router to the new App Router. This migration aims to leverage the latest features and improvements provided by the App Router, enhancing the overall performance, scalability, and maintainability of the application.

Key Changes

1. Project Structure:

Moved all page components from the pages directory to the app directory.
Updated the directory structure to follow the App Router conventions.

2. Routing:

Replaced all pages-based routing with app-based routing.
Updated dynamic routes to use the new file-based routing system.

3. Data fetching:

Moved data fetching to server on most places to optimize page loadings

4. Components:

Refactored components to be compatible with the App Router.
Updated import paths to reflect the new directory structure.

5. Dependencies:

Updated dependencies to the latest versions compatible with the App Router.

@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from b50d6de to 1245da6 Compare October 5, 2024 00:48
@RyukTheCoder RyukTheCoder requested a review from nikaaru October 5, 2024 08:57
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
Copy link
Member

@nikaaru nikaaru left a comment

Choose a reason for hiding this comment

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

Thanks,
I just left some comments

@nikaaru

This comment was marked as resolved.

Sign in to view
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
app/layout.tsx Outdated Show resolved Hide resolved
@nikaaru

This comment was marked as resolved.

Sign in to view
@nikaaru

This comment was marked as resolved.

Sign in to view
@nikaaru

This comment was marked as resolved.

Sign in to view
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
app/layout.tsx Outdated Show resolved Hide resolved
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
app/actions.ts Outdated Show resolved Hide resolved
@nikaaru

This comment was marked as resolved.

Sign in to view
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
app/search/page.tsx Outdated Show resolved Hide resolved
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
services/constants.ts Outdated Show resolved Hide resolved
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
app/statistics/page.tsx Outdated Show resolved Hide resolved
nikaaru
nikaaru reviewed Dec 21, 2024
View reviewed changes
package.json Show resolved Hide resolved
@nikaaru

This comment was marked as resolved.

Sign in to view
@RyukTheCoder
Copy link
Collaborator Author

RyukTheCoder commented Dec 22, 2024 •
edited
Loading

Please use the src directory as a single source of the project.

Done in 47c361c

@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from 9786fd1 to 466e485 Compare December 22, 2024 07:02
@RyukTheCoder
Copy link
Collaborator Author

RyukTheCoder commented Dec 22, 2024 •
edited
Loading

Please use title tag template for generate title tag: https://nextjs.org/docs/app/api-reference/functions/generate-metadata#template-object

Done in e2d348f

@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from 4c3dbd1 to e2bf41d Compare December 23, 2024 09:51
@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from e2bf41d to 647b04b Compare December 23, 2024 09:59
@RyukTheCoder
Copy link
Collaborator Author

We had several changes, for example, we moved the chart to a separate package. Please also take the latest changes from the next branch and resolve the conflicts.

Tnx, resolved.

@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from 647b04b to 06aea3c Compare December 23, 2024 10:06
@RyukTheCoder RyukTheCoder requested a review from nikaaru December 29, 2024 05:46
@nikaaru

This comment was marked as resolved.

Sign in to view
@nikaaru

This comment was marked as resolved.

Sign in to view
@RyukTheCoder
Copy link
Collaborator Author

please use Suspense in src/app/swap/[id]/page.tsx for the SwapDetailSummary and SwapSteps components.

Done in b4039e5

@RyukTheCoder
Copy link
Collaborator Author

Please use the progress bar with app router compatibility.

Added in 0005068

@nikaaru

This comment was marked as resolved.

Sign in to view
@RyukTheCoder
Copy link
Collaborator Author

RyukTheCoder commented Jan 6, 2025 •
edited
Loading

please use useRouter of nextjs-toploader/app in src/components/common/SearchBox/SearchInput.tsx instead of next useRouter.

In order to import nextjs-toploader/app we need to update module and moduleResolution in tsconfig and also update other settings to make all imports compatible. Considering that maybe those changes go out of scope of this PR, we can do that in another PR.

@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from 8d94fb3 to 9a4b69a Compare January 6, 2025 06:02
@RyukTheCoder RyukTheCoder force-pushed the chore/rf-1507-migrate-explorer-to-app-router branch from 9a4b69a to 0005068 Compare January 7, 2025 09:54
nikaaru
nikaaru approved these changes Jan 7, 2025
View reviewed changes
Copy link
Member

@nikaaru nikaaru left a comment

Choose a reason for hiding this comment

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

Thanks,
LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikaaru nikaaru nikaaru approved these changes

Assignees

@nikaaru nikaaru

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RyukTheCoder @nikaaru