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

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

Closed
sawyerh opened this issue Oct 25, 2022 · 2 comments · Fixed by #255
Closed

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

sawyerh opened this issue Oct 25, 2022 · 2 comments · Fixed by #255
Assignees

Comments

@sawyerh
Copy link
Contributor

sawyerh commented Oct 25, 2022

Resources

The app directory includes support for:

  • Layouts: Easily share UI between routes while preserving state and avoiding expensive re-renders.
  • Server Components: Making server-first the default for the most dynamic applications.
  • Streaming: Display instant loading states and stream in units of UI as they are rendered.
  • Support for Data Fetching: async Server Components and extended fetch API enables component-level fetching.

Additional considerations:

  • The new router works in a new directory named app. Should we rename the top-level directory in our template repo so the directory structure isn't app/app/?
@sawyerh sawyerh changed the title Migrate layout to Next.js v13's new Layout feature Migrate to Next.js v13's new Layout feature Oct 25, 2022
sawyerh added a commit that referenced this issue Nov 1, 2022
## Ticket

Closes #65

## Context for reviewers

Next.js 13 introduced [pretty significant new
functionality](https://nextjs.org/blog/next-13), but it's backwards
compatible with the existing approach implemented in this repo, so
should be safe to upgrade. Projects can adopt its new functionality as
they want. We have a separate ticket for adopting the new functionality
in this repo: #66
@jcq
Copy link
Contributor

jcq commented May 19, 2023

Definitely worth doing now that app router is now stable. Vercel has this as the default, and so should we (it brings a LOT of great things).

As soon as Server Actions are out of beta, we should adopt and implement an example as well

@sawyerh sawyerh changed the title Migrate to Next.js v13's new Layout feature Migrate from Next.js "pages" directory to v13's new "app directory" Jun 30, 2023
@sawyerh sawyerh added Next.js and removed Next.js labels Jun 30, 2023
@sawyerh sawyerh added this to the Next.js App Router milestone Nov 8, 2023
@sawyerh sawyerh changed the title Migrate from Next.js "pages" directory to v13's new "app directory" Migrate from Next.js "pages" router to v13's new "app" router Nov 8, 2023
@sawyerh sawyerh changed the title Migrate from Next.js "pages" router to v13's new "app" router Migrate from Next.js "pages" router to newer "app" router Nov 8, 2023
@sawyerh sawyerh self-assigned this Nov 8, 2023
sawyerh added a commit that referenced this issue Dec 6, 2023
## Ticket

Part of #66 

## Changes

- Replace all I18next dependencies with
[`next-intl`](https://next-intl-docs.vercel.app/)
- Add new `tests/test-utils` file that would be used for rendering and
querying a React tree in tests, rather than directly importing from
`@testing-library/react`. This is so that the i18n content is provided
to the component being tested.

## Context for reviewers

This is one of the main pieces of the larger migration effort from the
Pages router to the App Router (#66). To reduce the size of that PR,
I've broken out the i18n migration work into this PR.

Next.js App Router doesn't support internationalized routing or locale
detection, like Pages router, and `next-intl` makes it easy to restore
that functionality via middleware.
sawyerh added a commit that referenced this issue Dec 7, 2023
## Ticket

#66 

## Changes

- Use a [dynamic
import](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import)
to load all strings for the selected locale. This simplifies the process
of adding a new language — now someone just needs to add the locale code
(e.g. `es-US`) to the `locales` array, whereas before they'd also have
to add an import statement.
- Renamed `getLocaleMessages` to `getMessagesWithFallbacks` and moved it
to its own file. A few reasons for this:
(1) Project engineers shouldn't ever need to interact with the
`getMessagesWithFallbacks` logic — it should Just Work. (2) We can now
rename `i18n/index.ts` to `i18n/config.ts` to make it clearer what the
purpose of that file is.

## Context for reviewers

I'm pulling this out from the larger App Router migration branch.
There's an additional piece that will be in that branch, resulting in a
final directory structure looking something like this:

```
├── i18n
│   ├── config.ts     # Supported locales and formatting options
│   ├── getMessagesWithFallbacks.ts 
│   └── server.ts     # 🆕 next-intl setup logic that can't be imported into client components
```

We could expand this also to export the safelisted subset of
hooks and components

```
├── i18n
│   ├── config.ts     # Supported locales and formatting options
│   ├── getMessagesWithFallbacks.ts 
│   ├── index.ts      # 🆕 Hooks and components for rendering content
│   └── server.ts     # next-intl setup logic that can't be imported into client components
```
@sawyerh
Copy link
Contributor Author

sawyerh commented Dec 13, 2023

I have a draft PR with the migration implementation, but I've also added a comment to it with context on a blocker and recommendation to pause the migration until tooling catches up. #255 (comment)

sawyerh added a commit that referenced this issue Dec 20, 2023
## 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](https://github.com/navapbc/template-application-nextjs/blob/main/docs/decisions/app/0008-app-router.md)
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](vercel/next.js#53940 (comment)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants