Update node+angular deps - #3153
Conversation
ptkach
left a comment
There was a problem hiding this comment.
@ptkach reviewed 31 files and all commit messages, and made 3 comments.
Reviewable status: 31 of 56 files reviewed, 3 unresolved discussions (waiting on gbrodman).
console-webapp/eslint.config.js line 31 at r1 (raw file):
], processor: angular.processInlineTemplates, rules: {
How did you come up with this set of rules? Have you tried to run it on our existing codebase?
console-webapp/tsconfig.json line 6 at r1 (raw file):
"compilerOptions": { "paths": { "src/*": ["./src/*"]
Not sure why you removed baseUrl? i think it was necessary for gcp deployment the last time I checked
console-webapp/src/app/registrar/registrarsTable.component.spec.ts line 36 at r1 (raw file):
BackendService, { provide: ActivatedRoute, useValue: {} as ActivatedRoute }, provideHttpClient(withXhr()),
Hmm is it necessary here and everywhere else? I'm not convinced that it is
- node to the most recent LTS version - angular dependencies to 22.x rather than 21.x - eslint to 10.7 (the old version was quite old) Deployed to alpha and everything looks all right. The Typescript changes were all done by the migration (nothing manually). G.1 numbers 8 and 9
gbrodman
left a comment
There was a problem hiding this comment.
@gbrodman made 3 comments.
Reviewable status: 21 of 56 files reviewed, 3 unresolved discussions (waiting on ptkach).
console-webapp/eslint.config.js line 31 at r1 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
How did you come up with this set of rules? Have you tried to run it on our existing codebase?
Yes. Basically, the upgrade version of eslint adds a bunch of new rules that would fail on our existing codebase. These exceptions allow us to run the upgrade without having to fix everything at once.
Tested everything on alpha and it looks OK and normal.
console-webapp/tsconfig.json line 6 at r1 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
Not sure why you removed baseUrl? i think it was necessary for gcp deployment the last time I checked
The automated system removed it. If we try to keep it in:
[ERROR] TS5101: Option 'baseUrl' is deprecated and will stop functioning in TypeScript 7.0.
But yeah it seems like telling the system that "src/" maps to "./src/" accomplishes what we need, where we can use imports as if they're from the root of the console-webapp dir.
console-webapp/src/app/registrar/registrarsTable.component.spec.ts line 36 at r1 (raw file):
Previously, ptkach (Pavlo Tkach) wrote…
Hmm is it necessary here and everywhere else? I'm not convinced that it is
This was also added automatically by the "ng update". Apparently the default transport was switched from XHR to Fetch and the tests kept this the same just in case. But you're right -- we don't need it. Removed.
Deployed to alpha and everything looks all right. The Typescript changes were all done by the migration (nothing manually).
G.1 numbers 8 and 9
This change is