-
Notifications
You must be signed in to change notification settings - Fork 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
Upgrade nextjs #109
Upgrade nextjs #109
Conversation
|
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.
LGTM, asked few queries. I deployed it to staging as well, seems to be working fine - https://scriptified-technetium-co.vercel.app/
Side Note: We should also think upon updating other dependencies like next-pwa
once.
@@ -92,6 +92,10 @@ function Curators(): JSX.Element { | |||
width={150} | |||
height={150} | |||
alt={curator.name} | |||
style={{ |
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.
- these inline styles were added by codemod?
- the codemod didn't changed our imports?
The next/image import was renamed to next/legacy/image. The next/future/image import was renamed to next/image. A codemod is available to safely and automatically rename your imports.
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.
Ran to codemods, first one converted the imports to legacy imports and another experimental codemod that converts legacy imports to new imports and updates the syle according to the new API.
https://nextjs.org/docs/advanced-features/codemods
<Link href="/" className="no-underline hover:underline"> | ||
← Back to home |
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.
also, I think we should redirect user to last visited page in the stack, not directly to Home.
If one goes to "All Issues" page, then open an issue, & then click this - it redirects back to home. not good UX.
let's take it up separately. I'll open an issue to remember.
@@ -1,10 +1,5 @@ | |||
/* eslint-disable @typescript-eslint/no-var-requires */ | |||
// eslint-disable-next-line @typescript-eslint/no-var-requires | |||
// @ts-check |
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.
why did we removed this?
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.
Was getting a bunch of type errors maybe because of the withPWA
conflicting with the original types. We can fix this separately though
@@ -21,13 +22,13 @@ | |||
"axios": "0.21.2", | |||
"glob": "7.1.6", | |||
"lodash": "4.17.21", | |||
"next": "12.2.0", | |||
"next": "13.1.2", |
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.
🚀
Upgrade to Next.js 13 and apply relevant codemods using this guide:
https://nextjs.org/docs/upgrading