Skip to content

feat: Migrate Angular2-HN to React (Phases 1-3)#154

Open
devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
devin/1769629824-react-migration
Open

feat: Migrate Angular2-HN to React (Phases 1-3)#154
devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
devin/1769629824-react-migration

Conversation

@devin-ai-integration
Copy link

@devin-ai-integration devin-ai-integration bot commented Jan 28, 2026

Migrate Angular2-HN to React (Phases 1-3)

Summary

This PR migrates the Angular 9 Hacker News PWA client to React with TypeScript. The migration covers Phases 1-3: Setup & Foundation, Core Components, and Feed Components. The React app is created in a new react-app/ directory alongside the existing Angular code.

Key changes:

  • New React app with Vite + TypeScript build tooling
  • Ported all TypeScript data models (Story, User, Comment, Settings, etc.)
  • Converted HackerNewsAPIService from RxJS Observables to Promise-based axios calls with custom React hooks (useFeed, useItem, useUser)
  • Implemented Settings system with React Context and localStorage persistence
  • Three themes working: Default (red), Night (dark blue), Black (AMOLED)
  • Feed components with pagination and scroll restoration
  • Routes: /news/:page, /newest/:page, /show/:page, /ask/:page, /jobs/:page

Not yet implemented (Phase 4):

  • /item/:id - Item details page (placeholder only)
  • /user/:id - User profile page (placeholder only)

Review & Testing Checklist for Human

  • Verify feed data accuracy: Check that story titles, points, authors, timestamps, and comment counts display correctly and match the original Angular app
  • Test all three themes: Open Settings (cog icon) and switch between Default, Night, and Black (AMOLED) themes - verify colors apply correctly
  • Test settings persistence: Change settings (theme, font size, list spacing, open links in new tab), refresh the page, and verify settings are retained
  • Test pagination: Navigate through pages using "More >" and "< Prev" links on different feeds
  • Test responsive design: Verify the app works on mobile viewport sizes (header should become fixed, layout should adapt)
  • Verify navigation: Click through all nav links (new, show, ask, jobs) and confirm correct feeds load

Recommended test plan:

  1. Run cd react-app && npm install && npm run dev
  2. Open http://localhost:3000 in browser
  3. Verify news feed loads with stories
  4. Test Settings modal and theme switching
  5. Navigate between different feeds
  6. Test pagination on a feed
  7. Resize browser to mobile width to test responsive design

Notes

  • The original Angular app remains untouched - this is an additive migration
  • CSS was manually converted from SCSS to plain CSS with CSS custom properties for theming
  • No automated tests are included in this phase
  • The API endpoint https://node-hnapi.herokuapp.com is the same as the original app

Default Theme


Link to Devin run: https://app.devin.ai/sessions/5aa9183e8ce24b039120d16d666b0cf3
Requested by: Eashan Sinha (@eashansinha)


Open with Devin

Phase 1: Setup & Foundation
- Initialize React app with Vite + TypeScript
- Set up folder structure: components/, services/, models/, hooks/, context/
- Install core dependencies: react-router-dom, axios

Phase 1.2: Port Data Models
- Migrate TypeScript interfaces from Angular: Story, User, Comment, Settings, FeedType, PollResult

Phase 1.3: Port API Service
- Convert HackerNewsAPIService to Promise-based service
- Create custom hooks: useFeed, useItem, useUser
- API endpoint: https://node-hnapi.herokuapp.com

Phase 2: Core Components
- App.tsx with React Router and Google Analytics tracking
- Header component with navigation links and settings toggle
- Footer component
- Settings component with theme selection, font size, list spacing

Phase 2.2: Settings & Theme System
- SettingsContext with localStorage persistence
- Dark mode detection via prefers-color-scheme
- Three themes: Default, Night, Black (AMOLED)

Phase 3: Feed Components
- Feed component with pagination and scroll restoration
- Item component with responsive design
- Loader and ErrorMessage components

Routes implemented:
- /news/:page, /newest/:page, /show/:page, /ask/:page, /jobs/:page
- /item/:id and /user/:id (placeholders for Phase 4)

Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
@devin-ai-integration
Copy link
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
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 issue and 4 additional flags in Devin Review.

Open in Devin Review

Comment on lines +37 to +39
const darkColorSchemeMedia = typeof window !== 'undefined'
? window.matchMedia('(prefers-color-scheme: dark)')
: null;
Copy link
Author

Choose a reason for hiding this comment

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

🔴 MediaQueryList object recreated on every render causes useEffect to run infinitely

The darkColorSchemeMedia variable is created on every render of SettingsProvider because window.matchMedia() returns a new object reference each time it's called.

Click to expand

Problem

At line 37-39, darkColorSchemeMedia is created during render:

const darkColorSchemeMedia = typeof window !== 'undefined' 
  ? window.matchMedia('(prefers-color-scheme: dark)') 
  : null;

This variable is then used as a dependency in the useEffect at line 66:

}, [darkColorSchemeMedia, handleSystemPreferredColorSchemeChange]);

Since window.matchMedia() returns a new MediaQueryList object on each call, the reference changes on every render. This causes:

  1. The useEffect to run on every render (not just on mount)
  2. Event listeners to be constantly added and removed
  3. Potential performance degradation and unnecessary re-renders

Expected Behavior

The MediaQueryList should be created once and stored using useRef or useMemo so the effect only runs when necessary.

Impact

This causes unnecessary effect executions on every component re-render, leading to performance issues and potential memory leaks from rapid listener attachment/detachment.

Recommendation: Use useMemo or useRef to create the MediaQueryList once:

const darkColorSchemeMedia = useMemo(() => 
  typeof window !== 'undefined' 
    ? window.matchMedia('(prefers-color-scheme: dark)') 
    : null,
  []
);
Open in Devin Review

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

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.

0 participants