feat: Initialize React Project with Create React App for Angular to React Migration#176
feat: Initialize React Project with Create React App for Angular to React Migration#176devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
Conversation
- Replace Angular 9 project with React 19 + TypeScript 4.9 (CRA template) - Configure Prettier with matching rules (trailingComma: es5, tabWidth: 4, singleQuote: true, printWidth: 120) - Configure ESLint with react-app preset and Prettier integration - Set up React Router v6 with routes matching Angular app (/news, /newest, /show, /ask, /jobs, /item, /user) - Create folder structure: components/, contexts/, hooks/, services/, types/ - Add SettingsContext provider for app-wide settings - Add TypeScript type definitions for HN API data models - Set up environment config files (.env.development, .env.production) - Update public/index.html with SEO meta tags (Open Graph, Twitter cards) from Angular app - Update manifest.json for PWA with matching theme color (#b92b27) - Add useDocumentTitle custom hook - Add API base URL service configuration Co-Authored-By: Matthew Guerra <matthew.guerra@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:
|
|
|
||
| test('renders learn react link', () => { | ||
| render(<App />); | ||
| const linkElement = screen.getByText(/learn react/i); |
There was a problem hiding this comment.
🟡 Test searches for 'learn react' text that doesn't exist in the App component
The test at src/App.test.tsx:7 uses screen.getByText(/learn react/i) to find an element, but the App component (src/App.tsx) renders "React HN" and "Hacker News client built with React + TypeScript" — neither of which contains the string "learn react".
Root Cause
This appears to be a leftover from the default Create React App template test that was not updated to match the actual App component content. getByText will throw an error when it cannot find a matching element, causing the test to always fail.
Impact: The npm test command will always produce a failing test, breaking CI and giving a false impression that the application is broken.
| const linkElement = screen.getByText(/learn react/i); | |
| const linkElement = screen.getByText(/React HN/i); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Devin Review found 3 potential issues.
🐛 1 issue in files not directly in the diff
🐛 Firebase hosting public directory points to dist instead of CRA's build output (firebase.json:6)
The firebase.json file at line 6 sets "public": "dist", which was the Angular CLI output directory. CRA's react-scripts build outputs to build/, not dist/.
Root Cause
The firebase.json was not updated as part of this migration. When running firebase deploy, Firebase Hosting will look for files in the dist/ directory, which won't exist after npm run build. The deployment will either fail or serve an empty/stale site.
Impact: Firebase deployments will not serve the React app — the production site will be broken after deploying.
View 3 additional findings in Devin Review.
Debug
| test('renders learn react link', () => { | ||
| render(<App />); | ||
| const linkElement = screen.getByText(/learn react/i); | ||
| expect(linkElement).toBeInTheDocument(); |
There was a problem hiding this comment.
🔴 Test searches for non-existent "learn react" text, will always fail
The test in App.test.tsx uses screen.getByText(/learn react/i) to find an element, but the App component (src/App.tsx:10-11) renders <h1>React HN</h1> and <p>Hacker News client built with React + TypeScript</p> — there is no text matching "learn react" anywhere in the rendered output.
Root Cause
This is the default CRA test that was left unchanged after the App component was customized with the HN routes and content. getByText throws when no matching element is found, so npm test will always fail with a TestingLibraryElementError.
Impact: The test suite is broken — npm test will fail, blocking any CI pipeline that runs tests.
| test('renders learn react link', () => { | |
| render(<App />); | |
| const linkElement = screen.getByText(/learn react/i); | |
| expect(linkElement).toBeInTheDocument(); | |
| test('renders app heading', () => { | |
| render(<App />); | |
| const headingElement = screen.getByText(/React HN/i); | |
| expect(headingElement).toBeInTheDocument(); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| @@ -0,0 +1,3 @@ | |||
| const API_BASE_URL = process.env.REACT_APP_API_URL || 'https://node-hnapi.herokuapp.com'; | |||
There was a problem hiding this comment.
🚩 API_BASE_URL fallback duplicates .env value — no actual fallback if env vars are misconfigured
In src/services/api.ts:1, the fallback || 'https://node-hnapi.herokuapp.com' is identical to the value in both .env.development:2 and .env.production:2. CRA embeds REACT_APP_* env vars at build time, so process.env.REACT_APP_API_URL will always be the string from .env files (never undefined) unless someone explicitly removes the variable. The fallback is therefore unreachable in practice. This isn't a bug, but if the Heroku endpoint is dead (Heroku removed free tier in 2022), both the env files and the fallback will point to a non-functional URL.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| import { SettingsProvider } from './contexts/SettingsContext'; | ||
| import reportWebVitals from './reportWebVitals'; | ||
|
|
||
| const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement); |
There was a problem hiding this comment.
add a comment above this line
feat: Initialize React + TypeScript project (CRA) for Angular migration
Summary
Replaces the entire Angular 9 codebase with a fresh Create React App + TypeScript foundation for the Angular-to-React migration. This is a scaffolding PR — all route components are placeholders (
TODO).Key changes:
.prettierrc) matching the Angular app's rules:trailingComma: es5,tabWidth: 4,singleQuote: true,printWidth: 120react-app+prettierintegration/news,/newest,/show,/ask,/jobs,/item,/user)src/components/,src/contexts/,src/hooks/,src/services/,src/types/SettingsContextprovider,useDocumentTitlehook, TypeScript type definitions for HN API models.env.developmentand.env.productionenvironment filespublic/index.htmlwith SEO meta tags (Open Graph, Twitter cards) and PWA-related tags from the Angular appmanifest.jsonwith app name and theme color (#b92b27)Deleted: All Angular-specific files (angular.json, TSLint, Karma, Protractor e2e, SCSS, Angular components/modules/services, .travis.yml, yarn.lock, all original icon assets).
Review & Testing Checklist for Human
https://node-hnapi.herokuapp.comin.envfiles is still live — Heroku removed its free tier in 2022 and this endpoint may be dead.npm auditto assess..firebaserc,firebase.json,database.rules.json) are still compatible — the React build outputs tobuild/whereas the Angular app useddist/angular-hnpwa.Suggested test plan:
npm install, thennpm start— verify app loads athttp://localhost:3000and redirects to/news/1/news/1,/newest/1,/show/1,/ask/1,/jobs/1) — all should show placeholder textnpm run build— verify production build succeedsnpm run lint— verify no lint errorscurl https://node-hnapi.herokuapp.com/news— confirm it returns dataNotes
<div>TODO</div>components — actual HN feed/item/user components will be implemented in follow-up PRs.logo192.pngandlogo512.pngremain. Custom icons will need to be re-added..travis.yml) was removed; no replacement CI is configured yet.@types/react-router-dompackage was installed but may be redundant since react-router-dom v6+ ships its own types.Link to Devin run: https://app.devin.ai/sessions/e51e6443a5ab4e0e895822be13c76402
Requested by: @matthewguerra-cog