This repository has been archived by the owner on Sep 18, 2024. It is now read-only.
Move Pages to App Router #7
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟢 | Statements | 83.23% (-0.94% 🔻) |
844/1014 |
| 🟡 | Branches | 68.31% (+3.19% 🔼) |
222/325 |
| 🟡 | Functions | 75.46% (-0.11% 🔻) |
163/216 |
| 🟢 | Lines | 82.78% (-1.44% 🔻) |
793/958 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / actions.ts |
57.14% | 100% | 0% | 57.14% |
| 🟡 | ... / page.tsx |
75% | 100% | 50% | 75% |
| 🟢 | ... / NewsletterForm.tsx |
93.75% | 78.57% | 87.5% | 93.62% |
| 🔴 | ... / page.tsx |
57.14% | 100% | 50% | 57.14% |
| 🟢 | ... / FeatureFlagsTable.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / USWDSIcon.tsx |
100% | 100% | 100% | 100% |
| 🟡 | ... / page.tsx |
69.23% | 100% | 50% | 69.23% |
| 🟢 | ... / IndexGoalContent.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ProcessAndResearchContent.tsx |
100% | 100% | 100% | 100% |
| 🟡 | ... / page.tsx |
73.33% | 100% | 50% | 73.33% |
| 🟢 | ... / ProcessIntro.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ProcessInvolved.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ProcessMilestones.tsx |
94.74% | 66.67% | 100% | 94.44% |
| 🟡 | ... / page.tsx |
76.47% | 100% | 50% | 76.47% |
| 🟢 | ... / ResearchIntro.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ResearchArchetypes.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ResearchImpact.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ResearchMethodology.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / ResearchThemes.tsx |
100% | 100% | 100% | 100% |
| 🟢 | ... / FundingContent.tsx |
100% | 100% | 100% | 100% |
| 🟡 | ... / page.tsx |
71.43% | 100% | 50% | 71.43% |
| 🟡 | ... / page.tsx |
71.43% | 100% | 50% | 71.43% |
Test suite run success
158 tests passing in 54 suites.
Report generated by 🧪jest coverage report action from c40b27b
rylew1
reviewed
May 15, 2024
| @@ -0,0 +1,51 @@ | |||
| import { Grid } from "@trussworks/react-uswds"; | |||
| import { useTranslations, useMessages } from "next-intl"; | |||
| import ContentLayout from "src/components/ContentLayout"; | |||
There was a problem hiding this comment.
Should process have its own folder under the component folder?
|
we probably want these changes to run against the updated e2e search.spec file |
acouch
force-pushed
the
acouch/issue-6-move-pages-app
branch
from
d2012e4 to
39f115d
Compare
acouch
force-pushed
the
acouch/issue-6-move-pages-app
branch
from
39f115d to
fab1ffa
Compare
rylew1
approved these changes
May 23, 2024
acouch
added a commit
that referenced
this pull request
Sep 18, 2024
Fixes #6 1. Move all pages to [`[locale]`](https://next-intl-docs.vercel.app/docs/getting-started/app-router/with-i18n-routing#getting-started) folder 2. Add [`generateMetata()`](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#generatemetadata-function) function and [next-intl `getTranslations()`](https://next-intl-docs.vercel.app/docs/environments/metadata-route-handlers#metadata-api) implementation * @rylew1 commented we could remove this from each page. To do that we could use [prop arguments](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#with-segment-props) and update the based on the param. There is also more we can do with the metadata to properly add [app links and twitter cards](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#applinks). TODO: create ticket 4. Replace i18n's `useTranslation` with next-intl's `useTranslations` 5. Remove hard-coded strings that were present b/c we were still b/w i18next and next-intl * [Move process page to app](32ba4ee) * [Move research page to app](5b5ad1a) * [Move health page to app](a3e6255) * [Move feature flag page to app](395baed) * [Move search page to app router](1e261e3) * [Move newsletter pages to app router](b509ef8) * [Move home page to app router](de1be98) * [Move home page to app router](74077ae) * [Move 404 page to app router](ccbc956) 1. [Delete hello api](5bad6ea) * This was left from the project creation 2. [Add USWDS icon component](0120c7b) * as noted in a slack discussion, when trying to access [one of the icons](https://github.com/trussworks/react-uswds/blob/main/src/components/Icon/Icons.ts) using `<Icon.Search/>` next errors: `You cannot dot into a client module from a server component. You can only pass the imported name through`. I'm not sure why it thinks the Icon component is a client module. [Dan A. suggests](vercel/next.js#51593 (comment)) trussworks should re-export as named exports. I tried importing the SVGs directly from the trussworks library but [svgr requires a custom webpack config](https://react-svgr.com/docs/next/) which is a road I didn't want to go down and [react svg](https://www.npmjs.com/package/react-svg) through an error in the app router 😥 . * I implemented @sawyerh 's [suggestion](0120c7b#diff-dadb35bd2f3f61f2c179f033cd0a2874fc343974236f2fb8613664703c751429), which did not work initially b/c next reported the USWDS icon was corrupt, which was fixed by adding a `viewBox` to the svg element 😮💨 . * [Remove unused WtGIContent](75490f7) * [Move layout and update for app router](af112fd) * [Update global components for the app router](40119e6) * [Move i18n strings for app router](eb3c07c) * [Adds next-intl config and removes i18n](c546571) * [Update tests for app router](3b9b193) * [Removes i18next and next-i18n packages](9d2e08a) * [Update storybook settings for app router](39f115d)
acouch
added a commit
that referenced
this pull request
Sep 18, 2024
Fixes #6 1. Move all pages to [`[locale]`](https://next-intl-docs.vercel.app/docs/getting-started/app-router/with-i18n-routing#getting-started) folder 2. Add [`generateMetata()`](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#generatemetadata-function) function and [next-intl `getTranslations()`](https://next-intl-docs.vercel.app/docs/environments/metadata-route-handlers#metadata-api) implementation * @rylew1 commented we could remove this from each page. To do that we could use [prop arguments](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#with-segment-props) and update the based on the param. There is also more we can do with the metadata to properly add [app links and twitter cards](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#applinks). TODO: create ticket 4. Replace i18n's `useTranslation` with next-intl's `useTranslations` 5. Remove hard-coded strings that were present b/c we were still b/w i18next and next-intl * [Move process page to app](32ba4ee) * [Move research page to app](5b5ad1a) * [Move health page to app](a3e6255) * [Move feature flag page to app](395baed) * [Move search page to app router](1e261e3) * [Move newsletter pages to app router](b509ef8) * [Move home page to app router](de1be98) * [Move home page to app router](74077ae) * [Move 404 page to app router](ccbc956) 1. [Delete hello api](5bad6ea) * This was left from the project creation 2. [Add USWDS icon component](0120c7b) * as noted in a slack discussion, when trying to access [one of the icons](https://github.com/trussworks/react-uswds/blob/main/src/components/Icon/Icons.ts) using `<Icon.Search/>` next errors: `You cannot dot into a client module from a server component. You can only pass the imported name through`. I'm not sure why it thinks the Icon component is a client module. [Dan A. suggests](vercel/next.js#51593 (comment)) trussworks should re-export as named exports. I tried importing the SVGs directly from the trussworks library but [svgr requires a custom webpack config](https://react-svgr.com/docs/next/) which is a road I didn't want to go down and [react svg](https://www.npmjs.com/package/react-svg) through an error in the app router 😥 . * I implemented @sawyerh 's [suggestion](0120c7b#diff-dadb35bd2f3f61f2c179f033cd0a2874fc343974236f2fb8613664703c751429), which did not work initially b/c next reported the USWDS icon was corrupt, which was fixed by adding a `viewBox` to the svg element 😮💨 . * [Remove unused WtGIContent](75490f7) * [Move layout and update for app router](af112fd) * [Update global components for the app router](40119e6) * [Move i18n strings for app router](eb3c07c) * [Adds next-intl config and removes i18n](c546571) * [Update tests for app router](3b9b193) * [Removes i18next and next-i18n packages](9d2e08a) * [Update storybook settings for app router](39f115d)
acouch
added a commit
to HHS/simpler-grants-gov
that referenced
this pull request
Sep 18, 2024
Fixes #6 1. Move all pages to [`[locale]`](https://next-intl-docs.vercel.app/docs/getting-started/app-router/with-i18n-routing#getting-started) folder 2. Add [`generateMetata()`](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#generatemetadata-function) function and [next-intl `getTranslations()`](https://next-intl-docs.vercel.app/docs/environments/metadata-route-handlers#metadata-api) implementation * @rylew1 commented we could remove this from each page. To do that we could use [prop arguments](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#with-segment-props) and update the based on the param. There is also more we can do with the metadata to properly add [app links and twitter cards](https://nextjs.org/docs/app/api-reference/functions/generate-metadata#applinks). TODO: create ticket 4. Replace i18n's `useTranslation` with next-intl's `useTranslations` 5. Remove hard-coded strings that were present b/c we were still b/w i18next and next-intl * [Move process page to app](navapbc@32ba4ee) * [Move research page to app](navapbc@5b5ad1a) * [Move health page to app](navapbc@a3e6255) * [Move feature flag page to app](navapbc@395baed) * [Move search page to app router](navapbc@1e261e3) * [Move newsletter pages to app router](navapbc@b509ef8) * [Move home page to app router](navapbc@de1be98) * [Move home page to app router](navapbc@74077ae) * [Move 404 page to app router](navapbc@ccbc956) 1. [Delete hello api](navapbc@5bad6ea) * This was left from the project creation 2. [Add USWDS icon component](navapbc@0120c7b) * as noted in a slack discussion, when trying to access [one of the icons](https://github.com/trussworks/react-uswds/blob/main/src/components/Icon/Icons.ts) using `<Icon.Search/>` next errors: `You cannot dot into a client module from a server component. You can only pass the imported name through`. I'm not sure why it thinks the Icon component is a client module. [Dan A. suggests](vercel/next.js#51593 (comment)) trussworks should re-export as named exports. I tried importing the SVGs directly from the trussworks library but [svgr requires a custom webpack config](https://react-svgr.com/docs/next/) which is a road I didn't want to go down and [react svg](https://www.npmjs.com/package/react-svg) through an error in the app router 😥 . * I implemented @sawyerh 's [suggestion](navapbc@0120c7b#diff-dadb35bd2f3f61f2c179f033cd0a2874fc343974236f2fb8613664703c751429), which did not work initially b/c next reported the USWDS icon was corrupt, which was fixed by adding a `viewBox` to the svg element 😮💨 . * [Remove unused WtGIContent](navapbc@75490f7) * [Move layout and update for app router](navapbc@af112fd) * [Update global components for the app router](navapbc@40119e6) * [Move i18n strings for app router](navapbc@eb3c07c) * [Adds next-intl config and removes i18n](navapbc@c546571) * [Update tests for app router](navapbc@3b9b193) * [Removes i18next and next-i18n packages](navapbc@9d2e08a) * [Update storybook settings for app router](navapbc@39f115d)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6
Time to review: 60 mins
Changes proposed
Move pages from page to app router:
[locale]foldergenerateMetata()function and next-intlgetTranslations()implementationuseTranslationwith next-intl'suseTranslationsChanges
Move process page to app
Move research page to app
Move health page to app
Move feature flag page to app
Move search page to app router
Move newsletter pages to app router
Move home page to app router
Move home page to app router
Move 404 page to app router
Misc
<Icon.Search/>next errors:You cannot dot into a client module from a server component. You can only pass the imported name through. I'm not sure why it thinks the Icon component is a client module. Dan A. suggests trussworks should re-export as named exports. I tried importing the SVGs directly from the trussworks library but svgr requires a custom webpack config which is a road I didn't want to go down and react svg through an error in the app router 😥 .viewBoxto the svg element 😮💨 .Layout and component updates
Move layout and update for app router
Update global components for the app router
Remaining next-intl config and removal of
Move i18n strings for app router
Adds next-intl config and removes i18n
Update tests for app router
Removes i18next and next-i18n packages
Update storybook settings for app router
Next Steps
<PageSEO/>componentContext for reviewers
[locale]folder was not implemented, and when the[locale]folder was implemented, it broke the routing for the non-app pages