Migrate Conduit (Angular) to React 18 + Vite in react-app/#35
Migrate Conduit (Angular) to React 18 + Vite in react-app/#35devin-ai-integration[bot] wants to merge 5 commits into
Conversation
… wire CI Co-Authored-By: Lukas Burger <lukaskburger@gmail.com>
Co-Authored-By: Lukas Burger <lukaskburger@gmail.com>
Co-Authored-By: Lukas Burger <lukaskburger@gmail.com>
🤖 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:
|
Co-Authored-By: Lukas Burger <lukaskburger@gmail.com>
CI status
The 6
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const updateUser = useCallback( | ||
| async (user: UserSettingsUpdate) => { | ||
| const updated = await updateUserRequest(user); | ||
| updateCurrentUser(updated); | ||
| return updated; | ||
| }, | ||
| [updateCurrentUser], | ||
| ); |
There was a problem hiding this comment.
🚩 updateUser does not persist the returned JWT token to localStorage
The updateUser callback at react-app/src/context/AuthContext.tsx:132-139 calls updateCurrentUser(updated) but does not call saveToken(updated.token). The PUT /user API returns a user object that includes a token field. If the backend rotates the JWT on user update, the in-memory user will have the new token but localStorage will retain the stale one. On next page refresh, the app would attempt to authenticate with the old token.
However, this is consistent with the Angular version at src/app/core/auth/services/user.service.ts:158-164, which also only calls currentUserSubject.next(user) without saving the token. So this is a faithful port of a pre-existing behavior — not a regression. Worth fixing in both versions if the demo API ever starts rotating tokens on user update.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentionally left as-is. As noted, this faithfully mirrors the Angular UserService.update (src/app/core/auth/services/user.service.ts), which only pushes the user into the in-memory subject and does not re-persist the token. Since this PR is a strict 1:1 migration, changing the token-persistence behavior here would diverge from the baseline. The live demo API does not rotate the JWT on PUT /user, so there's no observable impact today; if that ever changes it should be fixed in both apps together.
| command: 'bun run dev', | ||
| cwd: 'react-app', |
There was a problem hiding this comment.
🚩 Playwright E2E tests now target the React app instead of Angular
The playwright.config.ts change switches the web server from npm run start (Angular CLI dev server) to bun run dev with cwd: 'react-app' (Vite dev server). This means all E2E tests in the e2e/ directory now run exclusively against the React app. Any behavioral differences between the Angular and React implementations could cause E2E test regressions. The Angular app's dev server is no longer exercised by CI at all. This is presumably intentional as part of the migration, but worth confirming that the Angular app isn't meant to continue being tested independently.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed intentional. The goal of this PR is to validate the React port against the existing e2e suite, so the Playwright webServer now boots the React app. The Angular build/unit tests still run via the existing test job (root bun run test) and the Angular source is untouched. If you'd prefer to keep an Angular e2e job running in parallel during the transition, that's easy to add — let me know and I'll wire up a second webServer/project. Final disposition of the Angular source (keep vs. remove) is the open question on the PR.
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| const updateUser = useCallback( | ||
| async (user: UserSettingsUpdate) => { | ||
| const updated = await updateUserRequest(user); | ||
| updateCurrentUser(updated); | ||
| return updated; | ||
| }, | ||
| [updateCurrentUser], |
There was a problem hiding this comment.
🟡 updateUser does not persist the new token from the API response to localStorage
The updateUser callback in AuthContext calls updateCurrentUser(updated) to update the in-memory user state, but unlike setAuth (used by login/register at react-app/src/context/AuthContext.tsx:50-58), it never calls saveToken(updated.token). The PUT /user RealWorld API endpoint returns a full user object including a token field. If the backend returns a refreshed/rotated token, currentUser.token (in-memory) and localStorage.jwtToken (used by apiClient at react-app/src/api/apiClient.ts:63 for all subsequent requests) will diverge. Subsequent API calls will use the stale localStorage token, potentially causing silent 401 failures.
The Angular version (src/app/core/auth/services/user.service.ts:158-163) has the same omission, so existing E2E tests pass, but the inconsistency between setAuth and updateUser is a latent correctness issue.
| const updateUser = useCallback( | |
| async (user: UserSettingsUpdate) => { | |
| const updated = await updateUserRequest(user); | |
| updateCurrentUser(updated); | |
| return updated; | |
| }, | |
| [updateCurrentUser], | |
| const updateUser = useCallback( | |
| async (user: UserSettingsUpdate) => { | |
| const updated = await updateUserRequest(user); | |
| saveToken(updated.token); | |
| updateCurrentUser(updated); | |
| return updated; | |
| }, | |
| [updateCurrentUser], | |
| ); |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| command: 'bun run dev', | ||
| cwd: 'react-app', |
There was a problem hiding this comment.
🚩 Playwright config now runs React app instead of Angular — Angular E2E coverage removed
The root playwright.config.ts was changed from command: 'npm run start' (Angular) to command: 'bun run dev', cwd: 'react-app' (React). This means all existing E2E tests in e2e/ now run exclusively against the React app. The Angular app's E2E coverage is silently dropped. This is presumably intentional for the migration, but it means the Angular app (still present in src/) no longer has automated E2E validation. If both apps need to be maintained in parallel, a separate Playwright config for Angular would be needed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Co-Authored-By: Lukas Burger <lukaskburger@gmail.com>
| event.preventDefault(); | ||
| setIsSubmitting(true); | ||
| try { | ||
| const user = await updateUser({ image, username, bio, email, password }); |
There was a problem hiding this comment.
🔴 Settings page always sends empty password string to API, potentially resetting user's password
In SettingsPage.tsx, password state is initialized to '' (line 18) and is always included in the updateUser call (line 35): await updateUser({ image, username, bio, email, password }). When the user updates any setting without typing a new password, the request payload includes password: ''. Depending on the backend implementation, this could overwrite the user's password with an empty string, locking them out.
The Angular version (src/app/features/settings/settings.component.ts:31-34) had Validators.required on the password FormControl, preventing form submission when the password was empty. The React port drops this validation entirely, meaning every profile update sends password: '' to PUT /user.
Fix: only include password when non-empty
Filter out the empty password before sending to the API:
const payload = { image, username, bio, email, ...(password ? { password } : {}) };
const user = await updateUser(payload);| const user = await updateUser({ image, username, bio, email, password }); | |
| const user = await updateUser({ image, username, bio, email, ...(password ? { password } : {}) }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I dug into this and intentionally kept the React port identical to Angular — the premise that Angular prevented submission isn't accurate.
Angular's password control does have Validators.required, but the submit button is <button type="submit">Update Settings</button> with no [disabled]="settingsForm.invalid" binding (src/app/features/settings/settings.component.html — the only disable is <fieldset [disabled]="isSubmitting()">). In Angular reactive forms, (ngSubmit) fires regardless of form validity, and submitForm() sends this.settingsForm.value, which includes password: ''. So the Angular app already sends { user: { ..., password: '' } } to PUT /user on every save without a new password — exactly what this React port does.
This is in fact the root cause of the 6 pre-existing settings.spec.ts e2e failures, which fail identically on the Angular baseline (the live api.realworld.show backend now rejects the empty-password payload). Applying the suggested ...(password ? { password } : {}) filter would make the React app diverge from Angular (its settings e2e tests would start passing while Angular's still fail), which breaks the 1:1 parity this migration is mandated to preserve.
So I'm leaving it as a faithful port. It's a real latent issue worth fixing — but it should be fixed in both apps in a separate change, not silently introduced in the React port. I've flagged this to the requester.
| const updateUser = useCallback( | ||
| async (user: UserSettingsUpdate) => { | ||
| const updated = await updateUserRequest(user); | ||
| updateCurrentUser(updated); | ||
| return updated; | ||
| }, | ||
| [updateCurrentUser], | ||
| ); |
There was a problem hiding this comment.
🚩 Angular update() also omits saveToken — the React port faithfully mirrors this
The bug I reported (BUG-0001) in updateUser not saving the token is actually a pre-existing pattern from the Angular UserService.update() at src/app/core/auth/services/user.service.ts:158-163, which also only calls this.currentUserSubject.next(user) without this.jwtService.saveToken(user.token). The React port faithfully replicates this. Whether to fix it depends on whether the demo backend at api.realworld.show ever rotates the JWT on PUT /user. For most Conduit implementations the token stays stable, so the practical impact is low.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
1 participant
Summary
Adds a 1:1 port of the Conduit app from Angular to React 18 + TypeScript + Vite in a new
react-app/directory, without removing the Angular source (per the migration playbook's "Do not remove Angular components" rule). Every route, the external RealWorld API contract, and the custom Conduit theme are preserved. Build, lint, and unit tests are green; the Playwright e2e suite now runs against the React app with full parity to the Angular baseline.The migration follows a direct mechanical mapping: Angular services → a single
apiClientfetch wrapper + plain async API modules; the auth interceptor/UserService→ React Context (AuthProvider); RxJS Observables →async/await+AbortController; Angular pipes → utility functions; templates → JSX.Architecture mapping (Angular → React)
react-app/src)api/apiClient.ts(single fetch wrapper)UserService/JwtService(identity, session)context/AuthProvider.tsx+hooks/useAuth.ts+api/jwt.tsArticlesService,CommentsService,ProfileService,TagsService,UserService.updateapi/{articles,comments,profiles,tags,users}.ts(async fns)requireAuth/requireUnauthroute guardscomponents/RouteGuards.tsx(RequireAuth/RedirectIfAuthenticated)DatePipe,MarkdownPipe,DefaultImagePipeutils/{date,markdown,defaultImage}.tscomponents/*.tsx,pages/*.tsx(JSX)app.config.tsroutingApp.tsx(React Router v6)Routes preserved (exact URL structure)
/,/tag/:tag(via query/state),/login,/register,/settings,/editor,/editor/:slug,/article/:slug,/profile/:username,/profile/:username/favorites.Behavioral parity notes (high-entropy)
https://api.realworld.show/api, JWT inlocalStorage['jwtToken'], sent asAuthorization: Token <jwt>. Article create still POSTs to the canonical/articles/path (matches Angular + e2e mocks).canActivatedecides once per navigation and latches.RequireAuthreplicates this with auseRefso a mid-session global 401 (e.g. a failed write) does not yank the user off a protected page — the form stays open to show the error, matching Angular UX. Eject only happens via explicitlogout().realworldtheme). Theme media assets are copied toreact-app/public/assets/so Vite can serve them.CI / tooling
.github/workflows/react.yml:bun install→lint→test(Vitest) →buildforreact-app/(deterministic, green).playwright.config.tsnow bootsreact-app(bun run dev, cwdreact-app) on port 4200;playwright.yml+security-tests.ymlinstallreact-appdeps before running.bun run format:check(the existing "Format Check" job) passes over the new files.Verification
cd react-app && bun run build→ zero errorscd react-app && bun run lint→ zero warningscd react-app && bun run test→ 8 passing unit tests (utils + ListErrors)bun run test:e2e(against React) → 116 passed, 6 failed;@security→ 16/16 passed. The 6 failures are pre-existingsettings.spec.tscases that hit the live API and fail identically on the Angular baseline (the live demo backend rejects the empty password field both apps send) — not a regression.Manually verified in the browser (screenshots below): global feed + popular tags, article view with rendered markdown + comments, register → authenticated state, publish article via editor, post comment, profile page.
Not done (needs a decision)
The issue asks to remove the Angular source after verification, but the migration playbook explicitly forbids removing Angular components. I left Angular in place and documented the React app in the README instead. Removal can be a fast follow-up PR once confirmed.
Screenshots
Home (global feed + popular tags):
Article view (markdown rendered, comments):
Authenticated home after register (New Article / Settings / username, Your Feed tab):
Profile page:
Link to Devin session: https://app.devin.ai/sessions/611aa0723cc6489186f58ed706c72be6
Requested by: @lburgers
Devin Review
0f05267