Migrate from Next.js Pages Router to App Router #255

sawyerh · 2023-11-30T01:59:14Z

Ticket

Resolves #66

Changes

This is purely an architecture migration and nothing functionally should have changed.

Pages Router → App Router:

pages/_app.tsx → app/[locale]/layout.tsx
pages/index.tsx → app/[locale]/page.tsx
pages/health.tsx → app/[locale]/health/page.tsx
pages/api/hello.ts → app/api/hello/route.tsx

Internationalized routing and locale detection:

App Router doesn't include built-in support for internationalized routing and locale detection like Pages Router.

Added Next.js Middleware (src/middleware.ts) to detect and store the user's preferred language, and routing them to /es-US/* when their preferred language is Spanish
Added i18n/server.ts to contain server-side configuration for the next-intl plugin. This makes locale strings available to all server components

Context for reviewers

See the related ADR for why we're moving towards App Router.
A page file is restricted in what it can export, hence why view.tsx exists instead of that component being colocated in the page.tsx file. More context on this Next.js issue.

Testing

…-migration # Conflicts: # app/.eslintrc.js # app/.storybook/preview.tsx # app/next.config.js # app/package-lock.json # app/package.json # app/src/components/Header.tsx # app/src/components/Layout.tsx # app/src/pages/_app.tsx # app/src/pages/health.tsx # app/src/pages/index.tsx # app/src/types/i18n.d.ts # app/tests/app/page.test.tsx # app/tests/components/Header.test.tsx # app/tests/components/Layout.test.tsx # app/tests/jest-i18n.ts

# Conflicts: # app/.storybook/I18nStoryWrapper.tsx # app/.storybook/preview.tsx # app/README.md # app/next.config.js # app/src/app/[locale]/page.tsx # app/src/i18n/config.ts # app/src/i18n/getMessagesWithFallbacks.ts # app/src/pages/_app.tsx # app/tests/react-utils.tsx # docs/internationalization.md

sawyerh · 2023-12-08T01:34:56Z

app/next-env.d.ts

@@ -1,5 +1,6 @@
 /// <reference types="next" />
 /// <reference types="next/image-types/global" />
+/// <reference types="next/navigation-types/compat/navigation" />


ℹ️ Automatically added by Next.js when using App Router

sawyerh · 2023-12-08T01:35:28Z

app/tsconfig.json

+      {
+        "name": "next"
+      }
+    ]


ℹ️ Automatically added by Next.js when using App Router

…-migration # Conflicts: # app/package-lock.json # app/package.json # app/src/app/[locale]/page.tsx # app/tests/app/page.test.tsx

sawyerh · 2023-12-13T00:55:42Z

⛔ I'm going to move this back into a Draft state. I encountered a blocker that is making me reconsider whether it's still too soon to migrate to React Server Components (AKA App Router).

Context

The main benefit of using App Router is to use React Server Components (RSC), which are a new feature.

First, it may be helpful to provide context on the RSC and Next.js release timeline so far, and my thinking for when it felt appropriate to finally perform a migration:

React Server Components (RSC) are currently in React's canary release. Next.js is the earliest adopter and main group evangelizing and building support for RSC (in collaboration with the React team). Next.js includes the React canary release to provide support for server components and server actions. Canary releases are said to be "stable for frameworks".
Next.js released "beta" App Router support in version 13.0 in October 2022, and it lacked functionality such as server actions and mutations (forms). The Nava template didn't migrate at this time because App Router was considered "beta" and it seemed like there were still a lot of open questions about RSC that needed worked out (data fetching, caching, mutations, best practices).
In version 13.4 in May 2023, Next.js moved App Router from "beta" to "stable", and introduced an "alpha" release of server actions. The Nava template didn't migrate at this time since there were still unstable RSC features.
In version 14.0 in October 2023 (a year after 13.0), Next.js moved server actions from "alpha" to "stable".

Blockers and concerns

With server actions now stable, and an entire year for teams to kick the tires of server components, it appeared like things were in a better place for the Nava template to finally migrate to the App Router architecture. For the most part, I think that's still true, but one thing this migration PR has surfaced is that not all tooling in the React ecosystem has made the jump.

The primary blocker right now is that Jest and React Testing Library lack support for async server components. This prevents testing any page that includes data fetching logic. The Next.js recommendation at the moment is to use end-to-end testing for async server components. The template currently doesn't provide an end-to-end testing framework.
Not a blocker, and partially resolved (by me), but the React USWDS component library didn't support server components, and only barely does. It ultimately needs further changes in order better support server components — right now every single component is a client component, and the entire library is included in the client-side bundle.
Next.js continues to iterate its caching strategy. Currently, they extend the native fetch API and cache by default, which is confusing and controversial. They're now acknowledging this and it sounds like they'll be moving away from this in a future release. This isn't a blocker, but another signal that things are still not stable.

Given the above, I'm leaning towards continuing to hold off on migrating to the App Router. I think the following need to happen before we reconsider:

Jest and React Testing Library support async server components;
and/or the template provides an example end-to-end test of a Next.js page.

My personal preference would be to continue to have the ability to test within Jest since these tests are often faster and don't require running the entire application.

…-migration # Conflicts: # app/README.md # app/src/app/[locale]/page.tsx # app/src/components/Layout.tsx # app/src/pages/health.tsx

leerob · 2023-12-13T18:23:22Z

Hey @sawyerh! Thanks for writing this up 😄 Your timeline seems accurate. I wanted to share some other thoughts.

Next.js is the earliest adopter and main group evangelizing and building support for RSC (in collaboration with the React team).

Redwood and Remix are also moving in the RSC direction. Here's Redwoods notes. There's other more experimental frameworks like Waku being released.

The template currently doesn't provide an end-to-end testing framework.

We hope to continue to improve our testing docs and examples here for unit, integration, and E2E test. We also have this RFC open around testing.

Right now every single component is a client component, and the entire library is included in the client-side bundle.

I wanted to mention that, while this isn't the most ideal state, it's also (IMO) still an improvement over the Pages Router. For example, you getting streaming SSR by default, which is very powerful. I made a video about this here.

This isn't a blocker, but another signal that things are still not stable (around caching).

All of the existing caching work is stable and functional. We just want to improve the DX further. There will be smooth migrations from the existing DX today and possibly some simplified ways of authoring code around caching when ready. We will share more details and plans are figured out. But that doesn't mean it's unstable today!

Hope this helps 🙏

sawyerh · 2023-12-13T18:39:23Z

@leerob Really appreciate you jumping in here to add more context, thank you. After posting this last night and sharing with colleagues, I think we've identified a way to move forwards with a migration to App Router and decouple "controller" and "view" logic and unit test async server components with mocked external services, similar to how we were testing getServerSideProps before. Thanks again!

sawyerh · 2023-12-15T00:39:03Z

@leerob @delbaoliveira Feel free to ignore me here, but wanted to follow-up on my previous comment. Seems like the original pattern we were thinking is currently prevented by the Next.js TypeScript plugin. Added a comment to this existing Next.js issue with more context.

aligg · 2023-12-15T22:50:35Z

app/src/app/[locale]/health/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {


is the /health/ page an aspect of next.js that's built in to handle some particular functionality? I couldn't find it looking in the docs.

My understanding is it's required by the infra template in order to know if the container is healthy, otherwise it spins up a new ECS task.

aligg · 2023-12-15T22:53:57Z

app/src/app/[locale]/page.tsx

+  locale: string;
+}
+
+export async function generateMetadata({ params }: { params: RouteParams }) {


Maybe overkill, but do you think worth linking to this section of the docs? https://nextjs.org/docs/app/api-reference/functions/generate-metadata

I imagine we don't want to reference for everything, but was wondering if for things that are new for server components (which many folks may be less familiar with at first) it could help 🤷‍♀️

Good question. I have a follow-up ticket where I want to add more docs so will fold code comments into that scope since I think it relates. #262

aligg · 2023-12-15T22:55:00Z

app/src/components/Header.tsx

@@ -1,3 +1,5 @@
+"use client";


We need to use client here b/ c the truss stuff?

This component uses useState for hiding/showing the mobile menu

aligg · 2023-12-15T22:56:03Z

app/tests/app/page.test.tsx


-describe("Index", () => {
+describe("Index - Controller", () => {
+  it("retrieves feature flags", async () => {


aligg · 2023-12-15T22:56:38Z

app/tests/app/page.test.tsx

+    render(<View isFooEnabled />);
+    expect(screen.getByText(enabledFlagTextMatcher)).toBeInTheDocument();
+
+    cleanup();


I was wondering how to do this before and couldn't find the cleanup function! Nice to see this.

jcq

Might it make sense to rename the top-level folder from app to something else? It's a bit confusing (especially if someone might be new to the repo or to the concepts of the App Router) to have multiple folders w/ that pseudo-magic app name?

sawyerh · 2023-12-18T21:38:09Z

Might it make sense to rename the top-level folder from app to something else?

@jcq I had a similar question in the spec's Open Questions, and there's some discussion there. I was going to leave it as is for now since the infra template expects app/ out-of-the-box. We could rename in a follow-up though (thoughts on a better name?).

jcq

Looking good — all things about which I had questions can be addressed later in follow-up discussions!

sawyerh added 5 commits November 29, 2023 17:50

Add next-intl

cc425f0

Remove i18next

7f6cd30

Automated changes due to app router being enabled

d391826

Migrate pages and layout to app directory

42c8b02

Use next-intl in components

ccecc89

sawyerh changed the title ~~Sawyerh/66 app router migration~~ Migrate from Next.js Pages Router to App Router Nov 30, 2023

sawyerh added 24 commits November 30, 2023 11:53

Remove unused ts-jest file

825292a

Fix i18n type safety

4c9de34

Migrate tests

1fadabd

Docs progress

d6de227

Setup middleware

fff60ce

Fix layout rendering in Storybook

b82e14d

Remove dupe files

022740f

Fix story import

447a4c5

Reduce diff size

739900c

Restructure i18n directory to support client components

9e20c82

Revert todo

d79bb65

Rename to getMessagesWithFallbacks

6af1c77

Use dynamic import for getting messages

79ca32b

Fix config path

6e759ea

Cleanup

2672191

Migrate API route

6fd126a

Update docs

b4c4c21

Update doc

02e2d93

Reduce diff

2acda08

Fix sticky footer

43e9695

Use timeZone

02b0546

Restore transpilePackages

3bebef7

sawyerh requested review from lorenyu and aligg December 8, 2023 01:31

sawyerh commented Dec 8, 2023

View reviewed changes

app/tsconfig.json

{

"name": "next"

}

]

Copy link

Contributor Author

sawyerh Dec 8, 2023

Choose a reason for hiding this comment

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

ℹ️ Automatically added by Next.js when using App Router

sawyerh added 2 commits December 12, 2023 16:00

Merge remote-tracking branch 'origin/main' into sawyerh/66-app-router…

c2a1c3f

…-migration # Conflicts: # app/package-lock.json # app/package.json # app/src/app/[locale]/page.tsx # app/tests/app/page.test.tsx

Missed merge changes

6c42256

sawyerh marked this pull request as draft December 13, 2023 00:55

sawyerh mentioned this pull request Dec 13, 2023

Migrate from Next.js "pages" router to newer "app" router #66

Closed

sawyerh added 3 commits December 13, 2023 10:02

Merge remote-tracking branch 'origin/main' into sawyerh/66-app-router…

06cd838

…-migration # Conflicts: # app/README.md # app/src/app/[locale]/page.tsx # app/src/components/Layout.tsx # app/src/pages/health.tsx

Seperate controller and view logic, fix tests and story

bd0c2de

Demonstate test of async server component

2d73ff6

Fix named export limitation of page files

ac211f2

sawyerh marked this pull request as ready for review December 15, 2023 21:57

sawyerh requested a review from jcq December 15, 2023 22:00

aligg reviewed Dec 15, 2023

View reviewed changes

aligg approved these changes Dec 15, 2023

View reviewed changes

jcq reviewed Dec 18, 2023

View reviewed changes

jcq approved these changes Dec 19, 2023

View reviewed changes

sawyerh merged commit 799a2ca into main Dec 20, 2023
6 checks passed

sawyerh deleted the sawyerh/66-app-router-migration branch December 20, 2023 17:07

acouch mentioned this pull request Dec 29, 2023

[Issue 877] Refactor Form Submission HHS/simpler-grants-gov#919

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from Next.js Pages Router to App Router #255

Migrate from Next.js Pages Router to App Router #255

sawyerh commented Nov 30, 2023 •

edited

Loading

sawyerh Dec 8, 2023

sawyerh Dec 8, 2023

sawyerh commented Dec 13, 2023

leerob commented Dec 13, 2023 •

edited

Loading

sawyerh commented Dec 13, 2023

sawyerh commented Dec 15, 2023

aligg Dec 15, 2023

sawyerh Dec 15, 2023

aligg Dec 15, 2023

sawyerh Dec 15, 2023

aligg Dec 15, 2023

sawyerh Dec 15, 2023

aligg Dec 15, 2023

aligg Dec 15, 2023

jcq left a comment

sawyerh commented Dec 18, 2023

jcq left a comment

Migrate from Next.js Pages Router to App Router #255

Migrate from Next.js Pages Router to App Router #255

Conversation

sawyerh commented Nov 30, 2023 • edited Loading

Ticket

Changes

Context for reviewers

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawyerh commented Dec 13, 2023

Context

Blockers and concerns

leerob commented Dec 13, 2023 • edited Loading

sawyerh commented Dec 13, 2023

sawyerh commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcq left a comment

Choose a reason for hiding this comment

sawyerh commented Dec 18, 2023

jcq left a comment

Choose a reason for hiding this comment

sawyerh commented Nov 30, 2023 •

edited

Loading

leerob commented Dec 13, 2023 •

edited

Loading