Phase 2: Services, Settings Context & Shared Utilities#310
Phase 2: Services, Settings Context & Shared Utilities#310vibhaseshadri-cognition wants to merge 3 commits into
Conversation
- Replace Angular build config with Vite + React 18 setup - Create vite.config.ts, tsconfig.json, package.json with React deps - Add entry points: src/main.tsx, src/App.tsx, index.html - Port SCSS theming files (_themes.scss, _theme_variables.scss, _media.scss) - Define TypeScript interfaces in src/types/ (Story, Comment, User, Settings, PollResult, FeedType) - Move static assets to public/ directory for Vite serving - Remove Angular-specific files: angular.json, tsconfig.app/spec.json, tslint.json, karma.conf.js, ngsw-config.json, browserslist, polyfills.ts, test.ts, all *.module.ts files - Remove Angular dependencies, add react, react-dom, react-router-dom v6 - Verify npm run dev starts and app shell renders Co-Authored-By: Vibha Seshadri <vibha.seshadri@cognition.ai>
- Add src/services/hackernews-api.ts: fetchFeed, fetchItemContent (with poll sub-fetching via Promise.all), fetchPollContent, fetchUser using plain fetch - Add src/contexts/SettingsContext.tsx: React Context + useReducer for Settings state, localStorage persistence, system dark mode detection with matchMedia listener - Add src/components/shared/Loader.tsx + SCSS: CSS loading animation ported from Angular - Add src/components/shared/ErrorMessage.tsx + SCSS: skull error display ported from Angular - Add src/utils/formatComment.ts: replaces Angular CommentPipe (0→'discuss', 1→'1 comment', n→'n comments') - Update tsconfig.json include paths for new directories Co-Authored-By: Vibha Seshadri <vibha.seshadri@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| <div id="root"></div> | ||
|
|
||
| <div class="app-loader" id="content"> | ||
| <img class="logo" src="/assets/images/logo.svg" alt="Logo"> | ||
| <noscript> | ||
| <h4 style="font-family: 'HelveticaNeue-Light', 'Helvetica Neue Light', 'Helvetica Neue', Helvetica;"> | ||
| Sorry, JavaScript needs to be enabled in order to run this application. | ||
| </h4> | ||
| </noscript> | ||
| </div> |
There was a problem hiding this comment.
🚩 Loading spinner CSS selector is broken after migration from to
The existing src/styles.scss:37-40 defines app-root:empty + .app-loader { opacity: 1; z-index: 100; } which showed the loading spinner while Angular bootstrapped. The new index.html:54 uses <div id="root"></div> instead of <app-root></app-root>, so this CSS selector never matches. The .app-loader default style (opacity: 0; z-index: -1 at src/styles.scss:22-23,28) keeps the loader permanently hidden. This means: (1) no loading indicator displays while React initializes, and (2) the <noscript> message at index.html:58-62 is invisible in production builds where the bundled CSS is loaded via a <link> tag even without JS. Consider updating the CSS selector to #root:empty + .app-loader or removing the .app-loader div entirely if the loading state is no longer needed.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch. Fixed in ee71d5d — updated the selector to #root:empty + .app-loader to match the React mount point.
| @@ -0,0 +1,37 @@ | |||
| import { Story, User, PollResult } from '../types'; | |||
|
|
|||
| const BASE_URL = 'https://node-hnapi.herokuapp.com'; | |||
There was a problem hiding this comment.
🚩 API base URL points to likely-defunct Heroku service
The BASE_URL at src/services/hackernews-api.ts:3 is https://node-hnapi.herokuapp.com. Heroku removed free dynos in November 2022, and this endpoint is unlikely to be responding. The same URL was used in the old Angular service (src/app/shared/services/hackernews-api.service.ts:16), so this is pre-existing. Before this service is actually wired into the React app, the URL should be updated to a working alternative (e.g., https://api.hnpwa.com/v0 or direct Firebase HN API).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is intentional — the migration spec requires using the same node-hnapi.herokuapp.com base URL as the original Angular app. During E2E testing, feed/item/comment endpoints all work correctly. Only the /user/ endpoint returns 404 (documented in the test report as an external API issue). The API is partially functional, not fully defunct.
The app-root:empty selector was left over from the Angular migration. Updated to #root:empty to match the new React mount point div. Co-Authored-By: Vibha Seshadri <vibha.seshadri@cognition.ai>
1 participant
Summary
Builds on Phase 1 to add the service layer, state management, and shared utility components for the React migration.
Services:
src/services/hackernews-api.ts: Plainfetch-based API layer replacing Angular's RxJSObservablewrapper.fetchItemContentusesPromise.allfor parallel poll sub-fetching (was sequentialsubscribecalls in Angular).State:
src/contexts/SettingsContext.tsx:useReducer-based context replacingSettingsService. Mirrors Angular's localStorage keys exactly (openLinkInNewTab,theme,titleFontSize,listSpacing). Auto-detects system dark mode viamatchMedia('(prefers-color-scheme: dark)')with live listener.Shared Components:
Loader.tsx+ SCSS: CSS animation loader, ported from AngularErrorMessage.tsx+ SCSS: Skull error display, ported from Angular (usessass:mathfor$skull-sizecalculations)src/utils/formatComment.ts:(count) => string— replacesCommentPipeDepends on Phase 1 (#309).
Link to Devin session: https://app.devin.ai/sessions/0bb1dd94ecbc4422a76b7a458d0f4e7c
Requested by: @vibhaseshadri-cognition
Devin Review