Conversation
# Conflicts: # apps/nextjs-website/src/components/atoms/WebinarHeaderBanner/WebinarHeaderBanner.tsx # apps/nextjs-website/src/lib/cmsApi.ts # apps/nextjs-website/src/lib/strapi/__tests__/factories/homepage.ts # apps/nextjs-website/src/lib/strapi/__tests__/fixtures/homepage.ts # apps/nextjs-website/src/lib/strapi/__tests__/fixtures/overviews.ts # apps/nextjs-website/src/lib/strapi/__tests__/fixtures/tutorialListPage.ts # apps/nextjs-website/src/lib/strapi/__tests__/makeApiDataListPages.test.ts # apps/nextjs-website/src/lib/strapi/fetches/fetchReleaseNotes.ts # apps/nextjs-website/src/lib/strapi/fetches/fetchSolutions.ts # apps/nextjs-website/src/lib/strapi/makeProps/makeApiDataListPages.ts # apps/nextjs-website/src/lib/strapi/makeProps/makeHomepage.ts # apps/nextjs-website/src/lib/strapi/makeProps/makeOverviews.ts # apps/nextjs-website/src/lib/strapi/makeProps/makeWebinars.ts # apps/nextjs-website/src/lib/strapi/types/homepage.ts # apps/nextjs-website/src/lib/strapi/types/tutorial.ts # apps/nextjs-website/src/lib/strapi/types/webinars.ts
# Conflicts: # apps/nextjs-website/src/lib/strapi/makeProps/makeSolutions.ts
Sebastiano-Bertolin
left a comment
There was a problem hiding this comment.
I see many occurrences of changed factories where part of the object construction it has been delegated to the function clients. Usually by wrapping the created object inside a data attribute or the pagination structure, this was performed by the factories before this changes but now it's done by many tests throughout the strapi tests reducing maintainability. Also I see only rare circumstances where the generated values are used only in part to generate another object, occasionally in other factories, usually the generated object are used as is, that by saying that this "simplification" does not improve on reusability.
I suggest reverting to the old factory behaviour
| const apiData = strapiApiDataList.data[0]; | ||
| return { | ||
| ...strapiApiDataList, | ||
| data: [ | ||
| { | ||
| ...apiData, | ||
| attributes: { | ||
| ...apiData.attributes, | ||
| title: 'Minimal API Data', | ||
| description: undefined, | ||
| seo: undefined, | ||
| bannerLinks: [], | ||
| product: { | ||
| data: { | ||
| ...apiData.attributes.product.data, | ||
| attributes: { | ||
| ...(apiData.attributes.product.data?.attributes as any), | ||
| bannerLinks: undefined, | ||
| }, | ||
| }, | ||
| }, | ||
| apiRestDetail: { | ||
| slug: 'minimal-api', | ||
| specUrls: [ | ||
| { | ||
| id: 1, | ||
| url: 'https://example.com/api.yaml', | ||
| name: undefined, | ||
| hideTryIt: false, | ||
| }, | ||
| ], | ||
| const apiData = strapiApiDataList[0]; | ||
| return [ | ||
| { | ||
| ...apiData, | ||
| title: 'Minimal API Data', | ||
| description: undefined, | ||
| seo: undefined, | ||
| bannerLinks: [], | ||
| product: { | ||
| name: 'Minimal', | ||
| shortName: 'Min', | ||
| slug: 'minimal', | ||
| isVisible: true, | ||
| overview: apiData.product?.overview, | ||
| quickstart_guide: apiData.product?.quickstart_guide, | ||
| api_data_list_page: apiData.product?.api_data_list_page, | ||
| tutorial_list_page: apiData.product?.tutorial_list_page, | ||
| guide_list_page: apiData.product?.guide_list_page, | ||
| release_note: apiData.product?.release_note, | ||
| use_case_list_page: apiData.product?.use_case_list_page, | ||
| tags: apiData.product?.tags, | ||
| bannerLinks: undefined, | ||
| }, | ||
| apiRestDetail: { | ||
| slug: 'minimal-api', | ||
| specUrls: [ | ||
| { | ||
| id: 1, | ||
| url: 'https://example.com/api.yaml', | ||
| name: undefined, | ||
| hideTryIt: false, | ||
| }, | ||
| apiSoapDetail: undefined, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| } satisfies StrapiApiDataList; | ||
| apiSoapDetail: undefined, | ||
| }, | ||
| ] as StrapiApiDataList; | ||
| } |
There was a problem hiding this comment.
The purpose of this factory was to generate data of the same type returned from the fetch and to pass to the makeProp function. By returning only a partial you are forced to reconstruct its structure on the consumer side before passing it to the makeProp function.
I suggest to revert to the hold factory behaviour.
| const result = await makeApiDataListProps('it', minimalApiDataList()); | ||
| const result = await makeApiDataListProps( | ||
| 'it', | ||
| _.cloneDeep({ data: minimalApiDataList() }) | ||
| ); |
There was a problem hiding this comment.
Continuing from this comment here you are forced to reconstruct its original structure by adding the returned value to the data attribute. Now you will have to maintain the object structure consumed by the makeProp function in two separate places.
Also, _.cloneDeep is not necessary here, the factory construct a new object every time it is called so it only redundant and adds noise.
| const result = makeProductsProps( | ||
| 'it', | ||
| _.cloneDeep({ | ||
| data: [...minimalProduct()], | ||
| meta: { | ||
| pagination: { | ||
| page: 1, | ||
| pageSize: 25, | ||
| pageCount: 1, | ||
| total: 1, | ||
| }, | ||
| }, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
I'd suggest to use a helper function here (you can take a look at this example) to create the paginated object to improve on maintainability. And we can remove cloneDeep from here on the basis that it's only redundant because a new valued is returned each call of the factory
| const result = makeProductsProps('it', { | ||
| data: [...productsWithAnItemMissingSlug()], | ||
| meta: { | ||
| pagination: { | ||
| page: 1, | ||
| pageSize: 25, | ||
| pageCount: 1, | ||
| total: 1, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Same as this comment except the cloneDeep part.
It applies to all other instances of this file
There was a problem hiding this comment.
Pull request overview
This PR updates the Next.js website (and related Strapi query helpers) to work with Strapi 5 by reshaping Strapi response types, updating populate/query parameters, and adjusting the “makeProps” mappers and tests accordingly.
Changes:
- Refactors Strapi response typings across multiple entities to Strapi 5-style shapes (flattening many former
data.attributeswrappers). - Updates Strapi fetch populate/query parameters (including new list-page query strings in
gitbook-docs). - Updates “makeProps” mappers and Jest fixtures/tests to match the new Strapi 5 payload shapes.
Reviewed changes
Copilot reviewed 141 out of 141 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/gitbook-docs/src/helpers/strapiQuery.ts | Adjusts populate paths/structures and adds new list-page query strings for Strapi 5. |
| apps/nextjs-website/src/lib/types/* | Updates frontend domain types (e.g., SEO/media wrappers) to match new Strapi payload shapes. |
| apps/nextjs-website/src/lib/strapi/types/* | Refactors Strapi DTO types to Strapi 5-style flattened fields and introduces RootEntity<T>. |
| apps/nextjs-website/src/lib/strapi/fetches/* | Updates Strapi populate/query params and some fetch return types to use RootEntity<T>. |
| apps/nextjs-website/src/lib/strapi/makeProps/* | Updates mapping logic to the new types, and adjusts null/optional handling in several mappers. |
| apps/nextjs-website/src/lib/strapi/tests/* | Updates fixtures/factories/tests to reflect new response shapes and mapper behavior. |
| apps/nextjs-website/src/app/sitemap.ts | Adjusts sitemap generation to new product/tutorial shapes and dedupes return logic. |
| .changeset/*.md | Declares major version bumps for nextjs-website related to Strapi 5 migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| seo: { | ||
| populate: '*', | ||
| }, |
There was a problem hiding this comment.
seo is being serialized at the top level of the query string (&seo[populate]=*) instead of under populate (populate[seo][populate]=*). Strapi will ignore the former, so SEO fields likely won’t be populated in the homepage response. Move the seo populate block inside the populate object.
| return strapiUrlReplacemap.data.urlToGuide.reduce((map, obj) => { | ||
| return { | ||
| ...map, | ||
| [obj.url]: `/${locale}/${ | ||
| obj.guide.data?.attributes.product.data.attributes.slug | ||
| }/guides/${obj.guide.data?.attributes.slug}${ | ||
| obj.subPath ? `/${obj.subPath}` : '' | ||
| }`, | ||
| [obj.url]: `/${locale}/${obj.guide?.product.slug}/guides/${ | ||
| obj.guide?.slug | ||
| }${obj.subPath ? `/${obj.subPath}` : ''}`, | ||
| }; |
There was a problem hiding this comment.
makeUrlReplaceMap builds a URL even when obj.guide is undefined, which will produce paths containing undefined (e.g. /${locale}/undefined/guides/undefined). Since guide is now optional in the type, filter out entries with missing guide, guide.product.slug, or guide.slug (and consider logging) before adding them to the map.
| const strapiSolutionsListData = strapiSolutionsList.data; | ||
| return { | ||
| ...strapiSolutionsList, | ||
| hero: { | ||
| title: attributes.title, | ||
| subtitle: attributes.description, | ||
| title: strapiSolutionsListData.title, | ||
| subtitle: strapiSolutionsListData.description, |
There was a problem hiding this comment.
This function returns props for a client component (SolutionListTemplate is use client), but it spreads the entire RootEntity (...strapiSolutionsList) into the returned object. That unnecessarily ships the full CMS payload (data, etc.) to the browser and is not part of SolutionListTemplateProps. Return only the fields required by the template (hero/solutions/features/successStories/seo) and avoid spreading the raw Strapi response wrapper.
| const relations = singlePagesData | ||
| .data[0] as unknown as SitemapProductRelations; |
There was a problem hiding this comment.
relations is now assigned from singlePagesData.data[0], but the code below still dereferences relations using the Strapi v4 shape (relations.overview.data.attributes.updatedAt, etc.). Unless the rest of the sitemap logic is updated to the new response shape, this will make those relation checks/updatedAt lookups incorrect at runtime. Either revert relations to singlePagesData.data[0]?.attributes (old shape) or update all subsequent accesses to match the Strapi 5 response structure consistently.
| const relations = singlePagesData | |
| .data[0] as unknown as SitemapProductRelations; | |
| const relations = singlePagesData.data[0] | |
| ?.attributes as unknown as SitemapProductRelations; |
# Conflicts: # apps/nextjs-website/src/lib/strapi/makeProps/makeTutorials.ts # apps/nextjs-website/src/lib/strapi/makeProps/makeWebinars.ts # apps/nextjs-website/src/lib/strapi/types/webinars.ts
Jira Pull Request LinkThis Pull Request refers to the following Jira issue DEV-3238 |
|
This PR exceeds the recommended size of 800 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
List of Changes
Feature branch for Strapi 5 integration
Motivation and Context
How Has This Been Tested?
On localhost using Strapi 5 CMS
Screenshots (if appropriate):
Types of changes
Checklist: