React migration: full app scaffold under react-app/ (Vite + TypeScript) by devanshi-gpta · Pull Request #416 · COG-GTM/angular2-hn

devanshi-gpta · 2026-07-01T15:06:14Z

Summary

Migrates the Hacker News PWA from Angular 9 to plain React (Vite + TypeScript) in a new top-level react-app/ directory. The existing Angular src/ and Angular config are left fully untouched — cutover/removal of Angular is explicitly out of scope for this pass. No tests were ported.

Data fetching is manual fetch, settings state uses React Context, routing uses react-router-dom, and PWA/Firebase deploy are preserved.

Layout (`react-app/src/`)

main.tsx            bootstrap + router (replaces main.ts / app.routes.ts)
App.tsx             theme wrapper + <Outlet> (replaces AppComponent)
api/hackerNews.ts   async fetch module (replaces HackerNewsAPIService)
context/SettingsContext.tsx  Context + useSettings() (replaces SettingsService)
models/             copied 1:1 from src/app/shared/models
utils/commentLabel.ts        pure fn (replaces comment.pipe.ts)
components/         Header Footer Settings Loader ErrorMessage
pages/             Feed Item ItemDetails Comment User
styles/            styles.scss + scss/ vars + per-component .scss

Key translation notes (not obvious from the diff)

Data layer — RxJS lazyFetch/cancel-token replaced by async/await + fetch (+ AbortController). Poll handling was reworked so poll data is complete before return:

// was: fire N subscriptions that mutate story AFTER return
const results = await Promise.all(range(N).map(i => fetchPollContent(story.id + i + 1)))
story.poll_votes_count = 0
results.forEach((r, i) => { story.poll[i] = r; story.poll_votes_count += r.points })
return story

Settings — same settings shape, initialized from localStorage; each setter persists as the original did. The prefers-color-scheme matchMedia listener + initTheme moved into a useEffect (add on mount / remove on unmount). Components consume useSettings() instead of constructor injection.
Routing — single parameterized /:feedType/:page route; feedType validated against news|newest|show|ask|jobs (invalid → <Navigate to="/news/1" replace />). / redirects to /news/1. /item/:id and /user/:id are React.lazy + Suspense (mirroring the Angular lazy modules); static segments make them rank above the feed route.
Template rules — *ngIf→&&, *ngFor→.map(), [routerLink]→<Link>, [innerHTML]→dangerouslySetInnerHTML (comment/poll/user HTML), [ngStyle]→inline style. Comment.tsx stays recursive with local useState collapse.
GA — NavigationEnd pageview tracking moved into a useEffect keyed on useLocation().pathname (guarded by typeof ga === 'function').
Models — copied unchanged (plain TS classes); strictPropertyInitialization: false in tsconfig so the uninitialized-field classes compile.

PWA & deploy

vite-plugin-pwa (Workbox) replaces @angular/service-worker/ngsw-config.json; manifest fields ported from src/manifest.json, icons copied into react-app/public/assets/.
firebase.json hosting public → react-app/dist.
.travis.yml now builds the React app (cd react-app && npm install && npm run build), node bumped to 18. Karma/Jasmine/Protractor toolchain intentionally not ported.

Verification

npm run build, npm run typecheck, npm run lint all pass in react-app/.
Headless-Chrome smoke test against the dev server: feed renders 30 stories, item page renders with recursive comments, invalid feedType redirects to /news/1. (The /user/:id API endpoint upstream returns 404 → ErrorMessage renders, same behavior as the Angular original.)

Out of scope

Deleting/modifying the existing Angular src/ and Angular config; porting tests.

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

Devin Review

Status	Commit
⚪ Not started	—

Run Devin Review

Migrate the Hacker News PWA from Angular 9 to plain React (Vite + TypeScript) into a new top-level react-app/ directory, leaving the existing Angular src/ untouched. Manual fetch data layer, Settings via React Context, react-router, vite-plugin-pwa, and Firebase deploy pointing at react-app/dist. Co-Authored-By: Devanshi Gupta <devanshi.gupta@cognition.ai>

devin-ai-integration · 2026-07-01T15:06:17Z

🤖 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 9 potential issues.

devin-ai-integration · 2026-07-01T15:20:48Z

+async function getJson<T>(url: string, signal?: AbortSignal): Promise<T> {
+  const res = await fetch(url, { signal });
+  return res.json() as Promise<T>;
+}


🔴 API error responses are silently treated as valid data, crashing the app

HTTP error responses (404, 500) from the API are never detected (res.ok is not checked at react-app/src/api/hackerNews.ts:8-9) before parsing the body as JSON, so a server error returning valid JSON is silently used as feed/item data.

Impact: The feed page crashes because .map() is called on a non-array error object, and with no error boundary configured the entire app goes blank.

Mechanism: fetch does not throw on HTTP errors

The fetch API only rejects on network failures, not HTTP error status codes. When the HN API returns a non-2xx response with a JSON body (e.g. {"error": "Not Found"}), res.json() at react-app/src/api/hackerNews.ts:9 successfully parses it. The callers in Feed.tsx:25-26 then call setItems(data) with this error object. During the next render at Feed.tsx:58, items.map(...) throws a TypeError because the error object has no .map method. Since no ErrorBoundary or errorElement is configured (verified by searching the codebase), this crashes the entire React tree.

The same issue affects fetchItemContent (used in ItemDetails.tsx:29-30) and fetchUser (used in User.tsx:22-23), which would render with missing/undefined properties instead of showing the error message.

Suggested change

async function getJson<T>(url: string, signal?: AbortSignal): Promise<T> {

const res = await fetch(url, { signal });

return res.json() as Promise<T>;

}

async function getJson<T>(url: string, signal?: AbortSignal): Promise<T> {

const res = await fetch(url, { signal });

if (!res.ok) {

throw new Error(`HTTP ${res.status}: ${res.statusText}`);

}

return res.json() as Promise<T>;

}

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

Fixed in a311905 — added the res.ok check in getJson, so HTTP error responses now reject and route to the components' .catch → ErrorMessage instead of being parsed as feed/item data.

devin-ai-integration · 2026-07-01T15:20:50Z

+                    style={{
+                      width:
+                        (pollResult.points / item.poll_votes_count) * 100 + '%',
+                    }}


🟡 Poll results bar width becomes invalid when all poll options have zero votes

The poll bar width is calculated by dividing by the total vote count (item.poll_votes_count at react-app/src/pages/ItemDetails.tsx:124), which is zero when every option has zero points, so the width is set to NaN%.

Impact: Poll result bars render with an invalid CSS width and may display incorrectly or not at all.

Mechanism: 0 / 0 = NaN in poll width calculation

In react-app/src/api/hackerNews.ts:41, poll_votes_count is initialized to 0 and accumulated from pollResult.points at line 44. If all poll options have 0 points, poll_votes_count remains 0.

Then in ItemDetails.tsx:124:

width: (pollResult.points / item.poll_votes_count) * 100 + '%'

This computes 0 / 0 = NaN, producing "NaN%" as the CSS width value, which the browser ignores.

Suggested change

style={{

width:

(pollResult.points / item.poll_votes_count) * 100 + '%',

}}

style={{

width:

(item.poll_votes_count > 0 ? (pollResult.points / item.poll_votes_count) * 100 : 0) + '%',

}}

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

Fixed in a311905 — guarded the divisor so width is 0% when poll_votes_count === 0 instead of NaN%.

devin-ai-integration · 2026-07-01T15:20:52Z

+  return {
+    showSettings: false,
+    openLinkInNewTab: openLinkInNewTab ? JSON.parse(openLinkInNewTab) : false,
+    theme: 'default',


🚩 Theme saved in localStorage is not applied during initial render, causing a flash

In react-app/src/context/SettingsContext.tsx:30, getInitialSettings() hardcodes theme: 'default' instead of reading from localStorage. The saved theme is only restored later in the useEffect at lines 75-77, causing a visual flash of the default (light) theme before the saved theme kicks in. By contrast, the other settings (titleFontSize, listSpacing, openLinkInNewTab) ARE read from localStorage during initialization at lines 29-32. The theme could follow the same pattern to avoid the flash.

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

Fixed in a311905 — getInitialSettings() now reads theme from localStorage (localStorage.getItem('theme') ?? 'default'), matching the other settings and avoiding the light-theme flash before the useEffect runs.

devin-ai-integration · 2026-07-01T15:20:53Z

+    const handleChange = (event: MediaQueryListEvent) => {
+      setTheme(event.matches ? 'night' : 'default');
+    };


🚩 System dark-mode listener overrides user-selected theme on every OS toggle

The handleChange listener at react-app/src/context/SettingsContext.tsx:71-73 calls setTheme() whenever the OS color scheme changes, which also saves to localStorage (line 52). This means if a user explicitly chose the 'default' (light) theme but their OS switches to dark mode, the app will override their choice to 'night' and persist it. A flag distinguishing user-chosen vs auto-detected themes would be needed to preserve user intent.

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

This is intentional parity with the original Angular SettingsService, whose subscribeToSystemPreferredColorScheme added a change listener that unconditionally called setTheme(...) on every OS color-scheme change (and persisted it). Changing it to preserve a user-chosen theme would be a behavior change beyond this migration's scope. Happy to add a "user-chosen vs auto" flag as a follow-up if desired.

devin-ai-integration · 2026-07-01T15:20:54Z

+  $theme-amoledblack-subtext-color,
+  $theme-amoledblack-secondary-color,
+  $theme-amoledblack-body-background-color,
+  $theme-amoledblack-secondary-color


📝 Info: AMOLED Black theme passes a color value where a border shorthand is expected

At react-app/src/styles/scss/_themes.scss:238, the amoledblack theme passes $theme-amoledblack-secondary-color (rgba(255, 255, 255, 0.6)) as the $border parameter. The other themes pass full border shorthand values like 2px solid #b92b27 (see _theme_variables.scss:18,31). This means border-bottom: rgba(255,255,255,0.6) is generated, which lacks width and style — so borders are invisible in the AMOLED Black theme. This is a pre-existing issue carried over from the original Angular app's identical SCSS files (src/app/shared/scss/_themes.scss).

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

Correct, and pre-existing — the SCSS was copied 1:1 from src/app/shared/scss/_themes.scss per the migration spec (styles copied unchanged). Leaving as-is to preserve visual parity; can be fixed separately if we want AMOLED borders.

devin-ai-integration · 2026-07-01T15:20:55Z

+    points: number;
+    user: string;
+    time: number;
+    time_ago: number;


📝 Info: Story model declares time_ago as number but API returns a string

In react-app/src/models/story.ts:11, time_ago is typed as number, but the node-hnapi returns it as a human-readable string like '2 hours ago'. This doesn't cause a runtime error since TypeScript types are erased and the actual JSON value is a string that renders correctly in JSX (Item.tsx:51, ItemDetails.tsx:99). However, the incorrect type means any code relying on the declared type (e.g., arithmetic or comparisons) would produce wrong results.

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

Pre-existing — the model was copied unchanged from src/app/shared/models/story.ts per the spec (models ported 1:1). The value is only ever rendered as a string in JSX, so no runtime impact; correcting the type to string would be a fine cleanup outside this migration's scope.

devin-ai-integration · 2026-07-01T15:20:56Z

+  <StrictMode>
+    <SettingsProvider>
+      <RouterProvider router={router} />
+    </SettingsProvider>
+  </StrictMode>


📝 Info: SettingsProvider is outside RouterProvider but this is intentionally correct

In react-app/src/main.tsx:49-53, SettingsProvider wraps RouterProvider. This might look like a potential issue since route components use useSettings(), but React Context propagates through the React component tree regardless of router boundaries. All route-rendered components (App, Feed, ItemDetails, etc.) are descendants in the React tree and correctly receive the settings context. This ordering is correct and standard practice.

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

Agreed — thanks for confirming. Intentional: settings context must wrap the router so all route-rendered components can consume useSettings().

devin-ai-integration · 2026-07-01T15:20:57Z

+import { User } from '../models/user';
+import { PollResult } from '../models/poll-result';
+
+const baseUrl = 'https://node-hnapi.herokuapp.com';


🚩 Base API URL points to Heroku free-tier service that has been discontinued

The API base URL at react-app/src/api/hackerNews.ts:5 is https://node-hnapi.herokuapp.com. Heroku discontinued its free tier in November 2022, so this endpoint is likely no longer available. The app would fail to load any data. Alternative HN API endpoints exist (e.g., the official Firebase HN API at hacker-news.firebaseio.com).

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

Kept intentionally — the migration spec requires the base URL stay https://node-hnapi.herokuapp.com. It's also still live: my smoke test against it returned 30 stories and full item/comment data. Switching APIs would be a separate change.

devin-ai-integration · 2026-07-01T15:20:59Z

+          <p
+            className="comment-text"
+            dangerouslySetInnerHTML={{ __html: comment.content }}
+          ></p>


🟨 User-generated HTML rendered without sanitization via dangerouslySetInnerHTML

Multiple components render API-provided HTML content directly using dangerouslySetInnerHTML without any sanitization. This includes comment content (react-app/src/pages/Comment.tsx:33), item content (react-app/src/pages/ItemDetails.tsx:117,133), and user about sections (react-app/src/pages/User.tsx:56). While the HN API may sanitize content server-side, the client has no defense-in-depth against XSS if the API returns malicious HTML.

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

Intentional per the migration spec, which maps Angular's [innerHTML] to dangerouslySetInnerHTML for comment/poll/user HTML. Note Angular's [innerHTML] applies DomSanitizer by default, so React loses that implicit sanitization — a valid defense-in-depth point. I've left it 1:1 for this pass; adding DOMPurify around these sinks would be a small, safe follow-up. Flagging to the maintainer to decide.

…heme from localStorage Co-Authored-By: Devanshi Gupta <devanshi.gupta@cognition.ai>

devin-ai-integration Bot reviewed Jul 1, 2026

View reviewed changes

Address review: check res.ok, guard poll width divide-by-zero, init t…

a311905

…heme from localStorage Co-Authored-By: Devanshi Gupta <devanshi.gupta@cognition.ai>

Uh oh!

Conversation

devanshi-gpta commented Jul 1, 2026 • edited by staging-devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Layout (react-app/src/)

Key translation notes (not obvious from the diff)

PWA & deploy

Verification

Out of scope

Devin Review

Uh oh!

devin-ai-integration Bot commented Jul 1, 2026

🤖 Devin AI Engineer

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

devanshi-gpta commented Jul 1, 2026 •

edited by staging-devin-ai-integration Bot

Loading

Layout (`react-app/src/`)