Skip to content
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

Fix: returning a loginresponse and not null #1781

Merged
merged 4 commits into from
Jun 17, 2023

Conversation

FabianGosebrink
Copy link
Collaborator

@FabianGosebrink FabianGosebrink commented Jun 17, 2023

Fixes #1780

@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1781.centralus.azurestaticapps.net

1 similar comment
@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1781.centralus.azurestaticapps.net

@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://nice-hill-002425310-1781.centralus.azurestaticapps.net

@FabianGosebrink
Copy link
Collaborator Author

@timdeschryver if you want to review, feel free :)

@FabianGosebrink FabianGosebrink requested review from damienbod and removed request for damienbod June 17, 2023 14:16
@damienbod damienbod merged commit bb5bccc into main Jun 17, 2023
@damienbod damienbod deleted the fabiangosebrink-fix/type-issue-LoginResponse branch June 19, 2023 12:20
@@ -200,7 +206,7 @@ export class CheckAuthService {
accessToken: this.authStateService.getAccessToken(config),
idToken: this.authStateService.getIdToken(config),
configId,
};
} as LoginResponse;
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer not to type cast this way, instead I prefer to add a return type to the method. Because this way, we can still cheat and return a different object. With using return types we get errors if the object doesn't match the return type.

Because you asked for a review 😁

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Will do a follow up PR ;)

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.

[Bug]: type issue LoginResponse
3 participants