Skip to content

Update node+angular deps - #3153

Open
gbrodman wants to merge 1 commit into
google:masterfrom
gbrodman:angular
Open

Update node+angular deps#3153
gbrodman wants to merge 1 commit into
google:masterfrom
gbrodman:angular

Conversation

@gbrodman

@gbrodman gbrodman commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator
  • 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


This change is Reviewable

@gbrodman gbrodman added the kokoro:force-run Force a Kokoro build. label Jul 14, 2026
@domain-registry-eng domain-registry-eng removed the kokoro:force-run Force a Kokoro build. label Jul 14, 2026
@gbrodman
gbrodman requested a review from ptkach July 16, 2026 17:12

@ptkach ptkach left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 gbrodman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants