Skip to content
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

chore: upgrade to next 15 #7155

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

chore: upgrade to next 15 #7155

wants to merge 11 commits into from

Conversation

ovflowd
Copy link
Member

@ovflowd ovflowd commented Oct 30, 2024

This PR attempts to upgrade to Next.js 15 with the following changes:

  • Updated also dependencies, such as next-intl to the latest version, which required refactoring on its usage for compliance
  • Updated all our dynamic segments and routes to be more performant (build only what is really needed in a minimal build, actually make it compatible with static exports)
    • We avoid building OG on static exports as it is costly to compute. Static exports should simply not support OG images at the moment.
  • Updates usages of JSON imports to use with instead of assert
  • Updated all other dependencies and fixed issues as needed

Copy link

vercel bot commented Oct 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Nov 4, 2024 2:26pm

@ovflowd

This comment was marked as resolved.

@ovflowd

This comment was marked as resolved.

@ovflowd

This comment was marked as resolved.

@AugustinMauroy

This comment was marked as resolved.

@amannn

This comment was marked as resolved.

@ovflowd

This comment was marked as resolved.

@ovflowd

This comment was marked as resolved.

@ovflowd

This comment was marked as resolved.

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Oct 31, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Oct 31, 2024
Copy link

Running Lighthouse audit...

@ovflowd
Copy link
Member Author

ovflowd commented Oct 31, 2024

EDIT: it's work if you follow the next-intl docs

@AugustinMauroy Yep, switching to await requestLocale can help you to get rid of a Next.js 15 warning.

In combination, you probably want to adopt createNavigation here:

import { createSharedPathnamesNavigation } from 'next-intl/navigation';
import { availableLocaleCodes } from './next.locales.mjs';
export const { Link, redirect, usePathname, useRouter } =
createSharedPathnamesNavigation({ locales: availableLocaleCodes });

These are the two changes that are relevant for next-intl to work with Next.js 15 without any warnings.

You might want to give the next-intl 3.22 release notes a look, I think there are other relevant deprecations that are relevant for this project.

Let me know if I can help with something!

@amannn thanks for the heads-up! I believe I've adjusted everything accordingly, feel free if you still have suggestions :)

@eps1lon
Copy link

eps1lon commented Oct 31, 2024

We are getting so many ESlint errors like these, any ideas?

These look like TypeScript issues due to duplicate versions of @types/react.

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Oct 31, 2024
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Oct 31, 2024
@ovflowd
Copy link
Member Author

ovflowd commented Oct 31, 2024

We are getting so many ESlint errors like these, any ideas?

These look like TypeScript issues due to duplicate versions of @types/react.

I have the feeling something was wrong with the package-lock.json? Deleting the file, regenerating and running specifically:

rm package-lock.json
npm i --force
npm update --force --save

fixed all the issues (linting and builds are passing locally now)

@ovflowd ovflowd marked this pull request as ready for review October 31, 2024 20:45
@ovflowd ovflowd requested review from a team as code owners October 31, 2024 20:45
@ovflowd
Copy link
Member Author

ovflowd commented Oct 31, 2024

Hmm, @amannn is there something wrong with our config? The build shows https://nodejs-org-git-chore-nextjs-15-openjs.vercel.app/en but running locally with npm run dev works (?) any clue from your side?

I also see that some translations (Header and Footer not working)

// In this case we want to catch-all possible requests. This ensures that we always generate and
// serve the OpenGrapgh images independently on the locale
// @see https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config#dynamicparams
export const dynamicParams = true;

// Enforces that this route is used as static rendering
// @see https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config#dynamic
export const dynamic = ENABLE_STATIC_EXPORT ? 'force-static' : 'auto';
export const dynamic = 'auto';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmuenzenmeyer we have an issue with this because this is the only endpoint failing build for static exports.

This endpoint is just incompatible with static exports. What now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also mean this whole time OG generation was supposed to be broken for static exports... Which we shouldn't break.

What should we do here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see this had since changed - i am just now looking at it - lemme know if it's still a concern. the original implementation of next og work referenced tutorials that didn't really help much as it pertained to our combinations of use cases

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still a concern.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixing this!

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Oct 31, 2024
@ovflowd
Copy link
Member Author

ovflowd commented Nov 4, 2024

Everything seems to be working right now except:

  • Internationalization (seems not to be kicking it. Able to reproduce on dev mode, and prod builds)
  • Storybook is not compatible with Next.js 15 yes, apparently, so we need to wait for them to update and be compatible.
  • Getting some warnings from Next.js on their route resolution

@amannn
Copy link
Contributor

amannn commented Nov 4, 2024

@ovflowd I had another look and found & fixed the issue here: #7178

I've also added a separate PR just for the next-intl update with minimal changes, in case you want to merge this part to main already: #7177

Hope this helps! 🙂

@@ -15,7 +15,7 @@
"engines": {
"node": "v20"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, will Node.js >20 also be supported? IIRC the only issue was the with / assert?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vercel just allow v20 for moment so I think we should stay on 20 for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants