-
Notifications
You must be signed in to change notification settings - Fork 3
Prerender resources pages #7
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
Conversation
let resources = await getAllResources({ octokit }); | ||
return { | ||
resources, | ||
featuredResource: null, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This used to have all the selectedCategory
/selectedTags
stuff - moved to a clientLoader
because SSG won't have query params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha yeah, going back to the server for this doesn't even super make sense, since we have to send down all the data anyway and it's not changing in between deploys
Cool that await serverLoader();
just grabs whatever the prerender phase created. Never really thought about it but makes total sense
return { ...resource, siteUrl }; | ||
}; | ||
|
||
export async function clientLoader({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty much everything in here is just copied from what we used to do on the server
<main> | ||
{show ? ( | ||
<div className="container flex flex-1 flex-col items-center md:mt-8"> | ||
<p>Loading resources....</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flash in a loading message if they stare at a blank screen for >2 seconds which should really never happen
// Force this layout HydrateFallback to render on initial load so we can | ||
// eliminate the footer and avoid needing to fill height with a skeleton | ||
export function clientLoader() { | ||
return null; | ||
} | ||
clientLoader.hydrate = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a bug or a gap in RR - I thought we bubbled to the nearest HydrateFallback
but it looks like it's 1:1 so just having the clientLoader.hydrate
on the child route is not enough to get this route HydrateFallback
to show. I wanted to do it at this level so we didn't have the footer collapse up on us when there was no content and cause a flicker. By doing it here I can just hide the footer until the content is ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm yeah, feels like it should work the way you expected. Either way, this approach makes sense
@@ -0,0 +1,2 @@ | |||
# A token to increase the rate limiting from 60/hr to 1000/hr | |||
GITHUB_TOKEN="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I missing something? I added this to my .env
file but build fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh - yeah I must have exported it locally in the shell during my testing - added dotenv
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It all works well for me!
Thanks for helping preserve my first (or second?) contribution on this team
I think the only things that need to be fixed are
- loading in the
GITHUB_GITHUB_TOKEN
toprocess.env
on build - adding that token on to GitHub for deployment
Went ahead and fixed some type errors btw @brophdawg11 |
2757dea
to
3fcccbe
Compare
@brophdawg11 thanks for fixing that up and adding the nav. I'm good to merge this whenever you are! Also happy to cleanup the |
I'll take a stab at it - it's a nice easy/defined piece of work I can do before vacation :) |
Mostly copied stuff straight over - just had to move the filtering to a
clientLoader