Upgrade to next 13 and react 18
therungg opened this issue · 3 comments
I've already tried, but got a million React Hydration mismatches, which I wasn't sure how to fix exactly. Also want to do it properly, and use all the new next js stuff
I'm going to take this on and also document my findings for others to follow along:
The upgrade to Next13 and React 18 are straight forward. Just npm install and we're most of the way done. Next has codemods that handle the vast majority of the code changes for the upgrade. Lastly, updating a few dependencies to make sure everything works as expected since some older versions rely on React <= 17 or Next <= 12.
- I encountered the hydration issues as you mentioned and I think that after some digging that it's all related to the
baseUrl
mutable export in_app.tsx
. First approach to fixing this would be to include the baseUrl in thegetInitialProps
return and include the baseUrl to a newAppContext
object that we can use across the app safely. Only downside to this approach is that it is exclusively accessible from within React components.
Alternatively, I'm thinking we could do an overhaul on how http requests are done across the application and have a centralized point of both retrieving and creating apiClients.
For example:
export const createFetch = (baseUrl: string) => {
return async (endpoint: string, options: RequestInit = {}) => {
const response = await fetch(`${baseUrl}${endpoint}`, options);
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
return await response.json();
};
};
export const twitchAuthClient = createFetch('https://id.twitch.tv/oauth2')
export const theRunClient = createFetch('https://therun.gg')
This is just an idea. There are loads of other approaches to achieve this but considering how much code would need to change for I figured a conversation is more warranted to see what others think before I get too deep into implementation.
Yeah, I saw that baseUrl global and wondered if it shouldn't be switched to using environment variables, but I've yet to test it. Something like this
export function getBaseUrl() {
if (process.env.NEXT_PUBLIC_VERCEL_ENV === "production")
return "https://therun.gg";
if (process.env.NEXT_PUBLIC_VERCEL_ENV === "preview")
return `https://${process.env.NEXT_PUBLIC_VERCEL_URL}`; //Get the generated slug
return "http://localhost:3000";
}
Yes! When I read frank's comment, I remembered I wanted to put this in environment variables but had just forgotten. dotneB's solution should be fine, I was thinking to just put the url in the env variable, but that won't do for the dynamically generated preview environments.
@francis-robert Refactoring how all the fetches are done in the application is a good idea, but out of scope for this issue IMO.