[Issue #134] Renable static rendering #85
Conversation
|
Not sure why pa11y is failing on the search page - I wonder if because part of the layout is static, but the actual page is dynamic it causes some problems - but also there is another PR that @btabaska is working on to have pa11y load the api on its runs (so we test the search results in a11y tests). I think I pulled this PR down the other day and the search page does work though - just with a red alert error because no api. |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟢 | Statements | 83.72% (-0.15% 🔻) |
756/903 |
| 🟡 | Branches | 66.99% (-1.26% 🔻) |
207/309 |
| 🟡 | Functions | 77.07% (+0.97% 🔼) |
158/205 |
| 🟢 | Lines | 83.83% (+0.37% 🔼) |
710/847 |
Show files with reduced coverage 🔻
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🟢 | ... / Header.tsx |
95.45% | 62.5% (-12.5% 🔻) |
100% | 95.24% |
| 🟢 | ... / breadcrumbs.ts |
70% (-25% 🔻) |
100% | 100% | 100% |
Test suite run success
158 tests passing in 49 suites.
Report generated by 🧪jest coverage report action from 6435c62
| @@ -8,7 +8,7 @@ import PageSEO from "src/components/PageSEO"; | |||
| import BetaAlert from "src/components/BetaAlert"; | |||
| import { useTranslations } from "next-intl"; | |||
| import { Metadata } from "next"; | |||
| import { getTranslations } from "next-intl/server"; | |||
| import { getTranslations, unstable_setRequestLocale } from "next-intl/server"; | |||
There was a problem hiding this comment.
Should we add a code comment here that unstable_setRequestLocale is a temporary API that next-intl aims to remove it in the future?
There was a problem hiding this comment.
…and that, per their docs, you must be "cautious to apply it in all relevant places"
There was a problem hiding this comment.
Maybe we make a ticket for dealing with this in the future?
There was a problem hiding this comment.
The name unstable_ is a common convention and speaks for itself, and this is used in every page, so would opt not to comment in the code. I'll create a ticket to remove it. It could be a while based on amannn/next-intl#663
Summary
Fixes #134
Time to review: 10 mins
Changes proposed
With the move to the app router, pages were no longer statically rendered which meant that the server re-rendered the pages every time a new user visits the site which is bad for performance and not necessary for static HTML. This fixes that.
This removes the unit tests for pages because of an error we got
"unstable_setRequestLocale" is not supported in Client Components.which is not true, but broke the unit tests. There is coverage for pages in the e2e tests.