-
-
Notifications
You must be signed in to change notification settings - Fork 11
390-feat: Add open graph previews #894
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
base: main
Are you sure you want to change the base?
Conversation
β¦ommitting temporary assets
π WalkthroughWalkthroughThis change implements comprehensive Open Graph image generation and enhanced SEO metadata across multiple pages. It introduces dynamic OG image routes, centralizes metadata creation via a helper, enriches metadata with descriptions, keywords, and canonical URLs, and adds utility functions for image and font processing. Type definitions and store logic are updated to support new SEO fields. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RouteHandler
participant Helper
participant Utils
User->>RouteHandler: Request OG image (e.g., /courses/[slug]/og.png)
RouteHandler->>Utils: Load course/page data, images, fonts
RouteHandler->>Helper: Call createCourseTree/createPageTree with data
Helper->>Utils: Fetch and process images/fonts as needed
Helper-->>RouteHandler: Return ImageResponse
RouteHandler-->>User: Respond with OG image
sequenceDiagram
participant Page
participant generateMetadata
participant generatePageMetadata
Page->>generateMetadata: On page load
generateMetadata->>generatePageMetadata: Pass title, description, keywords, etc.
generatePageMetadata-->>generateMetadata: Return metadata object
generateMetadata-->>Page: Return enriched metadata
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. π§ ESLint
npm error Exit handler never called! π Recent review detailsConfiguration used: .coderabbit.yaml π Files selected for processing (2)
π§ Files skipped from review as they are similar to previous changes (2)
β° Context from checks skipped due to timeout of 90000ms (2)
β¨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 5
π§Ή Nitpick comments (11)
scripts/utils/fetch-courses-list.ts (1)
3-10
: Good implementation with proper error handlingThe function correctly handles API errors and type casting. Consider adding a timeout to the fetch request for better error handling in case of slow network conditions.
export async function fetchCoursesList(url: string): Promise<ApiCourse[]> { - const res = await fetch(url); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + + try { + const res = await fetch(url, { + signal: controller.signal + }); + + if (!res.ok) { + throw new Error(`API error ${res.status}`); + } + return (await res.json()) as ApiCourse[]; + } finally { + clearTimeout(timeoutId); + } - if (!res.ok) { - throw new Error(`API error ${res.status}`); - } - return (await res.json()) as ApiCourse[]; }src/app/mentorship/[course]/page.tsx (1)
9-9
: Consider course-specific metadata for each course pageCurrently, this course-specific page uses the same generic mentorship description and image as the main mentorship page. For better SEO and social media sharing, consider generating unique descriptions and OG images for each course.
Also applies to: 11-16
src/app/courses/reactjs/page.tsx (1)
11-12
: Consider updating the React course description.While the title is correctly fetched, the description doesn't seem specific to the React course - it appears to be a generic RS School description rather than React-specific content.
const title = await getCourseTitle(courseName); - const description = 'Everyone can study at RS School, regardless of age, professional employment, or place of residence'; + const description = 'Learn modern React, hooks, state management, and build production-ready applications with our comprehensive React course';src/app/courses/javascript/page.tsx (1)
3-3
: Fix extra quotation mark in descriptionThe description contains an erroneous closing quotation mark at the end of the string.
- const description = 'Everyone can study at RS School, regardless of age, professional employment, or place of residence".'; + const description = 'Everyone can study at RS School, regardless of age, professional employment, or place of residence.';Also applies to: 11-20
scripts/utils/course-info.ts (2)
5-19
: The getCourseInfo function looks good but could be simplifiedThe function correctly extracts and formats course dates from course information, but the conditional logic in lines 8-14 could be simplified.
export const getCourseInfo = (coursesList: ApiCourse[], slug: string): string => { const courseInfo: ApiCourse | undefined = coursesList.find((c) => c.descriptionUrl.toLowerCase().endsWith(slug)); - let rawDate: string; - - if (!courseInfo) { - rawDate = ''; - } else { - rawDate = courseInfo.startDate; - } - - const formattedDate: string = rawDate ? dayjs(rawDate).format('MMM DD, YYYY') : 'TBD'; + const rawDate = courseInfo?.startDate || ''; + const formattedDate: string = rawDate ? dayjs(rawDate).format('MMM DD, YYYY') : 'TBD'; return formattedDate; };
5-6
: Consider adding null check for coursesList parameterThe function assumes coursesList is always a valid array, but doesn't validate this input.
-export const getCourseInfo = (coursesList: ApiCourse[], slug: string): string => { +export const getCourseInfo = (coursesList: ApiCourse[] | null | undefined, slug: string): string => { + if (!coursesList || !Array.isArray(coursesList)) { + return 'TBD'; + } const courseInfo: ApiCourse | undefined = coursesList.find((c) => c.descriptionUrl.toLowerCase().endsWith(slug));src/app/courses/javascript-ru/page.tsx (1)
12-12
: Consider moving description to constantsThe description is hardcoded in this file but may be reused across course pages.
If this description is common across courses, consider moving it to a constants file.
src/app/docs/[lang]/[...slug]/page.tsx (1)
48-48
: Consider moving description to constantsSimilar to course pages, this hardcoded description could be moved to a constants file.
If this description appears in multiple docs-related pages, consider extracting it to a constants file.
dev-data/open-graph.data.ts (1)
21-27
: Naming convention inconsistency.Consider using consistent naming conventions across the constants.
-export const Descriptions = { +export const DESCRIPTIONS = {This would match the uppercase naming pattern used for
COURSE_SLUGS
andRS_PAGES
.scripts/utils/generate-courses-tree.ts (1)
1-122
: Extract common style values as constantsThe component has multiple hardcoded style values (colors, sizes, fonts) that are repeated throughout. Consider extracting these as constants for better maintainability.
import { JSX, createElement } from 'react'; +// Design constants +const COLORS = { + dark: '#000', + light: '#f0f2f5', + accent: '#ffda1f' +}; + +const FONTS = { + family: 'Inter', + sizes: { + large: 72, + medium: 50, + small: 38, + body: 36 + } +}; + +const IMAGE_DIMENSIONS = { + width: 1200, + height: 630, + logo: 250 +}; export function createCourseTree( /* ... parameters remain the same ... */ ) { return createElement( 'div', { style: { display: 'flex', width: IMAGE_DIMENSIONS.width, height: IMAGE_DIMENSIONS.height, fontFamily: FONTS.family, fontWeight: 400, }, }, /* ... remainder of the function using these constants ... */ ); }scripts/utils/generate-page-tree.ts (1)
30-40
: Alt text doesnβt match the image content.The map background is declared with
alt: 'RS Logo'
, which is misleading and hurts accessibility. Use a description that actually represents the image purpose (e.g."World map background"
or empty alt if itβs purely decorative).- alt: 'RS Logo', + alt: 'World map background',
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
β Files ignored due to path filters (18)
package-lock.json
is excluded by!**/package-lock.json
public/fonts/Inter_28pt-Bold.ttf
is excluded by!**/*.ttf
public/fonts/Inter_28pt-Regular.ttf
is excluded by!**/*.ttf
src/shared/assets/og-logos/angular.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/aws-cloud-developer.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/aws-devops.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/aws-fundamentals.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/javascript-preschool-ru.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/javascript-ru.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/javascript.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/map.png
is excluded by!**/*.png
src/shared/assets/og-logos/mentor-with-his-students.png
is excluded by!**/*.png
src/shared/assets/og-logos/nodejs.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/reactjs.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/rs-banner.png
is excluded by!**/*.png
src/shared/assets/og-logos/rs-banner.svg
is excluded by!**/*.svg
src/shared/assets/og-logos/rs-school.png
is excluded by!**/*.png
src/shared/assets/og-logos/rss-logo.svg
is excluded by!**/*.svg
π Files selected for processing (32)
.gitignore
(1 hunks)dev-data/open-graph.data.ts
(1 hunks)package.json
(2 hunks)scripts/generate-og-script.ts
(1 hunks)scripts/types.ts
(1 hunks)scripts/utils/course-info.ts
(1 hunks)scripts/utils/ensure-dir-exists.ts
(1 hunks)scripts/utils/fetch-courses-list.ts
(1 hunks)scripts/utils/generate-courses-tree.ts
(1 hunks)scripts/utils/generate-image.tsx
(1 hunks)scripts/utils/generate-page-tree.ts
(1 hunks)scripts/utils/load-fonts.ts
(1 hunks)scripts/utils/load-image-as-data-uri.ts
(1 hunks)src/app/community/page.tsx
(1 hunks)src/app/courses/angular/page.tsx
(1 hunks)src/app/courses/aws-cloud-developer/page.tsx
(1 hunks)src/app/courses/aws-devops/page.tsx
(1 hunks)src/app/courses/aws-fundamentals/page.tsx
(1 hunks)src/app/courses/javascript-preschool-ru/page.tsx
(1 hunks)src/app/courses/javascript-ru/page.tsx
(1 hunks)src/app/courses/javascript/page.tsx
(1 hunks)src/app/courses/nodejs/page.tsx
(1 hunks)src/app/courses/page.tsx
(1 hunks)src/app/courses/reactjs/page.tsx
(1 hunks)src/app/docs/[lang]/[...slug]/page.tsx
(2 hunks)src/app/docs/[lang]/page.tsx
(1 hunks)src/app/layout.tsx
(1 hunks)src/app/mentorship/[course]/page.tsx
(1 hunks)src/app/mentorship/page.tsx
(1 hunks)src/app/page.tsx
(1 hunks)src/shared/helpers/generate-page-metadata.ts
(1 hunks)tsconfig.json
(1 hunks)
π§° Additional context used
𧬠Code Graph Analysis (7)
src/app/courses/aws-cloud-developer/page.tsx (4)
dev-data/index.ts (1)
COURSE_TITLES
(36-36)src/app/courses/reactjs/page.tsx (1)
generateMetadata
(10-21)src/shared/helpers/get-course-title.ts (1)
getCourseTitle
(4-10)src/shared/helpers/generate-page-metadata.ts (1)
generatePageMetadata
(1-26)
scripts/utils/fetch-courses-list.ts (1)
scripts/types.ts (1)
ApiCourse
(1-4)
scripts/utils/course-info.ts (1)
scripts/types.ts (1)
ApiCourse
(1-4)
src/app/courses/page.tsx (7)
src/app/community/page.tsx (1)
generateMetadata
(6-17)src/app/courses/reactjs/page.tsx (1)
generateMetadata
(10-21)src/app/page.tsx (1)
generateMetadata
(6-17)src/app/mentorship/page.tsx (1)
generateMetadata
(7-18)src/app/docs/[lang]/page.tsx (1)
generateMetadata
(9-20)src/app/layout.tsx (1)
metadata
(11-36)src/shared/helpers/generate-page-metadata.ts (1)
generatePageMetadata
(1-26)
scripts/generate-og-script.ts (10)
scripts/utils/load-fonts.ts (2)
Font
(4-9)loadFont
(11-23)dev-data/open-graph.data.ts (3)
COURSE_SLUGS
(1-11)RS_PAGES
(13-19)Descriptions
(21-27)scripts/types.ts (1)
ApiCourse
(1-4)scripts/utils/fetch-courses-list.ts (1)
fetchCoursesList
(3-10)scripts/utils/ensure-dir-exists.ts (1)
ensureDirExists
(3-14)scripts/utils/load-image-as-data-uri.ts (1)
loadImageAsDataUri
(4-8)scripts/utils/course-info.ts (1)
getCourseInfo
(5-19)scripts/utils/generate-courses-tree.ts (1)
createCourseTree
(3-122)scripts/utils/generate-image.tsx (1)
generateImage
(6-32)scripts/utils/generate-page-tree.ts (1)
createPageTree
(3-130)
scripts/utils/generate-image.tsx (1)
scripts/utils/load-fonts.ts (1)
Font
(4-9)
src/app/docs/[lang]/[...slug]/page.tsx (3)
src/app/layout.tsx (1)
metadata
(11-36)src/shared/helpers/generate-page-metadata.ts (1)
generatePageMetadata
(1-26)src/app/docs/constants.ts (1)
TITLE_POSTFIX
(2-2)
πͺ GitHub Actions: Pull Request
scripts/generate-og-script.ts
[error] 28-28: TypeError: Failed to parse URL from undefined. The error originates from an invalid URL input in fetchCoursesList function called at line 4 in fetch-courses-list.ts. This caused the build to fail with exit code 1.
π Additional comments (40)
tsconfig.json (1)
56-58
: Correctly updated the TypeScript configuration to include new scriptThe inclusion of "scripts/generate-og-script.ts" in the TypeScript configuration ensures proper type checking and compilation for the new OG image generation script.
scripts/types.ts (1)
1-4
: Type definition looks goodThe ApiCourse type correctly defines the expected structure with the necessary properties for course data handling.
.gitignore (1)
35-36
: Good practice: ignoring generated OG image directoriesThe addition of generated OG image directories to .gitignore prevents unwanted version control of dynamically generated assets.
src/app/layout.tsx (1)
12-12
: Remember to change this URL during local developmentThe metadataBase URL is set to the production URL. As mentioned in the PR description, developers will need to modify this to 'http://localhost:3000' during local development to ensure proper OG image rendering.
Ensure this is documented in the project README or development guide to help other developers.
src/app/courses/page.tsx (3)
3-3
: Proper import added for metadata helper functionThe import of
generatePageMetadata
enables consistent metadata generation across the site.
8-8
: Great descriptive text for SEOThe description provides a concise summary that effectively captures the value proposition of the courses.
10-15
: Good implementation of centralized metadata generationUsing the helper function ensures consistency across all pages and properly includes Open Graph image data for social sharing.
src/app/mentorship/page.tsx (3)
3-3
: Proper import added for metadata helper functionThe import aligns with the overall approach to centralize metadata generation.
9-9
: Concise and engaging mentorship descriptionThe description effectively communicates the reciprocal benefit of mentorship.
11-16
: Consistent implementation of metadata generationThe helper function is properly used with all required parameters, maintaining consistent structure.
src/app/mentorship/[course]/page.tsx (1)
3-3
: Proper import added for metadata helper functionThe import matches the pattern used across other page components.
src/app/page.tsx (3)
3-3
: Proper import added for metadata helper functionThe import enables consistent metadata generation for the home page.
8-8
: Effective and concise home page descriptionThe description captures the community spirit of the school in a memorable way.
10-16
: Proper implementation of metadata generationThe helper function is correctly used with all required parameters to generate complete metadata including Open Graph image data.
src/app/community/page.tsx (3)
3-3
: New import added correctly.The import for
generatePageMetadata
helper is properly added.
8-8
: Good description for community page.Clear, concise description that accurately represents the community page content.
10-16
: Metadata implementation looks good.The implementation correctly uses the new helper function with appropriate parameters:
- Title from existing code
- New description
- Proper OG image path that follows the convention
This ensures consistent metadata structure across the site.
src/app/courses/nodejs/page.tsx (3)
3-3
: New import added correctly.The import for
generatePageMetadata
helper is properly added.
11-12
: Good metadata implementation.The title is correctly fetched asynchronously, and the Node.js course has a detailed, specific description.
14-20
: Metadata generation looks good.The implementation correctly uses the helper function with appropriate parameters:
- Dynamic title
- Specific course description
- Proper OG image path following the naming convention
This ensures consistent metadata across course pages.
src/app/courses/reactjs/page.tsx (2)
3-3
: New import added correctly.The import for
generatePageMetadata
helper is properly added.
14-20
: Metadata implementation is correct.The helper function is used correctly with the appropriate parameters, ensuring consistent metadata structure.
src/app/docs/[lang]/page.tsx (2)
4-4
: New import added correctly.The import for
generatePageMetadata
helper is properly added.
10-19
: Metadata implementation looks good.The implementation correctly:
- Uses the existing title with postfix
- Adds a clear and concise description
- References the appropriate OG image path
- Uses the helper function consistently
This ensures standardized metadata structure across the site.
src/app/courses/javascript-preschool-ru/page.tsx (1)
3-3
: Properly implemented Open Graph metadataThe metadata generation is correctly implemented using the new helper function. The course-specific description and image path provide good context for social sharing.
Also applies to: 11-20
src/app/courses/angular/page.tsx (1)
3-3
: Well-implemented OG metadataThe metadata implementation follows the project's standard pattern and provides a clear description that accurately represents the Angular course.
Also applies to: 11-20
src/app/courses/aws-fundamentals/page.tsx (1)
3-3
: Correctly implemented metadataThe AWS Fundamentals course metadata is properly implemented with a relevant description and appropriate image path.
Also applies to: 11-20
src/app/courses/javascript-ru/page.tsx (1)
10-21
: Metadata implementation looks goodThe implementation correctly uses the central
generatePageMetadata
helper to create consistent metadata across the site.src/app/docs/[lang]/[...slug]/page.tsx (1)
48-56
: Well-implemented metadata enhancementThe metadata generation follows the standardized pattern using the shared helper function.
scripts/utils/ensure-dir-exists.ts (1)
3-14
: Well-implemented directory existence checkThis utility function correctly handles directory creation with proper error handling.
The implementation:
- Properly checks if the directory exists
- Creates it recursively if needed
- Handles errors appropriately, distinguishing between missing directories and other errors
src/app/courses/aws-cloud-developer/page.tsx (2)
3-3
: New import for metadata generation utility.Good addition of the
generatePageMetadata
helper to standardize metadata generation.
11-20
: Metadata implementation looks good.The enhanced metadata implementation properly includes title, description, and OG image path, following the project's new metadata standard.
package.json (2)
7-9
: Script integration for OG image generation.Good implementation of pre-build OG image generation script that runs before both development and build processes.
95-95
: Added tsx dependency.Appropriate addition of the tsx package as a dev dependency to support the new OG image generation script.
dev-data/open-graph.data.ts (2)
1-11
: Course slugs mapping looks good.The course slugs mapping provides a clean way to reference course URLs.
13-20
: Page mapping implementation.Clear mapping of page identifiers to their display names.
src/app/courses/aws-devops/page.tsx (2)
3-3
: New import for metadata generation utility.Good addition of the
generatePageMetadata
helper to standardize metadata generation.
11-20
: Metadata implementation looks good.The enhanced metadata implementation properly includes title, description, and OG image path, following the project's new metadata standard.
src/shared/helpers/generate-page-metadata.ts (1)
1-26
: Well-structured metadata generatorThis function provides a clean, standardized way to generate page metadata including Open Graph properties. The structure matches Next.js metadata requirements and includes all essential fields for proper social media sharing.
scripts/utils/generate-image.tsx (1)
1-32
: Robust image generation with proper error handlingThe function correctly handles JSX-to-image conversion with appropriate input validation and error catching. The use of
Buffer
for the output makes it versatile for file system operations.
style: { | ||
display: 'flex', | ||
width: 1200, | ||
height: 630, | ||
fontFamily: 'Inter', | ||
weight: 400, | ||
}, | ||
}, |
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.
Fix invalid CSS property in root element styles
The root div uses an invalid CSS property weight
which should be fontWeight
.
style: {
display: 'flex',
width: 1200,
height: 630,
fontFamily: 'Inter',
- weight: 400,
+ fontWeight: 400,
},
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
style: { | |
display: 'flex', | |
width: 1200, | |
height: 630, | |
fontFamily: 'Inter', | |
weight: 400, | |
}, | |
}, | |
style: { | |
display: 'flex', | |
width: 1200, | |
height: 630, | |
fontFamily: 'Inter', | |
fontWeight: 400, | |
}, | |
}, |
Lighthouse Report:
|
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.
Actionable comments posted: 0
π§Ή Nitpick comments (3)
scripts/utils/get-courses-schedule.ts (3)
4-7
: Consider adding JSDoc documentation.This utility function would benefit from JSDoc documentation explaining its purpose, return type, and potential errors.
+/** + * Fetches course schedule data from the API + * @returns Array of course objects with startDate and descriptionUrl properties + * @throws Logs error and returns empty array if fetch fails + */ export async function getCoursesSchedule() { try { const response = await api.course.queryCoursesSchedule();
7-8
: Type assertion should be validated.The type assertion (
as CoursesScheduleResponse
) doesn't validate the actual shape of the data. Consider using runtime validation or a more type-safe approach.- const courses = response.result as CoursesScheduleResponse; + // Option 1: Basic validation + const result = response.result; + if (!Array.isArray(result)) { + throw new Error('Invalid API response format'); + } + const courses = result as CoursesScheduleResponse; + + // Option 2: With a validation library like zod, ajv, etc. + // import { courseScheduleSchema } from '../../path/to/validation'; + // const courses = courseScheduleSchema.parse(response.result);
4-17
: Consider dependency injection for better testability.The current implementation directly imports the API client, making it difficult to test in isolation. Consider using dependency injection for better testability.
-export async function getCoursesSchedule() { +export async function getCoursesSchedule(apiClient = api) { try { - const response = await api.course.queryCoursesSchedule(); + const response = await apiClient.course.queryCoursesSchedule(); const courses = response.result as CoursesScheduleResponse;
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
π Files selected for processing (3)
scripts/generate-og-script.ts
(1 hunks)scripts/utils/course-info.ts
(1 hunks)scripts/utils/get-courses-schedule.ts
(1 hunks)
π§ Files skipped from review as they are similar to previous changes (2)
- scripts/utils/course-info.ts
- scripts/generate-og-script.ts
π§° Additional context used
𧬠Code Graph Analysis (1)
scripts/utils/get-courses-schedule.ts (2)
src/shared/api/api.ts (1)
api
(3-3)src/entities/course/types.ts (1)
CoursesScheduleResponse
(13-32)
π Additional comments (3)
scripts/utils/get-courses-schedule.ts (3)
1-3
: Appropriate imports for the utility.The imports correctly reference the required type and API client needed for this utility function.
9-12
: Appropriate data extraction logic.The mapping logic correctly extracts the needed fields with fallbacks for missing values.
13-16
: Error handling looks good.The function appropriately catches errors, logs them, and returns a sensible fallback value.
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.
Actionable comments posted: 3
β»οΈ Duplicate comments (1)
scripts/generate-og-script.ts (1)
133-141
: π οΈ Refactor suggestionUse Promise.all for better error handling
Running tasks in parallel with separate error handlers can terminate the process before all tasks complete.
- generateOgImagePages().catch((err) => { - console.error(err); - process.exit(1); - }); - - generateOGCourses().catch((err) => { - console.error(err); - process.exit(1); - }); + Promise.all([generateOgImagePages(), generateOGCourses()]) + .then(() => console.log('OG image generation finished β ')) + .catch((err) => { + console.error('OG image generation failed β', err); + process.exit(1); + });
π§Ή Nitpick comments (3)
scripts/utils/courses-tree/generate-courses-tree.tsx (2)
5-12
: Parameter naming could be clearerThe parameters
rsLogoDataUriPromise
andlogoCourseUriPromise
suggest they're Promises, but they're actually string values (already resolved data URIs).export function createCourseTree( title: string, leftTitle: string, leftSubtitle: string, formattedDate: string, - rsLogoDataUriPromise: string, - logoCourseUriPromise: string, + rsLogoDataUri: string, + logoCourseDataUri: string, ): React.JSX.Element {Corresponding changes should be made in the function body:
<img - src={rsLogoDataUriPromise} + src={rsLogoDataUri} style={stylesCourseTree.logo} alt="RS School Logo" /> <img - src={logoCourseUriPromise} + src={logoCourseDataUri} style={stylesCourseTree.courseLogo} alt={`${title} logo`} />
16-20
: Improve accessibility with more descriptive alt textThe current alt text is generic. Consider adding more context for better accessibility.
<img src={rsLogoDataUriPromise} style={stylesCourseTree.logo} - alt="RS School Logo" + alt="Rolling Scopes School Logo" />scripts/generate-og-script.ts (1)
24-80
: Inconsistent function namingFunction naming has inconsistent capitalization:
generateOGCourses
vsgenerateOgImagePages
.- async function generateOGCourses(): Promise<void> { + async function generateOgCourses(): Promise<void> {Update references too:
- Promise.all([generateOgImagePages(), generateOGCourses()]) + Promise.all([generateOgImagePages(), generateOgCourses()])π§° Tools
πͺ GitHub Actions: Preview
[error] 57-57: Error: ENOENT: no such file or directory, open '/home/runner/work/site/site/src/shared/assets/og-logos/undefined.svg'. The file 'undefined.svg' is missing, causing the build script to fail.
πͺ GitHub Actions: Pull Request
[error] 57-57: Error: ENOENT: no such file or directory, open '/home/runner/work/site/site/src/shared/assets/og-logos/undefined.svg'. The file 'undefined.svg' is missing, causing the build script to fail.
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
π Files selected for processing (3)
scripts/generate-og-script.ts
(1 hunks)scripts/utils/courses-tree/generate-courses-tree.styles.ts
(1 hunks)scripts/utils/courses-tree/generate-courses-tree.tsx
(1 hunks)
β Files skipped from review due to trivial changes (1)
- scripts/utils/courses-tree/generate-courses-tree.styles.ts
π§° Additional context used
πͺ GitHub Actions: Preview
scripts/generate-og-script.ts
[error] 57-57: Error: ENOENT: no such file or directory, open '/home/runner/work/site/site/src/shared/assets/og-logos/undefined.svg'. The file 'undefined.svg' is missing, causing the build script to fail.
πͺ GitHub Actions: Pull Request
scripts/generate-og-script.ts
[error] 57-57: Error: ENOENT: no such file or directory, open '/home/runner/work/site/site/src/shared/assets/og-logos/undefined.svg'. The file 'undefined.svg' is missing, causing the build script to fail.
π Additional comments (2)
scripts/utils/courses-tree/generate-courses-tree.tsx (1)
1-35
: Component looks good overallThe JSX structure is clean and well-organized. The component correctly uses the imported styles and creates a visually balanced layout for the Open Graph image.
scripts/generate-og-script.ts (1)
1-23
: Script setup looks well-organizedThe imports, font loading, and type definitions are well-structured and follow good practices.
scripts/generate-og-script.ts
Outdated
for (const key of Object.keys(COURSE_TITLES) as CourseKey[]) { | ||
const slug: string = COURSE_SLUGS[key]; | ||
const title: string = COURSE_TITLES[key]; | ||
|
||
const formattedDate: string = getCourseInfo(coursesSchedule, slug); | ||
|
||
const logoDataUri: string = await loadImageAsDataUri( | ||
`src/shared/assets/og-logos/${slug}.svg`, | ||
'image/svg+xml', | ||
); | ||
|
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.
Handle undefined slug to prevent build failure
Pipeline failures show that the script is trying to access undefined.svg
. You need to validate course slugs before using them.
for (const key of Object.keys(COURSE_TITLES) as CourseKey[]) {
const slug: string = COURSE_SLUGS[key];
+ if (!slug) {
+ console.warn(`Skipping OG image generation for ${key} - slug is undefined`);
+ continue;
+ }
const title: string = COURSE_TITLES[key];
const formattedDate: string = getCourseInfo(coursesSchedule, slug);
const logoDataUri: string = await loadImageAsDataUri(
`src/shared/assets/og-logos/${slug}.svg`,
'image/svg+xml',
);
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
for (const key of Object.keys(COURSE_TITLES) as CourseKey[]) { | |
const slug: string = COURSE_SLUGS[key]; | |
const title: string = COURSE_TITLES[key]; | |
const formattedDate: string = getCourseInfo(coursesSchedule, slug); | |
const logoDataUri: string = await loadImageAsDataUri( | |
`src/shared/assets/og-logos/${slug}.svg`, | |
'image/svg+xml', | |
); | |
for (const key of Object.keys(COURSE_TITLES) as CourseKey[]) { | |
const slug: string = COURSE_SLUGS[key]; | |
if (!slug) { | |
console.warn(`Skipping OG image generation for ${key} - slug is undefined`); | |
continue; | |
} | |
const title: string = COURSE_TITLES[key]; | |
const formattedDate: string = getCourseInfo(coursesSchedule, slug); | |
const logoDataUri: string = await loadImageAsDataUri( | |
`src/shared/assets/og-logos/${slug}.svg`, | |
'image/svg+xml', | |
); | |
// β¦rest of your loop bodyβ¦ | |
} |
π§° Tools
πͺ GitHub Actions: Preview
[error] 57-57: Error: ENOENT: no such file or directory, open '/home/runner/work/site/site/src/shared/assets/og-logos/undefined.svg'. The file 'undefined.svg' is missing, causing the build script to fail.
πͺ GitHub Actions: Pull Request
[error] 57-57: Error: ENOENT: no such file or directory, open '/home/runner/work/site/site/src/shared/assets/og-logos/undefined.svg'. The file 'undefined.svg' is missing, causing the build script to fail.
π€ Prompt for AI Agents
In scripts/generate-og-script.ts around lines 51 to 61, the code uses
COURSE_SLUGS[key] without checking if the slug is defined, causing build
failures when slug is undefined. Add a validation step after retrieving the slug
to check if it is defined; if not, skip processing that key or handle the error
gracefully to prevent accessing undefined.svg files.
scripts/generate-og-script.ts
Outdated
const buffer: Buffer<ArrayBufferLike> | null = await generateImage(tree, fonts); | ||
|
||
if (!buffer) { | ||
throw new Error(`Doesn't generate image for "${slug}"`); | ||
} | ||
|
||
await fs.writeFile(path.join(ogDir, `${slug}.png`), buffer); | ||
console.log(`${slug} created`); |
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.
π οΈ Refactor suggestion
Add try-catch for image file operations
Missing error handling for image file operations could lead to unhandled exceptions.
const buffer: Buffer<ArrayBufferLike> | null = await generateImage(tree, fonts);
if (!buffer) {
throw new Error(`Doesn't generate image for "${slug}"`);
}
- await fs.writeFile(path.join(ogDir, `${slug}.png`), buffer);
- console.log(`${slug} created`);
+ try {
+ await fs.writeFile(path.join(ogDir, `${slug}.png`), buffer);
+ console.log(`${slug} created`);
+ } catch (error) {
+ console.error(`Error writing image file for ${slug}:`, error);
+ throw error;
+ }
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const buffer: Buffer<ArrayBufferLike> | null = await generateImage(tree, fonts); | |
if (!buffer) { | |
throw new Error(`Doesn't generate image for "${slug}"`); | |
} | |
await fs.writeFile(path.join(ogDir, `${slug}.png`), buffer); | |
console.log(`${slug} created`); | |
const buffer: Buffer<ArrayBufferLike> | null = await generateImage(tree, fonts); | |
if (!buffer) { | |
throw new Error(`Doesn't generate image for "${slug}"`); | |
} | |
try { | |
await fs.writeFile(path.join(ogDir, `${slug}.png`), buffer); | |
console.log(`${slug} created`); | |
} catch (error) { | |
console.error(`Error writing image file for ${slug}:`, error); | |
throw error; | |
} |
π€ Prompt for AI Agents
In scripts/generate-og-script.ts around lines 71 to 78, the image generation and
file writing operations lack error handling, which can cause unhandled
exceptions. Wrap the code that generates the image buffer and writes the file in
a try-catch block. In the catch block, log or handle the error appropriately to
prevent the script from crashing unexpectedly.
scripts/generate-og-script.ts
Outdated
const fileName: string = `${title.toLowerCase()}.png`.replace(/\s+/g, '-'); | ||
|
||
if (!buffer) { | ||
throw new Error(`Doesn't generate image for "${title}"`); | ||
} | ||
|
||
await fs.writeFile(path.join(ogDir, fileName), buffer); | ||
console.log(`${fileName} created`); | ||
} |
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.
π οΈ Refactor suggestion
Similar error handling needed in page image generation
Apply the same error handling pattern here as suggested for course image generation.
const buffer: Buffer<ArrayBufferLike> | null = await generateImage(tree, fonts);
const fileName: string = `${title.toLowerCase()}.png`.replace(/\s+/g, '-');
if (!buffer) {
throw new Error(`Doesn't generate image for "${title}"`);
}
- await fs.writeFile(path.join(ogDir, fileName), buffer);
- console.log(`${fileName} created`);
+ try {
+ await fs.writeFile(path.join(ogDir, fileName), buffer);
+ console.log(`${fileName} created`);
+ } catch (error) {
+ console.error(`Error writing image file for ${fileName}:`, error);
+ throw error;
+ }
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const fileName: string = `${title.toLowerCase()}.png`.replace(/\s+/g, '-'); | |
if (!buffer) { | |
throw new Error(`Doesn't generate image for "${title}"`); | |
} | |
await fs.writeFile(path.join(ogDir, fileName), buffer); | |
console.log(`${fileName} created`); | |
} | |
const buffer: Buffer<ArrayBufferLike> | null = await generateImage(tree, fonts); | |
const fileName: string = `${title.toLowerCase()}.png`.replace(/\s+/g, '-'); | |
if (!buffer) { | |
throw new Error(`Doesn't generate image for "${title}"`); | |
} | |
try { | |
await fs.writeFile(path.join(ogDir, fileName), buffer); | |
console.log(`${fileName} created`); | |
} catch (error) { | |
console.error(`Error writing image file for ${fileName}:`, error); | |
throw error; | |
} | |
} |
π€ Prompt for AI Agents
In scripts/generate-og-script.ts around lines 122 to 130, the error handling for
the image generation is missing proper checks similar to those used in course
image generation. Add a check to verify if the buffer is valid before attempting
to write the file, and if not, throw an error with a descriptive message
including the title. This ensures consistent and clear error handling for image
generation failures.
Lighthouse Report:
|
// type CourseData = { | ||
// name: string; | ||
// logo: StaticImageData; | ||
// startDate: string; | ||
// }; |
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.
commented code
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.
Deleted comments
// }; | ||
|
||
type CourseLogo = { | ||
src: string; // Data URI ΠΈΠ»ΠΈ URL |
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.
let's use english
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.
Deleted comments
Lighthouse Report:
|
Lighthouse Report:
|
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.
Actionable comments posted: 3
π Outside diff range comments (1)
src/app/courses/[slug]/page.tsx (1)
12-18
: π οΈ Refactor suggestion
params
isnβt aPromise
βtighten the type.
generateMetadata
receives{ params: Params }
, not a promise. The current typing silently widens everything toany
and the extraawait
is a no-op.-type CourseRouteParams = { - params: Promise<Params>; -}; +type CourseRouteParams = { + params: Params; +}; β¦ -const { slug } = await params; +const { slug } = params;This keeps the types honest and removes an unnecessary
await
.
π§Ή Nitpick comments (9)
src/shared/og/utils/fetch-and-convert-to-data-uri.ts (1)
3-28
: Add basic caching & size-guard to cut repeated fetch + sharp costThe helper works, but each OG-image build now performs an unconditional network fetch + full PNG re-encode β expensive when dozens of pages hit the same logo.
Consider:
+// naive in-memory cache β good enough for build-time +const cache = new Map<string, string>(); + export async function fetchAndConvertToDataUri(url: string): Promise<string> { + if (cache.has(url)) return cache.get(url)!; try { const res = await fetch(url); @@ - const pngBuffer = await sharp(buffer) + // Guard against unexpectedly large assets (>2 MB) + if (buffer.byteLength > 2 * 1024 * 1024) { + throw new Error(`Image ${url} is too large (${buffer.byteLength} bytes)`); + } + + const pngBuffer = await sharp(buffer) .png({ compressionLevel: 9, quality: 90, }) .toBuffer(); @@ - return `data:image/png;base64,${base64}`; + const dataUri = `data:image/png;base64,${base64}`; + cache.set(url, dataUri); + return dataUri; } catch (err) {Cuts duplicate work and prevents bloated builds.
src/shared/og/utils/load-image-as-data-uri.ts (1)
5-21
: Preferpath.resolve
and bubble original errorMinor, but improves clarity:
- const buf: Buffer = await fs.readFile(path.join(process.cwd(), relativePath)); + const absPath = path.resolve(process.cwd(), relativePath); + const buf = await fs.readFile(absPath); @@ - } catch (error) { - console.error(`Error loading image from ${relativePath}:`, error); - throw new Error(`Failed to load image as data URI: ${error}`); + } catch (error) { + console.error(`Error loading image from ${relativePath}:`, error); + throw error; // preserves stack & type }Keeps error stacks intact and avoids surprises if
relativePath
already starts with/
.src/views/course/model/store.ts (1)
30-34
: Tiny optimisation: bail early instead of mutating empty array-let sections: Section[] = []; - -if (coursePageSections && coursePageSections.length > 0) { - sections = transformCourseSections(coursePageSections); -} +const sections: Section[] = + coursePageSections?.length ? transformCourseSections(coursePageSections) : [];Not critical, but terser & side-effect-free.
src/shared/og/utils/load-fonts.ts (1)
16-18
: Avoid the extra memory copy when convertingBuffer
βArrayBuffer
.
new Uint8Array(buffer).buffer
allocates a freshUint8Array
. You can get the underlyingArrayBuffer
for free with:-const arrayBuffer = new Uint8Array(buffer).buffer; +const arrayBuffer = buffer.buffer.slice( + buffer.byteOffset, + buffer.byteOffset + buffer.byteLength, +);This shaves an additional allocation and keeps GC pressure lower when the util is imported in many edge routes.
src/app/courses/[slug]/page.tsx (1)
24-26
:robots
should follow Next.js type, not a raw string.
Metadata.robots
expectsRobots
orstring | boolean
. A plain string passes type-checking but loses semantic richness (e.g.,noarchive
). Prefer the object form:-const robots = 'index, follow'; +const robots = { index: true, follow: true };src/shared/og/view/pages-tree/generate-pages-tree.tsx (1)
27-35
: Incorrectalt
text for purely decorative imagery.
The background map and mascot images are decorative in the OG card and should carry emptyalt
to avoid noisy screen-reader output. Conversely,"RS Logo"
is misleading for a map.-<img src={mapBgUri} alt="RS Logo" β¦ +<img src={mapBgUri} alt="" aria-hidden="true" β¦ β¦ -<img src={rsMascotsUri} alt="RS mascots" β¦ +<img src={rsMascotsUri} alt="" aria-hidden="true" β¦Keeps accessibility lint quiet even though OG images arenβt rendered in browsers.
src/app/docs/[lang]/og.png/route.ts (1)
12-20
: Russian title is still English.
titleMap.ru
repeats the English string; probably meant something like"ΠΠΎΠΊΡΠΌΠ΅Π½ΡΠ°ΡΠΈΡ RS School"
. Quick fix:-ru: 'RS School Docs', +ru: 'ΠΠΎΠΊΡΠΌΠ΅Π½ΡΠ°ΡΠΈΡ RS School',src/app/courses/[slug]/og.png/route.ts (2)
41-46
: Uncached remote logo fetch = NΓ network hits at build.
fetchAndConvertToDataUri
will run for every course duringnext export
, potentially hammering the remote CDN. Cache per-process:-let logoDataUri: string; +let logoDataUri: string | undefined = logoCache.get(meta.iconSrc.src); + if (!logoDataUri) { try { - logoDataUri = await fetchAndConvertToDataUri(meta.iconSrc.src); + logoDataUri = await fetchAndConvertToDataUri(meta.iconSrc.src); + logoCache.set(meta.iconSrc.src, logoDataUri); } catch (err) {A simple
Map<string,string>
at module scope suffices.
34-38
:||
collapses legitimate zero dimensions.
Ifwidth
is explicitly0
,||
will fall back to 250. Use nullish-coalescing:-const logoWidth = meta.iconSrc.width || LOGO_FALLBACK_SIZE; -const logoHeight = meta.iconSrc.height || LOGO_FALLBACK_SIZE; +const logoWidth = meta.iconSrc.width ?? LOGO_FALLBACK_SIZE; +const logoHeight = meta.iconSrc.height ?? LOGO_FALLBACK_SIZE;
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
β Files ignored due to path filters (3)
package-lock.json
is excluded by!**/package-lock.json
src/shared/assets/fonts/Inter_28pt-Bold.ttf
is excluded by!**/*.ttf
src/shared/assets/fonts/Inter_28pt-Regular.ttf
is excluded by!**/*.ttf
π Files selected for processing (26)
package.json
(1 hunks)src/app/community/og.png/route.ts
(1 hunks)src/app/community/page.tsx
(1 hunks)src/app/courses/[slug]/og.png/route.ts
(1 hunks)src/app/courses/[slug]/page.tsx
(2 hunks)src/app/courses/og.png/route.ts
(1 hunks)src/app/courses/page.tsx
(1 hunks)src/app/docs/[lang]/[...slug]/page.tsx
(2 hunks)src/app/docs/[lang]/og.png/route.ts
(1 hunks)src/app/docs/[lang]/page.tsx
(1 hunks)src/app/mentorship/[course]/page.tsx
(1 hunks)src/app/mentorship/og.png/route.ts
(1 hunks)src/app/mentorship/page.tsx
(1 hunks)src/app/og.png/route.ts
(1 hunks)src/app/page.tsx
(1 hunks)src/shared/constants.tsx
(0 hunks)src/shared/helpers/generate-page-metadata.ts
(1 hunks)src/shared/og/utils/fetch-and-convert-to-data-uri.ts
(1 hunks)src/shared/og/utils/load-fonts.ts
(1 hunks)src/shared/og/utils/load-image-as-data-uri.ts
(1 hunks)src/shared/og/view/courses-tree/generate-courses-tree.styles.ts
(1 hunks)src/shared/og/view/courses-tree/generate-courses-tree.tsx
(1 hunks)src/shared/og/view/pages-tree/generate-pages-tree.styles.ts
(1 hunks)src/shared/og/view/pages-tree/generate-pages-tree.tsx
(1 hunks)src/shared/types/contentful/TypeHomePage.ts
(1 hunks)src/views/course/model/store.ts
(2 hunks)
π€ Files with no reviewable changes (1)
- src/shared/constants.tsx
β Files skipped from review due to trivial changes (7)
- src/shared/og/view/pages-tree/generate-pages-tree.styles.ts
- src/app/community/og.png/route.ts
- src/app/og.png/route.ts
- src/app/courses/og.png/route.ts
- src/app/mentorship/og.png/route.ts
- src/shared/og/view/courses-tree/generate-courses-tree.tsx
- src/shared/og/view/courses-tree/generate-courses-tree.styles.ts
π§ Files skipped from review as they are similar to previous changes (8)
- package.json
- src/app/courses/page.tsx
- src/app/mentorship/[course]/page.tsx
- src/app/docs/[lang]/[...slug]/page.tsx
- src/app/community/page.tsx
- src/app/mentorship/page.tsx
- src/app/page.tsx
- src/shared/helpers/generate-page-metadata.ts
π§° Additional context used
𧬠Code Graph Analysis (3)
src/app/docs/[lang]/og.png/route.ts (7)
src/app/community/og.png/route.ts (3)
dynamic
(4-4)preferredRegion
(3-3)GET
(6-14)src/app/courses/og.png/route.ts (3)
dynamic
(4-4)preferredRegion
(3-3)GET
(6-14)src/app/mentorship/og.png/route.ts (3)
dynamic
(4-4)preferredRegion
(3-3)GET
(6-14)src/app/og.png/route.ts (3)
dynamic
(4-4)preferredRegion
(3-3)GET
(6-14)src/app/docs/[lang]/page.tsx (1)
generateStaticParams
(34-36)src/app/docs/[lang]/[...slug]/page.tsx (1)
generateStaticParams
(68-108)src/shared/og/view/pages-tree/generate-pages-tree.tsx (1)
createPageTree
(16-47)
src/app/courses/[slug]/page.tsx (2)
src/views/course/model/store.ts (1)
coursePageStore
(77-77)src/shared/helpers/generate-page-metadata.ts (1)
generatePageMetadata
(12-48)
src/app/courses/[slug]/og.png/route.ts (6)
src/app/courses/[slug]/page.tsx (1)
generateStaticParams
(37-39)src/views/course/model/store.ts (1)
coursePageStore
(77-77)src/entities/course/helpers/resolve-course-page-locale.ts (1)
resolveCoursePageLocale
(9-13)src/shared/og/utils/fetch-and-convert-to-data-uri.ts (1)
fetchAndConvertToDataUri
(3-28)src/shared/og/utils/load-image-as-data-uri.ts (1)
loadImageAsDataUri
(5-21)src/shared/og/view/courses-tree/generate-courses-tree.tsx (1)
createCourseTree
(22-57)
β° Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build rs.school
- GitHub Check: CI
π Additional comments (4)
src/views/course/model/store.ts (1)
16-22
:seoDescription
/seoKeywords
may be undefined β keep defaults in syncGood defensive defaults (
''
), but downstream code treatskeywords
as string; later metadata helper may expect string | undefined | string[]. If you make fields optional in Contentful types (see comment above) you can simplify:- const { - title = '', - seoDescription = '', - seoKeywords = '', + const { + title = '', + seoDescription, + seoKeywords,β¦and pass them through untouched; helper can decide fallbacks.
src/shared/og/utils/load-fonts.ts (1)
31-37
: Top-levelawait
can break older toolchains / Jest.
While Next.js (Edge / Node 18+) happily supports TLA, any consumer that still runs Jest under CommonJS ("type": "commonjs"
) will explode. Consider lazily exportinggetFonts()
that internally memoises the promise:-const [fontRegular, fontBold] = await Promise.all([ β¦ ]); -export const fonts: Font[] = [fontRegular, fontBold]; +let fontsPromise: Promise<Font[]> | null = null; +export const getFonts = (): Promise<Font[]> => { + if (!fontsPromise) { + fontsPromise = Promise.all([loadFont(400), loadFont(700)]); + } + return fontsPromise; +};This sidesteps the TLA requirement entirely.
src/shared/og/view/pages-tree/generate-pages-tree.tsx (1)
12-15
: Eager asset loading blocks every route import.
Declaring the threePromise
s at module scope means any failed read kills all pages that import this helper. Wrap in a try/catch insidecreatePageTree
or memoise with a safe fallback to prevent a single missing asset from taking the whole OG generation offline.src/app/docs/[lang]/og.png/route.ts (1)
22-33
: Missingdynamic = 'force-static'
consistency.
Other OG routes exportdynamic = 'force-static'
to guarantee SSG. Omitting it here may leave the docs OG images in βautoβ mode, breakingnext export
.
export async function generateMetadata({ | ||
params, | ||
}: { | ||
params: Promise<RouteParams>; | ||
}) { | ||
const { lang } = await params; | ||
const title = `RS School Overview ${TITLE_POSTFIX}`; |
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.
params
isnβt a Promise β remove the await
pattern
generateMetadata
receives { params: RouteParams }
, not a Promise.
Awaiting a non-promise silently succeeds but is misleading and trips up tooling.
-export async function generateMetadata({
- params,
-}: {
- params: Promise<RouteParams>;
-}) {
- const { lang } = await params;
+export async function generateMetadata({ params }: { params: RouteParams }) {
+ const { lang } = params;
Apply the same correction to DocsIndex
below for consistency.
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export async function generateMetadata({ | |
params, | |
}: { | |
params: Promise<RouteParams>; | |
}) { | |
const { lang } = await params; | |
const title = `RS School Overview ${TITLE_POSTFIX}`; | |
export async function generateMetadata({ params }: { params: RouteParams }) { | |
const { lang } = params; | |
const title = `RS School Overview ${TITLE_POSTFIX}`; | |
// β¦ | |
} |
π€ Prompt for AI Agents
In src/app/docs/[lang]/page.tsx around lines 9 to 15, the function
generateMetadata incorrectly treats params as a Promise and uses await on it,
but params is actually a plain object of type RouteParams. Remove the await
keyword before params and adjust the type annotation to reflect that params is
not a Promise. Also, apply the same fix to the DocsIndex function below by
removing any await usage on params and correcting its type accordingly for
consistency.
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.
This is part of the breaking changes that came with v15
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.
@LaraNU Got itβthanks for the heads-up. Next.js v15 indeed changed the App Router helpers so that params
(and other context values) are now delivered as Promises, which makes the existing await
usage correct. Iβll keep that in mind for future reviews and retract the previous suggestion. π
βοΈ Learnings added
Learnt from: LaraNU
PR: rolling-scopes/site#894
File: src/app/docs/[lang]/page.tsx:9-15
Timestamp: 2025-06-11T12:39:10.882Z
Learning: In Next.js v15, App Router helpers such as `generateMetadata` now receive context properties (`params`, `searchParams`, etc.) as Promises, so awaiting them is required and correct.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Lighthouse Report:
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
Problem
Project uses output: 'export' which limits dynamic image generation capabilities Based on the solution described in:
When running the project in development mode (npm run dev), developers need to:
Modify metadataBase in the root layout:
Related Tickets & Documents
Screenshots, Recordings
I've deployed a test version with the new metadata system: Preview Deployment
For courses

For pages

Added/updated tests?
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
.gitignore
to exclude generated Open Graph image assets.