feature: migrate Angular HN PWA to Vite + React + TypeScript#320
feature: migrate Angular HN PWA to Vite + React + TypeScript#320devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Co-Authored-By: Samir Chaudhry <samir@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:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| return { | ||
| showSettings: false, | ||
| openLinkInNewTab: openLinkInNewTab ? JSON.parse(openLinkInNewTab) : false, | ||
| theme: 'default', | ||
| titleFontSize: titleFontSize ? titleFontSize : '16', | ||
| listSpacing: listSpacing ? listSpacing : '0', | ||
| }; |
There was a problem hiding this comment.
🟡 Theme not read from localStorage synchronously, causing visible flash on page load
The readInitialSettings function at src/context/SettingsContext.tsx:24-36 reads openLinkInNewTab, titleFontSize, and listSpacing from localStorage synchronously, but hardcodes theme: 'default' (line 32). The actual saved theme is only restored later in a useEffect (line 47-65), which runs after the first paint. Since the App component at src/App.tsx:11 renders <div className={theme}> which controls all theming CSS, users who have selected 'night' or 'amoledblack' will see a flash of the light/default theme on every page load. This is a regression from the Angular version where the theme was initialized synchronously in the service constructor before rendering.
| return { | |
| showSettings: false, | |
| openLinkInNewTab: openLinkInNewTab ? JSON.parse(openLinkInNewTab) : false, | |
| theme: 'default', | |
| titleFontSize: titleFontSize ? titleFontSize : '16', | |
| listSpacing: listSpacing ? listSpacing : '0', | |
| }; | |
| const savedTheme = localStorage.getItem('theme'); | |
| const systemDark = !savedTheme && window.matchMedia('(prefers-color-scheme: dark)').matches; | |
| return { | |
| showSettings: false, | |
| openLinkInNewTab: openLinkInNewTab ? JSON.parse(openLinkInNewTab) : false, | |
| theme: savedTheme || (systemDark ? 'night' : 'default'), | |
| titleFontSize: titleFontSize ? titleFontSize : '16', | |
| listSpacing: listSpacing ? listSpacing : '0', | |
| }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| useEffect(() => { | ||
| const darkColorSchemeMedia = window.matchMedia('(prefers-color-scheme: dark)'); | ||
|
|
||
| const handleChange = (event: MediaQueryListEvent | MediaQueryList) => { | ||
| setTheme(event.matches ? 'night' : 'default'); | ||
| }; | ||
|
|
||
| const savedTheme = localStorage.getItem('theme'); | ||
| if (savedTheme) { | ||
| setSettings((prev) => ({ ...prev, theme: savedTheme })); | ||
| } else { | ||
| handleChange(darkColorSchemeMedia); | ||
| } | ||
|
|
||
| darkColorSchemeMedia.addEventListener('change', handleChange); | ||
| return () => { | ||
| darkColorSchemeMedia.removeEventListener('change', handleChange); | ||
| }; | ||
| }, [setTheme]); |
There was a problem hiding this comment.
🚩 System color scheme listener persists theme to localStorage on first detection
In src/context/SettingsContext.tsx:57-58, when no saved theme exists, handleChange(darkColorSchemeMedia) calls setTheme (line 51), which writes to localStorage (line 43). This means the first system-preference-based theme choice gets persisted, so subsequent loads will use the saved value rather than continuing to follow the system preference. The same setTheme call happens when the event listener fires (line 61), so any runtime system theme change also gets persisted. This behavior is inherited from the original Angular SettingsService which had the same pattern, so it's not a regression, but it's arguably unexpected — users who never explicitly choose a theme might expect the app to always follow their OS preference.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
1 participant
Summary
Replaces the Angular 9 Hacker News PWA with an equivalent Vite + React 18 + TypeScript app, preserving all routes, theming, settings, PWA behavior, and the
node-hnapi.herokuapp.comdata source. All Angular scaffolding (angular.json,karma,protractor,src/app/**,ngsw-config.json, ejected build) is removed; icons/SVGs are kept (moved topublic/assets).Architecture mapping:
HackerNewsAPIService(RxJSObservable+unfetch)src/api/hackernews.ts(asyncfetch) +@tanstack/react-queryhooks insrc/hooks/useHackerNews.tsSettingsService(DI singleton)src/context/SettingsContext.tsx(SettingsProvider+useSettings)app.routes.ts(modules, lazyloadChildren)src/router.tsx(React Router v6,React.lazyfor item/user)CommentPipeformatCommentCountinsrc/utils/index.ts.scss(:host,ngStyle)src/styles/Behavior preserved deliberately:
fetchItemContentsequentially fetchesstory.id + ifor each option and sumspoll_votes_count.SettingsContextreads the same localStorage keys (openLinkInNewTab,titleFontSize,listSpacing,theme) and keeps theprefers-color-scheme: darklistener that flips theme tonight/defaultwhen no theme is saved.default/night/amoledblack) is kept as global descendant selectors (.<theme> .wrapper …), with the theme class applied to the rootdivinApp.tsxfromuseSettings().theme— so CSS Modules were intentionally not used (they would break these selectors)./→/news/1,/:feed/:pagefor news/newest/show/ask/jobs,/item/:id,/user/:id.PWA:
@angular/service-worker+ngsw-config.jsonreplaced byvite-plugin-pwa(WorkboxgenerateSW) configured invite.config.ts, including aNetworkFirstruntime cache for the HN API and a generated web manifest.package.jsonscripts (dev/build/lint/typecheck/test) are kept to match the repo blueprint.build,lint, andtypecheckpass; verified rendering against the live API (feed loads, theming applies).Testing
npm run build✅ (only Sass@import/darkendeprecation warnings)npm run typecheck✅npm run lint✅/news/1in the browser against the live API — stories, header nav, and day theme render correctly.Link to Devin session: https://app.devin.ai/sessions/5742358eeb0245c9823e9812addb0e55
Requested by: @schaudhry123
Devin Review
96cbb9a