Skip to content

Conversation

@robearlam
Copy link
Member

@robearlam robearlam commented Dec 23, 2025

Upgrading the Sugon Sites to the latest version of the Content SDK

Types of changes

  • Upgraded from JSS to latest Content SDK

  • Tidied code along the way

  • Removed Storybook in favour of Design Library

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read the Contributing guide.
  • My code/comments/docs fully adhere to the Code of Conduct.
  • My change is a code change.
  • My change is a documentation change and there are NO other updates required.

Closes #559

@robearlam robearlam marked this pull request as ready for review January 11, 2026 23:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request upgrades the Sugcon2024 site from Sitecore JSS to the latest Content SDK, representing a major architectural shift with breaking changes. The upgrade modernizes the codebase by migrating from Next.js Pages Router to App Router, consolidating configuration, and removing Storybook in favor of the Design Library.

Changes:

  • Upgraded from @sitecore-jss/sitecore-jss-nextjs to @sitecore-content-sdk/nextjs with all related imports and APIs updated
  • Migrated from Next.js Pages Router to App Router with new file structure (src/app/ directory)
  • Consolidated and simplified middleware using new Content SDK middleware components
  • Removed Storybook, legacy API routes, plugin systems, and build scripts in favor of SDK-native tooling

Reviewed changes

Copilot reviewed 252 out of 266 changed files in this pull request and generated no comments.

Show a summary per file
File Description
xmcloud.build.json Updated Node.js version to 22.11.0 and added build/run commands
package.json Major dependency upgrade from JSS 21.5.3 to Content SDK 1.0.0
tsconfig.json Updated TypeScript config for Next.js 15 compatibility
next.config.ts New TypeScript-based config replacing JavaScript version
sitecore.config.ts New configuration file using Content SDK's defineConfig
src/middleware.ts Complete refactor using Content SDK middleware components
src/app/ New App Router structure with layout, page, and API route handlers
src/components/ All components updated to use Content SDK imports and APIs
src/lib/ Removed custom services/plugins, replaced with SDK client
SASS files Updated import paths from @sass alias to relative paths
Scripts Removed legacy scaffold/bootstrap scripts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Chris1415
Copy link
Collaborator

[Cursor Review Comments]

Code review: Next.js App Router best practices

Critical issues

  1. Using next/head in App Router (must fix)
  • Location: src/Layout.tsx line 6

  • Issue: next/head is Pages Router only and doesn't work in App Router

  • Impact: Metadata won't render correctly

  • Fix: Use the Metadata API (generateMetadata or metadata export) instead

  1. Metadata implementation incomplete
  • Location: src/app/[site]/[locale]/[[...path]]/page.tsx

  • Issue: generateMetadata only returns title; missing description, Open Graph, etc.

  • Impact: SEO and social sharing metadata incomplete

  • Fix: Expand generateMetadata to include all metadata fields

  1. Using component in App Router
  • Location: src/Layout.tsx lines 37-43

  • Issue: from next/head doesn't work in App Router

  • Impact: Head tags won't render

  • Fix: Move metadata to generateMetadata or use the root layout's metadata

Good practices

  1. App Router structure
  • Correct nested route structure: [site]/[locale]/[[...path]]

  • Proper use of catch-all routes

  • Correct error boundaries (not-found.tsx, global-error.tsx)

  1. Async/await patterns
  • Correctly awaiting params and searchParams (Next.js 15)

  • Proper async server components

  1. Route handlers
  • API routes use proper exports (GET, POST, OPTIONS)

  • Correct use of dynamic = 'force-dynamic' for dynamic routes

  1. Middleware
  • Well-structured middleware with proper matcher configuration

  • Good separation of concerns (locale, multisite, redirects, personalize)

  1. Static generation
  • Proper generateStaticParams implementation

  • Conditional static generation based on environment

  1. Client/server boundaries
  • Appropriate use of 'use client' directives

  • Good separation between server and client components

Recommendations

  1. High priority
  • Remove next/head and migrate to the Metadata API

  • Complete generateMetadata with all SEO fields

  • Move meta tags from Layout.tsx to generateMetadata

  1. Medium priority
  • Add loading.tsx files for loading states

  • Consider adding route segment config exports (export const dynamic, export const revalidate) where appropriate

  • Add error boundaries at route segment levels if needed

  1. Low priority
  • Consider extracting metadata logic into a utility function

  • Add TypeScript types for metadata return values

  • Consider using metadata object export for static metadata

Specific code issues

  1. src/Layout.tsx:

    // ❌ WRONG - next/head doesn't work in App Router

    import Head from "next/head";

    <title>...</title>
  2. src/app/[site]/[locale]/[[...path]]/page.tsx:

    // ⚠️ INCOMPLETE - only returns title

    export const generateMetadata = async ({ params }: PageProps) => {

    // Should include description, og:image, etc.

    }

  3. src/components/templates/header/HeaderMeta.tsx:

  • Uses client-side hooks (usePathname) but should be handled server-side via metadata API

Verdict

The structure follows App Router patterns, but there are critical issues with metadata handling that must be fixed before merging. The use of next/head will cause runtime issues in App Router.

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.

Upgrade SUGCON Sites to Latest version of Content-SDK

3 participants