feat: upgrade Angular 9 to Angular 18#263
Open
devin-ai-integration[bot] wants to merge 1 commit into
Open
Conversation
- Update all @angular/* packages from ~9.0.1 to ^18.2.0 - Update TypeScript from ~3.7.5 to ~5.4.5 - Update RxJS from ~6.5.4 to ~7.8.1 - Update zone.js from ~0.10.2 to ~0.14.10 - Update tslib from ^1.10.0 to ^2.6.0 - Remove rxjs-compat (no longer needed with RxJS 7) - Remove deprecated packages: codelyzer, tslint (dev), protractor, ts-node, jasmine-spec-reporter, @types/jasminewd2, @angular/language-service - Update karma ecosystem: karma ~6.4, karma-coverage ~2.2 (replaces karma-coverage-istanbul-reporter), karma-jasmine ~5.1, karma-chrome-launcher ~3.2, karma-jasmine-html-reporter ~2.1 - Update jasmine-core to ~5.1 and @types/jasmine to ~5.1 - Update @types/node to ^20.0.0 - Fix RxJS imports: rxjs/Observable -> rxjs, rxjs/Subscription -> rxjs - Fix zone.js imports: zone.js/dist/zone -> zone.js, zone.js/dist/zone-testing -> zone.js/testing - Update polyfills configuration in angular.json to use array format - Update tsconfig.json for ES2022 target/module, add Angular compiler options - Remove extractCss (deprecated) from angular.json production config - Remove defaultProject (deprecated) from angular.json - Remove protractor e2e config from angular.json - Update browserslist for modern browsers - Update karma.conf.js to use karma-coverage instead of karma-coverage-istanbul-reporter - Fix TS7030 error in hackernews-api.service.ts (not all code paths return) Co-Authored-By: david.bean <david.bean@cognition.ai>
Author
🤖 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:
|
Author
E2E Test ResultsRan Test Results — 7/7 passed
ScreenshotsApp loads with news feedItem detail with commentsNight themeConsole — no Angular errorsNotes:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upgrades the Angular Hacker News PWA from Angular 9 (
~9.0.1) to Angular 18 (^18.2.0). This is a direct version jump with manual dependency and config updates rather than incrementalng updatemigrations (which were hanging due to Node.js 20 incompatibility with older Angular CLI versions).Key changes:
@angular/*packages:~9.0.1→^18.2.0~3.7.5→~5.4.5~6.5.4→~7.8.1(removedrxjs-compat)~0.10.2→~0.14.10^1.10.0→^2.6.0codelyzer,tslint,protractor,ts-node,jasmine-spec-reporter,@types/jasminewd2,@angular/language-servicekarma-coverage-istanbul-reporterwithkarma-coveragetsconfig.jsonto target ES2022 with Angular compiler optionsrxjs/Observable→rxjs,rxjs/Subscription→rxjs)zone.js/dist/zone→zone.js)extractCss,defaultProject, and Protractor e2e config fromangular.jsonangular.jsonBuild passes (
ng buildsucceeds).Review & Testing Checklist for Human
lazyFetchreturn removal inhackernews-api.service.ts:54: Thereturnwas removed fromreturn res.json().then(...)to fix TS7030. This changes error propagation — errors fromres.json()will no longer bubble to the outer.catch(). Confirm this doesn't break API error handling at runtime.ng lintstill works: The lint builder inangular.jsonstill references@angular-devkit/build-angular:tslint, buttslintwas removed from devDependencies. Thetslint.jsonfile is also still present. Runningng lintwill likely fail. Consider whether to remove the lint config entirely or migrate to ESLint.ng serve) and verify the HN feed loads, navigation works, item details render, and user profiles display correctly.src/polyfills.tsis no longer referenced bytsconfig.app.jsonorangular.jsonbut still exists.tslint.jsonand thee2e/directory remain in the repo. Consider cleanup./-for-division warnings in SCSS files will become errors in Dart Sass 2.0. Not blocking but worth tracking.Recommended test plan: Run
ng serve, openhttp://localhost:4200, navigate through news/newest/show/ask/jobs feeds, click into an item to view comments, click a username to view a user profile, and toggle settings (theme, font size).Notes
ng updateincremental approach was attempted but hung indefinitely due to Node.js 20 incompatibility with Angular CLI 9/10. Manual package.json and config updates were used instead.angular.jsonstill usesbrowserTarget(deprecated in Angular 17+ in favor ofbuildTarget) — this may produce warnings but is functional.Link to Devin session: https://app.devin.ai/sessions/204a757cce74416c974e7e6f3d712019
Requested by: @davidbean-hash
Devin Review