Skip to content

feat: add Feed page and Item component (migration step 3)#246

Draft
devin-ai-integration[bot] wants to merge 2 commits into
react-migration-scaffoldfrom
react-migration-feed
Draft

feat: add Feed page and Item component (migration step 3)#246
devin-ai-integration[bot] wants to merge 2 commits into
react-migration-scaffoldfrom
react-migration-feed

Conversation

@devin-ai-integration
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot commented May 14, 2026

Summary

Implements Step 3 of the Angular-to-React migration: the Feed page and Item component.

src/pages/Feed.tsx

Ported from src/app/feeds/feed/feed.component.ts + feed.component.html:

  • Uses useParams() from react-router-dom to extract page param; receives feedType as a prop
  • Uses useReducer for state management (loading → loaded/error), avoiding synchronous setState in effects
  • Calls fetchFeed() from src/api/hackerNewsApi.ts whenever feedType or pageNum changes
  • Renders a list of <Item> components inside an ordered list with correct start numbering
  • Pagination links (Prev / More) via react-router <Link>
  • Shows <Loader /> while loading, <ErrorMessage /> on fetch failure
  • Jobs feed header text preserved from Angular original

src/components/Item.tsx

Ported from src/app/feeds/item/item.component.ts + item.component.html:

  • Accepts item: Story and index as props
  • Uses useSettings() to apply titleFontSize, listSpacing, and openLinkInNewTab
  • Uses formatCommentCount() from src/utils/formatCommentCount.ts
  • External URLs open via <a href>, internal stories link to /item/:id via <Link>
  • User profile links to /user/:user
  • Responsive layout: subtext-palm (mobile) and subtext-laptop (desktop)

Styles

SCSS files ported from the Angular originals, updated to use @use instead of @import for Sass module system compatibility.

Review & Testing Checklist for Human

  • Verify Feed page renders correctly for each feed type (news, newest, show, ask, jobs) once routing is wired up
  • Verify pagination (Prev / More links) navigates correctly and list numbering updates
  • Verify Item component respects Settings context (font size, spacing, open in new tab)
  • Verify responsive layout — subtext-palm visible on mobile, subtext-laptop on desktop
  • Check that the jobs feed shows the Y Combinator header text

Notes

  • These components are not yet wired into the app's router (App.tsx still shows the scaffold placeholder). Routing will be added in a subsequent migration step.
  • Build (tsc -b && vite build) and lint (eslint .) both pass cleanly.

Link to Devin session: https://app.devin.ai/sessions/d7454e226af841039b9d379447226356
Requested by: @eashansinha


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Open in Devin Review (Staging)

…ep 3)

- Create src/pages/Feed.tsx: ported from feed.component.ts/.html
  - Uses useParams for feedType/page, useReducer for state management
  - Renders Item list with pagination (Prev/More) via react-router Link
  - Shows Loader while loading, ErrorMessage on error
  - Jobs feed header support

- Create src/components/Item.tsx: ported from item.component.ts/.html
  - Accepts Story prop, uses useSettings for font size, spacing, openLinkInNewTab
  - Uses formatCommentCount utility for comment display
  - Links to /item/:id for comments and /user/:user for profiles
  - Responsive layout with subtext-palm (mobile) and subtext-laptop (desktop)

- Add corresponding SCSS files ported from Angular originals

Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
@devin-ai-integration
Copy link
Copy Markdown
Author

Original prompt from Eashan

Repository: COG-GTM/angular2-hn
Branch: react-migration-feed (branched from react-migration-scaffold)

You are performing Step 3 of an Angular-to-React migration. The scaffold branch react-migration-scaffold already has Vite + React + TypeScript set up with types, API layer, SettingsContext, Loader, and ErrorMessage components.

Complete ALL of the following steps:

  1. Create your branch from react-migration-scaffold:

    git fetch origin
    git checkout -b react-migration-feed origin/react-migration-scaffold
    
  2. Create src/pages/Feed.tsx — port from src/app/feeds/feed/feed.component.ts + feed.component.html. Use useParams() from react-router-dom to get feedType and page. Use useEffect to call fetchFeed() from src/api/hackerNewsApi.ts whenever params change. Render a list of &lt;Item&gt; components. Add pagination links (Prev/More) using &lt;Link&gt;. Show a &lt;Loader /&gt; while loading and &lt;ErrorMessage /&gt; on error.

  3. Create src/components/Item.tsx — port from src/app/feeds/item/item.component.ts + item.component.html. Accept item: Story as a prop (plus index for numbering). Use useSettings() for font size, spacing, and openLinkInNewTab. Use formatCommentCount() from src/utils/formatCommentCount.ts. Link to /item/${id} for comments and /user/${user} for user profile.

  4. Make sure the project builds without errors (npm run build).

  5. Commit, push the branch react-migration-feed, and open a draft PR targeting react-migration-scaffold.

@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread src/pages/Feed.tsx
Comment on lines +43 to +57
useEffect(() => {
dispatch({ type: 'loading' });

fetchFeed(feedType, pageNum)
.then((data) => {
dispatch({ type: 'loaded', items: data });
window.scrollTo(0, 0);
})
.catch(() => {
dispatch({
type: 'error',
message: `Could not load ${feedType} stories.`,
});
});
}, [feedType, pageNum]);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🟡 Race condition: stale fetch response overwrites current data when feedType or page changes rapidly

The useEffect in Feed.tsx:43-57 fires a new fetchFeed call whenever feedType or pageNum changes, but never cancels the previous in-flight request. If a user navigates quickly (e.g., page 1 → page 2 → page 3), multiple fetches run concurrently. If an earlier fetch resolves after a later one, its stale data overwrites the current results via dispatch({ type: 'loaded', items: data }), causing the UI to display data from a page/feed the user has already navigated away from. The fix is to use an AbortController or a stale-closure guard (boolean flag set in the cleanup function) to ignore responses from superseded requests.

Example scenario
  1. User on page 1 clicks "More" → effect fires fetch for page 2
  2. User immediately clicks "More" again → effect fires fetch for page 3, dispatches 'loading'
  3. Page 3 fetch completes first → UI shows page 3 data
  4. Page 2 fetch completes later → UI now incorrectly shows page 2 data while URL says page 3
Suggested change
useEffect(() => {
dispatch({ type: 'loading' });
fetchFeed(feedType, pageNum)
.then((data) => {
dispatch({ type: 'loaded', items: data });
window.scrollTo(0, 0);
})
.catch(() => {
dispatch({
type: 'error',
message: `Could not load ${feedType} stories.`,
});
});
}, [feedType, pageNum]);
useEffect(() => {
let cancelled = false;
dispatch({ type: 'loading' });
fetchFeed(feedType, pageNum)
.then((data) => {
if (!cancelled) {
dispatch({ type: 'loaded', items: data });
window.scrollTo(0, 0);
}
})
.catch(() => {
if (!cancelled) {
dispatch({
type: 'error',
message: `Could not load ${feedType} stories.`,
});
}
});
return () => { cancelled = true; };
}, [feedType, pageNum]);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — added a stale-closure guard (cancelled flag) in the cleanup function to ignore responses from superseded requests. Fixed in 3073209.

Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant