From aa276260c351c90d32b4a09c1e2a3b8578ac03c9 Mon Sep 17 00:00:00 2001 From: Chris Bongers Date: Tue, 15 Sep 2026 10:45:04 +0200 Subject: [PATCH 1/3] refactor(feed): source the hero from the dedicated feedHero query The section fetched majorHeadlines, sliced the leading four and hydrated them through feedByIds, then re-keyed the answer by id to get its own order back. Both counts lived in the client, and feedByIds is @auth, so the cards never rendered for a logged-out reader on Popular. feedHero returns the posts and the headlines together, already in order and index-aligned, so the section only asks and renders. How many of each the hero shows is now the server's call. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/feeds/hero/FeedHero.spec.tsx | 103 ++++++++++++++++++ .../src/components/feeds/hero/FeedHero.tsx | 55 +++------- packages/shared/src/graphql/feed.ts | 30 +++++ packages/shared/src/lib/query.ts | 1 + 4 files changed, 152 insertions(+), 37 deletions(-) create mode 100644 packages/shared/src/components/feeds/hero/FeedHero.spec.tsx diff --git a/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx b/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx new file mode 100644 index 0000000000..836f549392 --- /dev/null +++ b/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx @@ -0,0 +1,103 @@ +import React from 'react'; +import nock from 'nock'; +import { render, screen, waitFor } from '@testing-library/react'; +import { QueryClient } from '@tanstack/react-query'; +import type { NextRouter } from 'next/router'; +import { useRouter } from 'next/router'; +import basePost from '../../../../__tests__/fixture/post'; +import { TestBootProvider } from '../../../../__tests__/helpers/boot'; +import { mockGraphQL } from '../../../../__tests__/helpers/graphql'; +import { + FEED_HERO_QUERY, + supportedTypesForPrivateSources, +} from '../../../graphql/feed'; +import { FeedHero } from './FeedHero'; +import { feedHeroShape } from './feedHeroShape'; +import { useFeedHeroAd } from './useFeedHeroAd'; + +jest.mock('next/router', () => ({ + useRouter: jest.fn(), +})); + +jest.mock('./useFeedHeroAd', () => ({ + useFeedHeroAd: jest.fn(), +})); + +const posts = ['First hero post', 'Second hero post'].map((title, index) => ({ + ...basePost, + id: `hero-post-${index}`, + title, +})); + +const highlights = ['First headline', 'Second headline', 'Third headline'].map( + (headline, index) => ({ + id: `hero-highlight-${index}`, + channel: 'agents', + headline, + highlightedAt: '2026-04-05T09:00:00.000Z', + post: { + id: `hero-post-${index}`, + commentsPermalink: `/posts/hero-post-${index}`, + }, + }), +); + +beforeEach(() => { + jest.clearAllMocks(); + nock.cleanAll(); + jest + .mocked(useRouter) + .mockImplementation(() => ({ pathname: '/' } as unknown as NextRouter)); + jest.mocked(useFeedHeroAd).mockReturnValue({ + ad: undefined, + placement: 'none', + shape: feedHeroShape(3), + }); +}); + +const mockHero = (data: { + posts: typeof posts; + highlights: typeof highlights; +}) => + mockGraphQL({ + request: { + query: FEED_HERO_QUERY, + variables: { + loggedIn: false, + supportedTypes: supportedTypesForPrivateSources, + }, + }, + result: { data: { feedHero: data } }, + }); + +const renderComponent = () => + render( + + + , + ); + +describe('FeedHero', () => { + it('should render its cards and its rail from the one query', async () => { + mockHero({ posts, highlights }); + + renderComponent(); + + await waitFor(() => + expect( + screen.getByRole('heading', { name: 'First hero post' }), + ).toBeInTheDocument(), + ); + expect(screen.getByText('Second headline')).toBeInTheDocument(); + expect(screen.getByText('Third headline')).toBeInTheDocument(); + }); + + it('should render nothing when the query returns no posts', async () => { + mockHero({ posts: [], highlights: [] }); + + const { container } = renderComponent(); + + await waitFor(() => expect(nock.isDone()).toBe(true)); + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/packages/shared/src/components/feeds/hero/FeedHero.tsx b/packages/shared/src/components/feeds/hero/FeedHero.tsx index e42306e3cd..d29e939972 100644 --- a/packages/shared/src/components/feeds/hero/FeedHero.tsx +++ b/packages/shared/src/components/feeds/hero/FeedHero.tsx @@ -2,14 +2,13 @@ import type { ReactElement } from 'react'; import React, { useCallback, useEffect, useMemo } from 'react'; import { useQuery } from '@tanstack/react-query'; import type { Post } from '../../../graphql/posts'; -import type { Connection } from '../../../graphql/common'; import { gqlClient } from '../../../graphql/common'; +import type { FeedHeroData } from '../../../graphql/feed'; import { - FEED_BY_IDS_QUERY, + FEED_HERO_QUERY, supportedTypesForPrivateSources, } from '../../../graphql/feed'; import type { PostHighlight } from '../../../graphql/highlights'; -import { majorHeadlinesQueryOptions } from '../../../graphql/highlights'; import type { ViewabilityData } from '../../../features/monetization/viewability'; import { viewabilityLogExtra } from '../../../features/monetization/viewability'; import { useAuthContext } from '../../../contexts/AuthContext'; @@ -29,15 +28,14 @@ import { generateQueryKey, RequestKey, StaleTime } from '../../../lib/query'; import { FeedHeroSection } from './FeedHeroSection'; import { useFeedHeroAd } from './useFeedHeroAd'; -const HIGHLIGHT_COUNT = 6; -const FEATURED_POST_COUNT = 4; // Distinct from `Origin.Feed` so the experiment can tell the hero's clicks and // impressions apart from the grid's. Matches the ad events' own origin. const HERO_ORIGIN = 'feed hero'; /** - * The carousel and the Happening Now list are the same headlines: the top few - * get their full post fetched for a card, the rest stay as rows. + * The carousel and the Happening Now list are the same headlines: `feedHero` + * returns the leading few already hydrated into posts for the cards, and every + * headline it kept for the rows beside them. */ export const FeedHero = ({ feedName, @@ -66,42 +64,25 @@ export const FeedHero = ({ const { ad, placement, shape } = useFeedHeroAd(); - const { data: headlines } = useQuery({ - ...majorHeadlinesQueryOptions({ first: HIGHLIGHT_COUNT }), - enabled: tokenRefreshed, - }); - const highlights = useMemo( - () => headlines?.majorHeadlines?.edges?.map(({ node }) => node) ?? [], - [headlines], - ); - - const postIds = useMemo( - () => highlights.slice(0, FEATURED_POST_COUNT).map(({ post }) => post.id), - [highlights], - ); - - const { data: featured } = useQuery({ - queryKey: generateQueryKey(RequestKey.FeedByIds, user, 'hero', ...postIds), + const { data: hero } = useQuery({ + queryKey: generateQueryKey(RequestKey.FeedHero, user), queryFn: () => - gqlClient.request<{ page: Connection }>(FEED_BY_IDS_QUERY, { - first: postIds.length, - postIds, + // How many headlines, and how many of them get a card, are the server's + // call — so the mix can be retuned without shipping a client. + gqlClient.request(FEED_HERO_QUERY, { loggedIn: !!user, supportedTypes: supportedTypesForPrivateSources, }), - enabled: tokenRefreshed && postIds.length > 0, - staleTime: StaleTime.Default, + enabled: tokenRefreshed, + // Breaking headlines, so the same minute the headline query kept rather + // than the five the post hydration used to. + staleTime: StaleTime.OneMinute, }); - // `feedByIds` answers in its own order, so re-key by id to keep the carousel - // in the same order as the headlines beside it. - const posts = useMemo(() => { - const byId = new Map( - featured?.page?.edges?.map(({ node }) => [node.id, node]) ?? [], - ); - - return postIds.map((id) => byId.get(id)).filter(Boolean) as Post[]; - }, [featured, postIds]); + const highlights = useMemo(() => hero?.feedHero?.highlights ?? [], [hero]); + // Already in headline order and index-aligned with the highlights above, so + // the carousel reads in the same order as the list beside it. + const posts: Post[] = useMemo(() => hero?.feedHero?.posts ?? [], [hero]); const isRendered = posts.length > 0; const adPlacement = isRendered ? placement : 'none'; diff --git a/packages/shared/src/graphql/feed.ts b/packages/shared/src/graphql/feed.ts index 3aa7ff3c32..28e6965bd0 100644 --- a/packages/shared/src/graphql/feed.ts +++ b/packages/shared/src/graphql/feed.ts @@ -412,6 +412,36 @@ export const FEED_V2_QUERY = gql` ${POST_HIGHLIGHT_FRAGMENT} `; +export interface FeedHeroData { + feedHero: { + posts: Post[]; + highlights: PostHighlight[]; + }; +} + +export const FEED_HERO_QUERY = gql` + query FeedHero( + $loggedIn: Boolean! = false + $first: Int + $featured: Int + ${SUPPORTED_TYPES} + ) { + feedHero(first: $first, featured: $featured, supportedTypes: $supportedTypes) { + posts { + ...FeedPost + contentHtml + ...UserPost @include(if: $loggedIn) + } + highlights { + ...PostHighlightCard + } + } + } + ${FEED_POST_FRAGMENT} + ${USER_POST_FRAGMENT} + ${POST_HIGHLIGHT_FRAGMENT} +`; + export const MOST_UPVOTED_FEED_QUERY = gql` query MostUpvotedFeed( $loggedIn: Boolean! = false diff --git a/packages/shared/src/lib/query.ts b/packages/shared/src/lib/query.ts index e9f0006a6d..4eeb4e4f15 100644 --- a/packages/shared/src/lib/query.ts +++ b/packages/shared/src/lib/query.ts @@ -189,6 +189,7 @@ export enum RequestKey { FeedSettings = 'feedSettings', Ads = 'ads', FeedByIds = 'feedByIds', + FeedHero = 'feedHero', SlackChannels = 'slack_channels', IntegrationRecentChannels = 'integration_recent_channels', UserIntegrations = 'user_integrations', From bf2e432d5c5394a2ee594d830c20cae63dff1441 Mon Sep 17 00:00:00 2001 From: Chris Bongers Date: Tue, 15 Sep 2026 11:25:42 +0200 Subject: [PATCH 2/3] refactor(feed): trim the hero query to what it sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit first and featured were declared and forwarded but never passed, shadowing the server defaults they would have to be kept in step with. The server owns the mix; the document no longer pretends otherwise. staleTime back to Default. OneMinute was the headline query's, from when the posts sat at Default behind it — under one query it would re-pull four full cards on every window focus after a minute, and the section is not a ticker. Co-Authored-By: Claude Opus 5 (1M context) --- packages/shared/src/components/feeds/hero/FeedHero.tsx | 9 ++------- packages/shared/src/graphql/feed.ts | 9 ++------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/packages/shared/src/components/feeds/hero/FeedHero.tsx b/packages/shared/src/components/feeds/hero/FeedHero.tsx index d29e939972..3d6a38864a 100644 --- a/packages/shared/src/components/feeds/hero/FeedHero.tsx +++ b/packages/shared/src/components/feeds/hero/FeedHero.tsx @@ -67,21 +67,16 @@ export const FeedHero = ({ const { data: hero } = useQuery({ queryKey: generateQueryKey(RequestKey.FeedHero, user), queryFn: () => - // How many headlines, and how many of them get a card, are the server's - // call — so the mix can be retuned without shipping a client. gqlClient.request(FEED_HERO_QUERY, { loggedIn: !!user, supportedTypes: supportedTypesForPrivateSources, }), enabled: tokenRefreshed, - // Breaking headlines, so the same minute the headline query kept rather - // than the five the post hydration used to. - staleTime: StaleTime.OneMinute, + staleTime: StaleTime.Default, }); const highlights = useMemo(() => hero?.feedHero?.highlights ?? [], [hero]); - // Already in headline order and index-aligned with the highlights above, so - // the carousel reads in the same order as the list beside it. + /** In headline order and index-aligned with the highlights, per `feedHero`. */ const posts: Post[] = useMemo(() => hero?.feedHero?.posts ?? [], [hero]); const isRendered = posts.length > 0; diff --git a/packages/shared/src/graphql/feed.ts b/packages/shared/src/graphql/feed.ts index 28e6965bd0..01723af9b2 100644 --- a/packages/shared/src/graphql/feed.ts +++ b/packages/shared/src/graphql/feed.ts @@ -420,13 +420,8 @@ export interface FeedHeroData { } export const FEED_HERO_QUERY = gql` - query FeedHero( - $loggedIn: Boolean! = false - $first: Int - $featured: Int - ${SUPPORTED_TYPES} - ) { - feedHero(first: $first, featured: $featured, supportedTypes: $supportedTypes) { + query FeedHero($loggedIn: Boolean! = false, ${SUPPORTED_TYPES}) { + feedHero(supportedTypes: $supportedTypes) { posts { ...FeedPost contentHtml From 67ff0ea1cab9791eb5212355362a841c5cbc7d7d Mon Sep 17 00:00:00 2001 From: Chris Bongers Date: Tue, 15 Sep 2026 15:15:17 +0200 Subject: [PATCH 3/3] refactor(feed): match the hero rail against the lead post feedHero now grades its cards across editorial highlights and lifecycle states while the rail stays major headlines, so the two are separate lists and highlights[0] is no longer guaranteed to be the lead card's story. The stacked layout dropped its first headline on position. It now drops the one whose post is actually on the card, and keeps the rest. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/feeds/hero/FeedHero.spec.tsx | 44 +++++++++++++++++++ .../src/components/feeds/hero/FeedHero.tsx | 21 ++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx b/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx index 836f549392..f64d6d0449 100644 --- a/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx +++ b/packages/shared/src/components/feeds/hero/FeedHero.spec.tsx @@ -92,6 +92,50 @@ describe('FeedHero', () => { expect(screen.getByText('Third headline')).toBeInTheDocument(); }); + it('should drop only the lead story from a stacked rail', async () => { + jest.mocked(useFeedHeroAd).mockReturnValue({ + ad: undefined, + placement: 'none', + // Stacked: the lead story is already a card above the list. + shape: feedHeroShape(1), + }); + mockHero({ posts, highlights }); + + renderComponent(); + + await waitFor(() => + expect( + screen.getByRole('heading', { name: 'First hero post' }), + ).toBeInTheDocument(), + ); + // Matched on the post behind it, not on its position in the list. + expect(screen.queryByText('First headline')).not.toBeInTheDocument(); + expect(screen.getByText('Second headline')).toBeInTheDocument(); + expect(screen.getByText('Third headline')).toBeInTheDocument(); + }); + + it('should keep a rail headline whose post is not the lead card', async () => { + jest.mocked(useFeedHeroAd).mockReturnValue({ + ad: undefined, + placement: 'none', + shape: feedHeroShape(1), + }); + // The cards and the headlines are separate lists, so the lead card need + // not be the first headline. + mockHero({ posts: [posts[1]], highlights }); + + renderComponent(); + + await waitFor(() => + expect( + screen.getByRole('heading', { name: 'Second hero post' }), + ).toBeInTheDocument(), + ); + expect(screen.getByText('First headline')).toBeInTheDocument(); + expect(screen.queryByText('Second headline')).not.toBeInTheDocument(); + expect(screen.getByText('Third headline')).toBeInTheDocument(); + }); + it('should render nothing when the query returns no posts', async () => { mockHero({ posts: [], highlights: [] }); diff --git a/packages/shared/src/components/feeds/hero/FeedHero.tsx b/packages/shared/src/components/feeds/hero/FeedHero.tsx index 3d6a38864a..48c7de3e88 100644 --- a/packages/shared/src/components/feeds/hero/FeedHero.tsx +++ b/packages/shared/src/components/feeds/hero/FeedHero.tsx @@ -33,9 +33,9 @@ import { useFeedHeroAd } from './useFeedHeroAd'; const HERO_ORIGIN = 'feed hero'; /** - * The carousel and the Happening Now list are the same headlines: `feedHero` - * returns the leading few already hydrated into posts for the cards, and every - * headline it kept for the rows beside them. + * The carousel and the Happening Now list are two lists, not one: `feedHero` + * grades the cards across editorial highlights and lifecycle states, while the + * rows beside them stay major headlines. A post can be in both, or in one. */ export const FeedHero = ({ feedName, @@ -76,7 +76,6 @@ export const FeedHero = ({ }); const highlights = useMemo(() => hero?.feedHero?.highlights ?? [], [hero]); - /** In headline order and index-aligned with the highlights, per `feedHero`. */ const posts: Post[] = useMemo(() => hero?.feedHero?.posts ?? [], [hero]); const isRendered = posts.length > 0; @@ -84,9 +83,17 @@ export const FeedHero = ({ const isAdShown = adPlacement !== 'none'; // Stacked, the lead story is already a card above the list, so drop it from - // the list rather than showing it twice a few pixels apart. - const railHighlights = - shape.layout === 'stacked' ? highlights.slice(1) : highlights; + // the list rather than showing it twice a few pixels apart. Matched on the + // post, not the position: the cards and the headlines are separate lists. + const railHighlights = useMemo(() => { + const leadPostId = posts[0]?.id; + + if (shape.layout !== 'stacked' || !leadPostId) { + return highlights; + } + + return highlights.filter(({ post }) => post.id !== leadPostId); + }, [highlights, posts, shape.layout]); const onAdAction = useCallback( (action: AdActions, extra?: Record) => {