Migrate from Angular 9 to React 18 + TypeScript + Vite

[devanshi-gpta]

Summary

Full migration of the Hacker News PWA from Angular 9 (with Webpack, RxJS, zone.js) to React 18 + TypeScript + Vite + react-router-dom v6.

Toolchain replacement:

• angular.json / Webpack / @angular/cli → vite.config.ts with @vitejs/plugin-react
• @angular/service-worker / ngsw-config.json → vite-plugin-pwa (Workbox generateSW, NetworkFirst runtime caching for the HN API, navigateFallback for app-shell)
• tslint.json / codelyzer → ESLint with @typescript-eslint + react-hooks + react-refresh plugins
• karma / protractor / jasmine → removed (no test framework added — matching original repo which had no passing tests)

Architecture:

• Angular HackerNewsAPIService (RxJS observables, lazyFetch with cancel tokens) → src/api/hackerNewsApi.ts exporting fetchFeed, fetchItemContent, fetchPollContent, fetchUser as async functions using fetch + AbortController. Poll logic uses Promise.all instead of nested subscriptions.
• Angular SettingsService (injectable singleton) → SettingsContext / SettingsProvider with useSettings() hook. Preserves localStorage persistence and prefers-color-scheme dark-mode listener.
• Angular CommentPipe → commentLabel() utility in src/utils/comment.ts
• Models (Story, User, Comment, Settings, PollResult, FeedType) kept as plain TS interfaces

Routing (react-router-dom v6 nested routes):

<Route path="/" element={<App />}>        // theme wrapper + Header/Outlet/Footer
  <Route index → Navigate to="/news/1" />
  <Route path=":feedType/:page" → <Feed /> />   // news, newest, show, ask, jobs
  <Route path="item/:id" → lazy(<ItemDetails />) />
  <Route path="user/:id" → lazy(<User />) />
</Route>

Component porting (Angular → React function components):

• AppComponent → App: theme className on root div, GA pageview tracking via useLocation effect
• FeedComponent → Feed: useEffect fetch with AbortController cleanup, prev/more pagination via <Link>
• ItemComponent → Item: consumes useSettings() for font size, spacing, open-in-new-tab
• ItemDetailsComponent → ItemDetails: poll results with dangerouslySetInnerHTML, recursive Comment tree
• CommentComponent → Comment: collapse toggle via useState, recursive rendering, deleted-comment handling
• UserComponent → User: profile with dangerouslySetInnerHTML for about
• HeaderComponent → Header: NavLinks with scroll-to-top, settings cog toggle
• SettingsComponent → Settings: theme radios, font/spacing inputs, open-links checkbox — all wired to context

SCSS: All existing SCSS files (themes, per-component styles) preserved and imported; theme class-name approach unchanged.

PWA: index.html keeps all meta tags, manifest link, GA script, noscript fallback; root mount changed from <app-root> to <div id="root">.

Removed: angular.json, karma.conf.js, tslint.json, ngsw-config.json, tsconfig.app.json, tsconfig.spec.json, e2e/, src/app/, src/main.ts, src/polyfills.ts, src/test.ts, src/environments/, yarn.lock.

Updated: README build instructions reflect Vite commands; .travis.yml drops @angular/cli install, bumps to Node 20.

Link to Devin session: https://app.devin.ai/sessions/d7689858b25b4d4f9a42e953b3156cbf
Requested by: @devanshi-gpta

---

Devin Review

Status	Commit
⚪ Not started	—

Run Devin Review

[devin-ai-integration]

🤖 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, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 8 potential issues.

[devin-ai-integration]

🟡 Theme radio buttons produce React warnings and may not visually update on click

The theme radio buttons are set as controlled inputs with checked but use onClick instead of onChange (onClick={() => setTheme('default')} at src/components/Settings/Settings.tsx:36), so React logs a console warning about a missing onChange handler on every render of the settings panel.

Impact: Console warnings clutter developer tools, and the radio button's visual checked state may briefly lag behind the actual state.

Controlled radio inputs require onChange in React

In React, when you provide a checked prop to a radio input, it becomes a "controlled component." React expects a corresponding onChange handler to manage state changes. Without it, React logs: "You provided a checked prop to a form field without an onChange handler."

The three radio buttons at src/components/Settings/Settings.tsx:35-36, src/components/Settings/Settings.tsx:48-49, and src/components/Settings/Settings.tsx:59-60 all use checked={...} with onClick instead of onChange. While onClick does fire and update state, the idiomatic React pattern is onChange.

The fix is to replace onClick with onChange on all three radio inputs:

onChange={() => setTheme('default')}

Prompt for agents

In src/components/Settings/Settings.tsx, three radio button inputs (at approximately lines 35-36, 48-49, and 59-60) use the checked prop (making them controlled) with onClick handlers instead of onChange handlers. React expects controlled form inputs to use onChange. Replace onClick with onChange on all three radio inputs to eliminate the React console warning. For example, change onClick={() => setTheme('default')} to onChange={() => setTheme('default')} on each of the three theme radio buttons.

---

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

[devin-ai-integration]

Fixed in de4b3ae — replaced onClick with onChange on all three theme radio inputs so the controlled inputs no longer trigger React's missing-onChange warning.

[devin-ai-integration]

🔍 Dead-code condition carried over from Angular: feedType is never 'new'

At src/components/Feed/Feed.tsx:58, the condition feedType !== 'new' is always true because the feed types passed via routes are 'news', 'newest', 'show', 'ask', and 'jobs' — none of which equal 'new'. This was the same in the old Angular template (src/app/feeds/feed/feed.component.html:10), where the route data used feedType: 'newest'. The <ol> is therefore always rendered. This is pre-existing dead code that was faithfully migrated, but it's confusing and could be cleaned up or the intent clarified.

---

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

[devin-ai-integration]

Acknowledged — this is a faithful 1:1 port of the original Angular template (feedType !== 'new', where the route feed types are news/newest/show/ask/jobs), so the <ol> always renders. Leaving it as-is to keep the migration behavior-preserving; happy to clean up the dead condition in a follow-up if desired.

[devin-ai-integration]

📝 Info: System dark mode changes always override user's manually selected theme

At src/context/SettingsContext.tsx:68-69, the handleChange listener for prefers-color-scheme always calls setTheme(event.matches ? 'night' : 'default'), which overwrites both the in-memory state and localStorage. If a user manually selects the 'amoledblack' theme and then their OS dark mode toggles, their choice is lost. The old Angular SettingsService had the same behavior (handleSystemPreferredColorSchemeChange called this.setTheme(theme)), so this is pre-existing. A fix would be to only auto-switch if the user hasn't manually chosen a theme (e.g., by tracking a userOverride flag).

---

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

[devin-ai-integration]

Acknowledged — as noted, this mirrors the original Angular SettingsService.handleSystemPreferredColorSchemeChange, so it's pre-existing behavior preserved by the migration. Adding a userOverride flag would be a behavior change beyond the migration scope; leaving as-is unless the maintainers want the enhancement.

[devin-ai-integration]

🔍 Potential duplicate manifest links: static manifest.json and VitePWA-generated manifest

The index.html:35 has <link rel="manifest" href="/manifest.json"> pointing to the static file in public/manifest.json. Meanwhile, vite-plugin-pwa (configured in vite.config.ts:9-63) with a manifest option will generate its own manifest.webmanifest and inject a second <link rel="manifest"> tag into the built HTML. The browser will use the first manifest link it encounters. The two manifests currently have similar content, but maintaining both is fragile — changes to the VitePWA config won't take effect if the static manifest link comes first. Consider removing either the static public/manifest.json + the HTML link, or the manifest option from VitePWA config.

---

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

[devin-ai-integration]

Fixed in de4b3ae. Confirmed the built dist/index.html previously emitted two <link rel="manifest"> tags (static /manifest.json + vite-plugin-pwa's /manifest.webmanifest). Set manifest: false in the VitePWA config so the plugin no longer generates/injects its own manifest; the static public/manifest.json (kept unchanged for Firebase Hosting) is now the single source of truth. The built HTML now contains exactly one manifest link.

[devin-ai-integration]

📝 Info: HTML meta descriptions still reference Angular CLI, RxJS, and Webpack

The index.html meta tags at lines 7, 12, and 20 still describe the app as "A Hacker News client built with Angular CLI, RxJS and Webpack". Since the project has been migrated to React + Vite, these descriptions are now inaccurate. The Twitter and OpenGraph metadata will show outdated technology references when the page is shared on social media.

---

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

[devin-ai-integration]

Fixed in de4b3ae — updated the description, twitter:description, and og:description meta tags in index.html to "A Hacker News client built with React, TypeScript and Vite".

[devin-ai-integration]

🟥 Unsanitized HTML rendering loses Angular's built-in XSS protection in comment content

The migration from Angular to React replaces Angular's [innerHTML] (which auto-sanitizes dangerous HTML) with React's dangerouslySetInnerHTML (src/components/Comment/Comment.tsx:36), which performs NO sanitization. Comment content from the third-party HN API (node-hnapi.herokuapp.com) is rendered as raw HTML. If the API returns or is manipulated to return malicious HTML (e.g., <img onerror="...">, <svg onload="...">), it will execute in the user's browser.

---

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

[devin-ai-integration]

Valid observation. Angular's [innerHTML] auto-sanitizes via DomSanitizer, whereas React's dangerouslySetInnerHTML does not — so this is a real behavioral difference. However, the migration spec explicitly mapped [innerHTML] → dangerouslySetInnerHTML, and the content comes from the HN API (node-hnapi). Rather than unilaterally add a sanitizer (new dependency + deviation from the requested mapping), I'm flagging this to the requester to decide whether to add DOMPurify here (and in ItemDetails/User). Not changing in this PR pending that decision.

[devin-ai-integration]

🟥 Unsanitized HTML rendering of item and poll content enables XSS

The ItemDetails component renders both item.content and pollResult.content from the third-party API using dangerouslySetInnerHTML (src/components/ItemDetails/ItemDetails.tsx:122 and src/components/ItemDetails/ItemDetails.tsx:132) without any HTML sanitization, allowing potential script injection if the API returns malicious markup.

---

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

[devin-ai-integration]

Same class of issue as the comment content in Comment.tsx — Angular's [innerHTML] sanitized, React's dangerouslySetInnerHTML does not. This was the spec's explicit mapping and the source is the HN API. Flagging to the requester to decide on adding DOMPurify across all three render sites (Comment, ItemDetails item/poll content, User about) as a single consistent change rather than deviating from the requested migration here.

[devin-ai-integration]

🟥 Unsanitized HTML rendering of user profile 'about' field enables XSS

The User component renders user.about from the third-party API using dangerouslySetInnerHTML (src/components/User/User.tsx:63) without sanitization. User profile 'about' fields on Hacker News can contain HTML, and without sanitization, any malicious content would execute in the viewer's browser.

---

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

[devin-ai-integration]

Same as the other two dangerouslySetInnerHTML sites. Deferring to the requester on whether to introduce DOMPurify for the about field; if approved I'll sanitize all three render sites consistently in one change.

[devin-ai-integration]

End-to-end test results

Ran the migrated app locally (Vite dev server) against the live node-hnapi and exercised the primary flows in the browser; also verified the PWA-manifest and meta-description fixes in the production build.

Escalations first:

• ⚠️ User profile happy-path untested — upstream node-hnapi.herokuapp.com/user/:id returns 404. This is the same endpoint the original Angular app used (confirmed against master), so it's a pre-existing upstream limitation, not a migration regression. The route + lazy User component + error handling all work (ErrorMessage rendered gracefully). Feeds/items on the same API work fine.
• 🟥 XSS via dangerouslySetInnerHTML — awaiting your decision on adding DOMPurify (see review threads).

Results:

• ✅ / → /news/1, numbered feed with real HN data
• ✅ Feed pagination: show/1 → "More ›" → show/2 starting at feat: migrate CommentComponent from Angular to React #31; no "Prev" on page 1
• ✅ Jobs feed: YC header text, no points/comments
• ✅ Item details + nested threaded comments
• ✅ Comment collapse/expand ([-] → [+] hides subtree; re-expands)
• ✅ Settings theme switch (the onClick→onChange fix): "Night" applies immediately, sets root className="night" + localStorage.theme="night", persists across reload
• ✅ PWA manifest fix: built dist/index.html now has exactly 1 <link rel="manifest"> (was 2)
• ✅ Meta descriptions: all 3 now reference React/TypeScript/Vite in the build
• ⚠️ User profile: untested (upstream 404)

Theme switching — onChange fix (before → after)

Default	Night (applied immediately, persists on reload)

Feed, pagination, item details, comment collapse

News feed (/ → /news/1)	Jobs feed (YC header)

Item details + threaded comments	Comment collapsed ([+])

Tested via this Devin session.