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

feat(core): Next 15 upgrade #1508

Merged
merged 2 commits into from
Oct 23, 2024
Merged

feat(core): Next 15 upgrade #1508

merged 2 commits into from
Oct 23, 2024

Conversation

deini
Copy link
Member

@deini deini commented Oct 23, 2024

What/Why?

Upgrade to Next 15, started with codemods and did many manual changes.

Testing

CI 👀

Copy link

changeset-bot bot commented Oct 23, 2024

🦋 Changeset detected

Latest commit: 2f0a414

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@bigcommerce/catalyst-core Minor
@bigcommerce/eslint-config-catalyst Patch
@bigcommerce/catalyst-client Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Oct 23, 2024

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

Name Status Preview Comments Updated (UTC)
catalyst-latest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 10:20pm
6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
catalyst ⬜️ Ignored (Inspect) Oct 23, 2024 10:20pm
catalyst-1millionproducts-store ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 10:20pm
catalyst-au ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 10:20pm
catalyst-test-store ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 10:20pm
catalyst-uk ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 10:20pm
catalyst-unstable ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 10:20pm

params: Promise<{ slug: string; locale: LocaleType }>;
}

export async function generateMetadata(props: Props): Promise<Metadata> {
Copy link
Member Author

@deini deini Oct 23, 2024

Choose a reason for hiding this comment

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

This one took a while to figure out. Turns out our generateMetadata from the dynamic page (which we used to re-export) is using searchParams, which causes this static page to be considered dynamic even if we use force-static.

Hard to track as this doesn't fail on dev, only in production builds.

Had to duplicate this fn and remove the usage of searchParams, since it's a static page we never hit it if we have searchParams set anyway.

I would like to know how this used to work in Next 14 and if there are plans to add searchParams to the list of force-static and return empty on static builds.

The odd thing is that the page does use searchParams 🤔 but only generateMetadata fails.

@@ -6,6 +6,7 @@ const config = {
rules: {
'react/react-in-jsx-scope': 'off',
'react/prefer-read-only-props': 'off',
'react/destructuring-assignment': 'off',
Copy link
Member Author

Choose a reason for hiding this comment

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

With the new changes to Next 15 you need to await things like searchParams and params. Destructuring props makes this a bit annoying in some scenarios.

@deini deini marked this pull request as ready for review October 23, 2024 16:18
@deini deini requested a review from a team as a code owner October 23, 2024 16:18
Comment on lines +12 to +14
if (typeof window !== 'undefined') {
bodl.initialize();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@@ -36,10 +36,15 @@ export async function updateItemQuantity({
variantEntityId,
selectedOptions,
}: UpdateProductQuantityParams) {
console.log('updateItemQuantity:start');
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove logs?

@deini deini added this pull request to the merge queue Oct 23, 2024
Merged via the queue into main with commit 3ebb617 Oct 23, 2024
13 checks passed
@deini deini deleted the next-15 branch October 23, 2024 22:29
@github-actions github-actions bot mentioned this pull request Oct 23, 2024
Copy link
Contributor

⚡️🏠 Lighthouse report

Lighthouse ran against https://catalyst-latest-mxwtzks7p-bigcommerce-platform.vercel.app

🖥️ Desktop

We ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:

Category Score
🟠 Performance 82
🟢 Accessibility 100
🟢 Best practices 100
🟠 SEO 82

📱 Mobile

We ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:

Category Score
🟢 Performance 91
🟢 Accessibility 100
🟢 Best practices 100
🟠 SEO 85

deini added a commit that referenced this pull request Oct 30, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 30, 2024
* Revert "chore(core): downgrade next (#1513)"

This reverts commit ea01e3b.

* Revert "feat(core): Next 15 upgrade (#1508)"

This reverts commit 3ebb617.

* chore(core): bump next-intl

* chore(core): only initialize bodl on browsers

* fix(tests): remove 'Other' section test

* fix(func): Update badge snapshot

---------

Co-authored-by: Anudeep Vattipalli <anudeep.vattipalli@bigcommerce.com>
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.

3 participants