[Issue #2622] Update opp page for ISR #2724
Conversation
acouch
force-pushed
the
acouch/opp-page-isr
branch
from
92c53ac
to
12aab7f
Compare
acouch
force-pushed
the
acouch/opp-page-isr
branch
from
af3ed1d
to
060650e
Compare
There was a problem hiding this comment.
love the added testing here. Not sure how I should be testing locally though. Here is what I'm seeing on load times if we want to base the AC on that. FWIW particularly on the production build it seems to be an improvement over the current timings
- main
npm run dev(1st load, and refresh)
- main (
npm run build && npm start)
-
this branch (1st load and refresh)
-
this branch (
npm run build && npm start)
| @@ -41,6 +42,10 @@ export async function generateMetadata({ params }: { params: { id: string } }) { | |||
| return meta; | |||
| } | |||
|
|
|||
| export function generateStaticParams() { | |||
| return []; | |||
There was a problem hiding this comment.
Ok so this seems to be the configuration that this change sets up:
- no pages are built and pre-cached at build time
- all pages are rendered the on their first hit within a 10 minute window, then cached for 10 minutes
Is that right? Is the idea to try this out and evaluate how it works, or are there plans for next steps, such as as generating a list of all current opportunities at build time and pre-caching? Is the 10 minute window a ballpark estimate at this point that we will analyze in some way after launch and adjust, or is that based on data that leads us to believe it should be a pretty solid permanent setting?
There was a problem hiding this comment.
Is that right? Is the idea to try this out and evaluate how it works, or are there plans for next steps, such as as generating a list of all current opportunities at build time and pre-caching?
Yes. We could certainly to that in the future. We don't have a good way of getting all of the opp ids. We might want an API for that in the future.
Is the 10 minute window a ballpark estimate at this point that we will analyze in some way after launch and adjust, or is that based on data that leads us to believe it should be a pretty solid permanent setting?
The file links refresh every 30 mins, so that is the higher end. The 10 mins is something we can test and adjust in the future.
| @@ -127,5 +136,3 @@ async function OpportunityListing({ params }: { params: { id: string } }) { | |||
| </div> | |||
| ); | |||
| } | |||
|
|
|||
| export default withFeatureFlag(OpportunityListing, "showSearchV0"); | |||
There was a problem hiding this comment.
when we merge, worth noting in the commit that this change is also included in case anyone wants to track it down later
Summary
Fixes #2622
Time to review: 5 mins
Changes proposed
Add ISR for opp pages.
Testing locally
npm run build && npm run start