Conversation
|
gupta-ji6
left a comment
There was a problem hiding this comment.
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.
| width={150} | ||
| height={150} | ||
| alt={curator.name} | ||
| style={{ |
There was a problem hiding this comment.
- 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.
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.
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.
Was getting a bunch of type errors maybe because of the withPWA conflicting with the original types. We can fix this separately though
| "glob": "7.1.6", | ||
| "lodash": "4.17.21", | ||
| "next": "12.2.0", | ||
| "next": "13.1.2", |
Upgrade to Next.js 13 and apply relevant codemods using this guide:
https://nextjs.org/docs/upgrading