-
Notifications
You must be signed in to change notification settings - Fork 422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update to Angular 16 #1760
Conversation
"extends": [ | ||
"plugin:@angular-eslint/recommended", | ||
"plugin:@angular-eslint/template/process-inline-templates", | ||
"plugin:@typescript-eslint/recommended" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had to be included otherwise ESLint was complaining because it couldn't find some rules.
The plugin automatically imports these e.g.
@@ -8,7 +8,7 @@ import { LoginService } from '../login/login.service'; | |||
import { AutoLoginService } from './auto-login.service'; | |||
|
|||
@Injectable({ providedIn: 'root' }) | |||
export class AutoLoginAllRoutesGuard implements CanActivate, CanActivateChild, CanLoad { | |||
export class AutoLoginAllRoutesGuard { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: revert this (this was automatically done by the ng update
command because class guards are deprecated in favor of functional guards).
Maybe that we should deprecate these classes?
@@ -8,7 +8,7 @@ import { LoginService } from '../login/login.service'; | |||
import { AutoLoginService } from './auto-login.service'; | |||
|
|||
@Injectable({ providedIn: 'root' }) | |||
export class AutoLoginPartialRoutesGuard implements CanActivate, CanActivateChild, CanLoad { | |||
export class AutoLoginPartialRoutesGuard { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
@@ -38,7 +38,7 @@ export class CallbackService { | |||
callback$ = this.codeFlowCallbackService.authenticatedCallbackWithCode(currentCallbackUrl, config, allConfigs); | |||
} else if (this.flowHelper.isCurrentFlowAnyImplicitFlow(config)) { | |||
if (currentCallbackUrl?.includes('#')) { | |||
let hash = currentCallbackUrl.substring(currentCallbackUrl.indexOf('#') + 1); | |||
const hash = currentCallbackUrl.substring(currentCallbackUrl.indexOf('#') + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This became an error with the recommended TS ESLint config.
@@ -28,7 +28,7 @@ export class PeriodicallyTokenCheckService { | |||
private readonly authStateService: AuthStateService, | |||
private readonly refreshSessionIframeService: RefreshSessionIframeService, | |||
private readonly refreshSessionRefreshTokenService: RefreshSessionRefreshTokenService, | |||
private intervalService: IntervalService, | |||
private readonly intervalService: IntervalService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This became an error with the recommended TS ESLint config.
@@ -71,7 +71,7 @@ describe('JwkExtractor', () => { | |||
n: 'wq0vJv4Xl2xSQTN75_N4JeFHlHH80PytypJqyNrhWIp1P9Ur4-5QSiS8BI8PYSh0dQy4NMoj9YMRcyge3y81uCCwxouePiAGc0xPy6QkAOiinvV3KJEMtbppicOvZEzMXb3EqRM-9Twxbp2hhBAPSAhyL79Rwy4JuIQ6imaqL0NIEGv8_BOe_twMPOLGTJhepDO6kDs6O0qlLgPRHQVuKAz3afVby0C2myDLpo5YaI66arU9VXXGQtIp8MhBY9KbsGaYskejSWhSBOcwdtYMEo5rXWGGVnrHiSqq8mm-sVXLQBe5xPFBs4IQ_Gz4nspr05LEEbsHSwFyGq5D77XPxGUPDCq5ZVvON0yBizaHcJ-KA0Lw6uXtOH9-YyVGuaBynkrQEo3pP2iy1uWt-TiQPb8PMsCAdWZP-6R0QKHtjds9HmjIkgFTJSTIeETjNck_bB4ud79gZT-INikjPFTTeyQYk2jqxEJanVe3k0i_1vpskRpknJ7F2vTL45LAQkjWvczjWmHxGA5D4-1msuylXpY8Y4WxnUq6dRTEN29IRVCil9Mfp6JMsquFGTvJO0-Ffl0_suMZZl3uXNt23E9vGreByalWHivYmfpIor5Q5JaFKekRVV-U1KDBaeQQaHp_VqliUKImdUE9-GXNOIaBMjRvfy0nxsRe_q_dD6jc_GU', | |||
e: 'AQAB', | |||
} as JsonWebKey; | |||
let keys: JsonWebKey[] = [key1, key2, key3, key4, key5, key6, key7, key8, key9]; | |||
const keys: JsonWebKey[] = [key1, key2, key3, key4, key5, key6, key7, key8, key9]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests fail local with the error:
An error was thrown in afterAll
Uncaught TypeError: Cannot read properties of undefined (reading 'buildErrorName')
TypeError: Cannot read properties of undefined (reading 'buildErrorName')
While building/running the app this error is gone 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, during the CI this fails too 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like TS changed the behavior of static members?
As a work around I moved the static members to const variables.
I don't know if this is the best way though.
WIP:
I want to see how the CI reacts because I'm having an issue while running the tests locally.