Skip to content

WS-197-Added route to render Homepages in Next.js app #13529

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.

Sign up for GitHub

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

Jump to bottom
Merged
Nabeel1276 merged 33 commits into from
Dec 17, 2025
Merged

WS-197-Added route to render Homepages in Next.js app #13529

Nabeel1276 merged 33 commits into from
Dec 17, 2025
+299 −14

Conversation

@Nabeel1276
Copy link
Contributor

@Nabeel1276 Nabeel1276 commented Dec 10, 2025 •
edited
Loading

Resolves JIRA: https://bbc.atlassian.net/browse/WS-197

Summary

Before:
image

After:
image

The task at hand, adds support for homepage routes in the Next.js app by implementing a new home page route handler and integrating it into the routing system.

Key Changes:

  • New homepage route handler with data fetching and error handling
  • Integration of homepage routing into the main catch-all route handler
  • Comprehensive unit tests for the new homepage handler
  • Updated documentation from frontpages to homepages
  • Moved shouldRender function into utilities folder and updated imports as required

Code changes

  • Created new handleHomepageRoute file to fetch homepage data
  • Created handleHomepageRoute test file for unit testing homepageroute
  • Called Homepage component to render data

Developer Checklist

  • UX
    • UX Criteria met (visual UX & screenreader UX)
  • Accessibility
    • Accessibility Acceptance Criteria met
    • Accessibility swarm completed
    • Component Health updated
    • P1 accessibility bugs resolved
    • P2/P3 accessibility bugs planned (if not resolved)
  • Security
    • Security issues addressed
    • Threat Model updated
  • Documentation
    • Docs updated (runbook, READMEs)
  • Testing
    • Feature tested on relevant environments
  • Comms
    • Relevant parties notified of changes

Testing

  • Manual Testing required?
    • Local (Ready-For-Test, Local)
    • Test (Ready-For-Test, Test)
    • Preview (Ready-For-Test, Preview)
    • Live (Ready-For-Test, Live)
  • Manual Testing complete?
    • Local
    • Test
    • Preview
    • Live

Additional Testing Steps

  1. List the steps required to test this PR.

Useful Links

@Nabeel1276 Nabeel1276 requested a review from Copilot December 11, 2025 11:54
Copilot AI reviewed Dec 11, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment •
edited by Nabeel1276
Loading

Choose a reason for hiding this comment

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

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
ws-nextjs-app/pages/[service]/homepages/handleHomepageRoute.ts Implements server-side handler for homepage routes with data fetching, caching, and error handling
ws-nextjs-app/pages/[service]/homepages/handleHomepageRoute.test.ts Adds unit tests covering success, error, and edge cases for the homepage handler
ws-nextjs-app/pages/[service]/[[...]].page.tsx Integrates homepage handling into the main routing logic by adding HOME_PAGE type detection and routing

@Nabeel1276 Nabeel1276 self-assigned this Dec 11, 2025
@Nabeel1276 Nabeel1276 changed the title WS-197-Added route for homepages as well as condition for page type header WS-197-Added route for homepages for NextJS Dec 11, 2025
@Nabeel1276 Nabeel1276 changed the title WS-197-Added route for homepages for NextJS WS-197-Added route to render Homepages in Next.js app Dec 11, 2025
amoore108
amoore108 reviewed Dec 12, 2025
View reviewed changes
@amoore108
Copy link
Contributor

amoore108 commented Dec 14, 2025

We'll need to add a check for HOME_PAGE in here: https://github.com/bbc/simorgh/blob/9ea7b19a3b202ccfc241bfc0692ca1f26d3b6040/ws-nextjs-app/utilities/derivePageType/index.ts

This is a bit harder to check against compared to the other routes, so I'd suggest something along the lines of using import SERVICES from '#app/lib/config/services';, which is an array of all of our service names, to check if the sanitisedPathname ends with a service name and if so, return HOME_PAGE

@Nabeel1276
Copy link
Contributor Author

Nabeel1276 commented Dec 15, 2025

We'll need to add a check for HOME_PAGE in here: https://github.com/bbc/simorgh/blob/9ea7b19a3b202ccfc241bfc0692ca1f26d3b6040/ws-nextjs-app/utilities/derivePageType/index.ts

This is a bit harder to check against compared to the other routes, so I'd suggest something along the lines of using import SERVICES from '#app/lib/config/services';, which is an array of all of our service names, to check if the sanitisedPathname ends with a service name and if so, return HOME_PAGE

Can I ask how you knew to do that, as well as why does this not need to be done for article routes?

@amoore108
Copy link
Contributor

amoore108 commented Dec 15, 2025

Can I ask how you knew to do that, as well as why does this not need to be done for article routes?

I implemented this function originally, so knew it was required. If you follow through the logic in _document.page.tsx, you'll see that derivePageType is used for logging, so thats one of the other reasons it needs added.

It has been done for the article routes. If you check the file you'll see Optimo and CPS ID checks that determine if its an ARTICLE_PAGE.

@Nabeel1276
Copy link
Contributor Author

Nabeel1276 commented Dec 15, 2025

Can I ask how you knew to do that, as well as why does this not need to be done for article routes?

I implemented this function originally, so knew it was required. If you follow through the logic in _document.page.tsx, you'll see that derivePageType is used for logging, so thats one of the other reasons it needs added.

It has been done for the article routes. If you check the file you'll see Optimo and CPS ID checks that determine if its an ARTICLE_PAGE.

Thanks for this, I meant why there doesn't need to be a check for the service? I presume it's because the CPS and Optimo IDs are checked instead?

@amoore108
Copy link
Contributor

amoore108 commented Dec 15, 2025

Thanks for this, I meant why there doesn't need to be a check for the service? I presume it's because the CPS and Optimo IDs are checked instead?

Ah sorry! Yea the CPS and Optimo IDs will only be used for article pages, so that's a good enough method of checking that the page is likely an 'article' page.

Homepages don't have any unique identifiers or patterns we can match on other than 'service'.

Thinking more about it, what you have looks good but we'll need to consider variants at the end of the pathname too.

amoore108
amoore108 reviewed Dec 15, 2025
View reviewed changes
amoore108
amoore108 reviewed Dec 15, 2025
View reviewed changes
amoore108
amoore108 reviewed Dec 15, 2025
View reviewed changes
amoore108
amoore108 reviewed Dec 15, 2025
View reviewed changes
@Nabeel1276 Nabeel1276 requested a review from amoore108 December 15, 2025 15:43
amoore108
amoore108 reviewed Dec 15, 2025
View reviewed changes
ws-nextjs-app/pages/[service]/homepage/handleHomepageRoute.ts Outdated

context.res.setHeader(
'Cache-Control',
'public, stale-if-error=90, stale-while-revalidate=30, max-age=60',
Copy link
Contributor

@amoore108 amoore108 Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just checked on Live and the cache-control for homepages is currently set to public, stale-if-error=300, stale-while-revalidate=120, max-age=30, so I think we should keep them like that for the migration at least.

Copy link
Contributor Author

@Nabeel1276 Nabeel1276 Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry can you please send a link or screenshot as to where on live you found this information?

Copy link
Contributor

@amoore108 amoore108 Dec 15, 2025

Choose a reason for hiding this comment

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

If you open chrome dev tools and navigate to the 'Network' tab and refresh the page, you should see the requests for the page. You can filter by Doc to see the actual document, if you click on it in the list it should open a side panel. Under Request Headers, you should see cache-control.

amoore108
amoore108 reviewed Dec 16, 2025
View reviewed changes
ws-nextjs-app/pages/[service]/homepage/handleHomepageRoute.ts Outdated
rendererEnv,
resolvedUrl: resolvedUrlWithoutQuery,
pageType: HOME_PAGE,
isAmp,
Copy link
Contributor

@amoore108 amoore108 Dec 16, 2025

Choose a reason for hiding this comment

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

We can remove isAmp from this as we don't support AMP on Homepages.

Copy link
Contributor Author

@Nabeel1276 Nabeel1276 Dec 16, 2025

Choose a reason for hiding this comment

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

Sorry just another question, looking at the readme throughout the codebase, it seems that AMP supports frontpages which are the older versions of homepages. So they support frontpages but not homepages, is that correct?

Copy link
Contributor

@amoore108 amoore108 Dec 16, 2025

Choose a reason for hiding this comment

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

Could be something that needs removed from the readme perhaps. I don't think we have frontpages anymore.

Copy link
Contributor Author

@Nabeel1276 Nabeel1276 Dec 16, 2025

Choose a reason for hiding this comment

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

Is it worth doing that as part of this work? Just a matter of deletions i'd imagine...

Copy link
Contributor

@amoore108 amoore108 Dec 16, 2025

Choose a reason for hiding this comment

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

Is it this bit you're talking about? https://github.com/bbc/simorgh/blob/eab8bd878c95532bfadf5236d9ac3d9b5636b48f/README.md#front-pages

If so, I would rename those to Home pages and remove the AMP references and links 👍

amoore108
amoore108 approved these changes Dec 16, 2025
View reviewed changes
Copy link
Contributor

@amoore108 amoore108 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for addressing all the comments 🙏

@amoore108
Copy link
Contributor

amoore108 commented Dec 16, 2025

@Nabeel1276 I merged in #13530 and merged its changes into here, so its change [[...]].page.tsx slightly but not too much

@Nabeel1276 Nabeel1276 merged commit 40e2b16 into latest Dec 17, 2025
13 checks passed
@Nabeel1276 Nabeel1276 deleted the WS-197-create-route-in-simorgh-to-render-homepages-nextJS branch December 17, 2025 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amoore108 amoore108 amoore108 approved these changes

Copilot code review Copilot Copilot left review comments

@LilyL0u LilyL0u LilyL0u approved these changes

Assignees

@Nabeel1276 Nabeel1276

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Nabeel1276 @amoore108 @LilyL0u