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

Adds raiseEvent parameter to getUser() method #1817

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

waldemarennsaed
Copy link

@waldemarennsaed waldemarennsaed commented Jan 21, 2025

Description

Relates to #712 and provides a solution that is backwards-compatible and will fix an issue for some users.

Adding a new parameter raiseEvent = false to the UserManager.getUser() method to allow execute the addUserLoaded() event handlers after raising the userLoaded event.

Background: The userLoaded event does currently not get raised whenever the user value is being read from the local storage, which happens whenever no user data has to be fetched from the authority but from the store. In #712 developers complained about this behavior since some business-logic depends on the getUser()/userLoaded method/event for .e.g. loading additional user-related data from third-party sources like APIs. In the case that a user is already authenticated and e.g. reloads the page, the storage provides the getUser() data but does not raise the userLoaded event anymore. That will break the described business-logic and must be avoided with a workaround, e.g. using Proxy() which is not preferred as the optimal solution (see #712 (comment) for an implementation example with Proxy()).

This solution allows raising the event on purpose whenever the parameter is set to handle further business logic after checking for a users authentication.

Tests

Added unit-tests for the new parameter which is by default set to false.

…aded event

By default set to `false` to maintain backwards-compatibility and avoid breaking changes.
@waldemarennsaed
Copy link
Author

@pamapa any thoughts on this?

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.

1 participant