[Issue #1657] Setup Next-Intl for App Pages #1851

acouch · 2024-04-26T16:05:41Z

Summary

Time to review: 30 mins

Changes proposed

Enables next-intl to enable translation for app pages. This sets up next-intl and enables it for the search page and app not found page. This adds one other app component. There are three which will be removed for #1361 .

Context for reviewers

The strings being declared and injected is still ugly and not addressed with this, but will be fixed with #1361 .

The [locale] folder has not been setup yet b/c it took over the full routing for the site. That folder will be setup in #1361 .

acouch · 2024-04-26T16:48:46Z

frontend/src/app/help/page.tsx

+  return meta;
+}
+
+export default async function Help() {


You use useTransations() and useMessages() if it is a sync function.

Believe all use* hooks are typically only be used by client components (useSearchParams, useEffect, useState). So its a bit weird to see next-intl using them in server components https://next-intl-docs.vercel.app/docs/environments/server-client-components

acouch · 2024-04-26T16:49:16Z

frontend/src/app/help/page.tsx

@@ -0,0 +1,54 @@
+import { Metadata } from "next";


This page was just for playing around, will delete.

acouch · 2024-04-30T16:09:37Z

This is green except for a single test for the AppLayout.tsx:

Error: `getTranslations` is not supported in Client Components.
    at /home/runner/work/simpler-grants-gov/simpler-grants-gov/frontend/node_modules/next-intl/dist/development/server/react-client/index.js:15:11
    at Layout (/home/runner/work/simpler-grants-gov/simpler-grants-gov/frontend/src/components/AppLayout.tsx:399:63)
    at renderWithHooks (/home/runner/work/simpler-grants-gov/simpler-grants-gov/frontend/node_modules/react-dom/cjs/react-dom.development.js:16305:18)

It is not a client component AFAICT, so not sure what the deal here is.

rylew1 · 2024-04-30T23:51:29Z

This is green except for a single test for the AppLayout.tsx:

Error: `getTranslations` is not supported in Client Components.
    at /home/runner/work/simpler-grants-gov/simpler-grants-gov/frontend/node_modules/next-intl/dist/development/server/react-client/index.js:15:11
    at Layout (/home/runner/work/simpler-grants-gov/simpler-grants-gov/frontend/src/components/AppLayout.tsx:399:63)
    at renderWithHooks (/home/runner/work/simpler-grants-gov/simpler-grants-gov/frontend/node_modules/react-dom/cjs/react-dom.development.js:16305:18)
It is not a client component AFAICT, so not sure what the deal here is.

I console logged in AppLayout and it only showed on the server. Wonder if the new next-intl changes are properly configured to support server components or otherwise causing things to rendier client side?

Also wondering if AppLayout needs to be an async component?

I'm not fully capacity at the moment but I should be able to help debug this better on Thursday if it's still an issue.

github-actions · 2024-05-01T02:02:46Z

Coverage report for `./frontend`

St.^❔	Category	Percentage	Covered / Total
🟢	Statements	84.06% (+0.17% 🔼)	870/1035
🟡	Branches	65.12% (+0.22% 🔼)	224/344
🟡	Functions	75.57% (+0.22% 🔼)	167/221
🟢	Lines	84.11% (+0.22% 🔼)	810/963

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / react-utils.tsx	100%	100%	100%	100%
🟢	src/i18n/config.ts	87.5%	100%	100%	100%
🟢	`...` / index.ts	100%	100%	100%	100%

Test suite run success

166 tests passing in 55 suites.

Report generated by 🧪jest coverage report action from c090e9b

acouch · 2024-05-01T02:04:23Z

frontend/src/components/AppBetaAlert.tsx

+
+const BetaAlert = () => {
+  const t = useTranslations("Beta_alert");
+  const heading = t.rich("alert_title", {


This file was added until #1361

Should we add a comment in the code that points to that ticket? Format we've been using is // TODO (Issue #1361): ....

Same thing for AppLayout file or other places

acouch · 2024-05-01T02:05:40Z

frontend/src/components/AppLayout.tsx

-  // TODO: Remove during move to app router and next-intl upgrade
+export default function Layout({ children, locale }: Props) {
+  const t = useTranslations();
+
  const header_strings = {


These strings will be removed once we moved the app router and are no longer using both i18n and next-intl in #1361

acouch · 2024-05-06T19:42:18Z

Met with @rylew1 , want to look more closely at how the browser locale works and how to only allow the selected locales in the routing layer.

acouch · 2024-05-06T20:26:29Z

@rylew1 The PR does remove non language routes, I just had the [locale] folder in place locally w/o the rest of the config. The browser locale docs explain the browser config strategy. Think we are good to go.

Add initial next-intl

cd6b56a

github-actions bot added frontend typescript javascript labels Apr 26, 2024

acouch mentioned this pull request Apr 26, 2024

[Issue #1657] - Enable i18n for App Router #1846

Closed

acouch commented Apr 26, 2024

View reviewed changes

acouch added 5 commits April 26, 2024 12:54

Remove test file

1a54c75

Fix some linting errors

725cf23

Update for linting and formatting

25c1b60

Update server.ts for ts format error

660ff52

Update for dynamic routing

9941fb8

acouch force-pushed the acouch/issue-1657-next-intl branch from 627928b to 9941fb8 Compare April 30, 2024 15:20

Update AppLayout.tsx

9f74e32

Update AppLayout.tsx

224d6a3

acouch force-pushed the acouch/issue-1657-next-intl branch from a866c45 to 224d6a3 Compare April 30, 2024 18:57

Adds react-utils and updates tests

5d95c3b

acouch commented May 1, 2024

View reviewed changes

acouch added 2 commits April 30, 2024 22:28

Update layout.tsx

4013e42

Update not-found.tsx to add t()

3afbf5d

acouch force-pushed the acouch/issue-1657-next-intl branch from 61ab471 to 3afbf5d Compare May 1, 2024 02:46

acouch marked this pull request as ready for review May 1, 2024 02:51

acouch requested review from andycochran and SammySteiner as code owners May 1, 2024 02:51

Merge branch 'main' into acouch/issue-1657-next-intl

c090e9b

rylew1 approved these changes May 6, 2024

View reviewed changes

acouch merged commit 35d6fe5 into main May 7, 2024
10 checks passed

acouch deleted the acouch/issue-1657-next-intl branch May 7, 2024 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Issue #1657] Setup Next-Intl for App Pages #1851

[Issue #1657] Setup Next-Intl for App Pages #1851

acouch commented Apr 26, 2024 •

edited

Loading

acouch Apr 26, 2024

rylew1 Apr 30, 2024 •

edited

Loading

acouch Apr 26, 2024

acouch commented Apr 30, 2024

rylew1 commented Apr 30, 2024 •

edited

Loading

github-actions bot commented May 1, 2024 •

edited

Loading

acouch May 1, 2024

rylew1 May 6, 2024 •

edited

Loading

acouch May 1, 2024

acouch commented May 6, 2024

acouch commented May 6, 2024

[Issue #1657] Setup Next-Intl for App Pages #1851

[Issue #1657] Setup Next-Intl for App Pages #1851

Conversation

acouch commented Apr 26, 2024 • edited Loading

Summary

Time to review: 30 mins

Changes proposed

Context for reviewers

acouch Apr 26, 2024

Choose a reason for hiding this comment

rylew1 Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

acouch Apr 26, 2024

Choose a reason for hiding this comment

acouch commented Apr 30, 2024

rylew1 commented Apr 30, 2024 • edited Loading

github-actions bot commented May 1, 2024 • edited Loading

Coverage report for ./frontend

Test suite run success

acouch May 1, 2024

Choose a reason for hiding this comment

rylew1 May 6, 2024 • edited Loading

Choose a reason for hiding this comment

acouch May 1, 2024

Choose a reason for hiding this comment

acouch commented May 6, 2024

acouch commented May 6, 2024

acouch commented Apr 26, 2024 •

edited

Loading

rylew1 Apr 30, 2024 •

edited

Loading

rylew1 commented Apr 30, 2024 •

edited

Loading

github-actions bot commented May 1, 2024 •

edited

Loading

Coverage report for `./frontend`

rylew1 May 6, 2024 •

edited

Loading