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

[SDK-3627] Make buildLogoutUrl internal only and replace with onRedirect handler on logout #982

Merged
merged 2 commits into from
Sep 14, 2022

Conversation

ewanharris
Copy link
Contributor

Changes

Similar to #948/#980 this removes buildLogoutUrl from the public API making it an internal private API, and instead adds a similar onRedirect handler that will perform the redirection, continuing to use window.location.assign if not provided. This change has the consequence that the logout method is no longer synchronous and must always be await-ed

For those that used buildLogoutUrl the change looks as follows

- const url = client.buildLogoutUrl();
- await Browser.open({ url })
+ await client.logout({
+   onRedirect: async (url) => {
+     await Browser.open({ url });
+   }
+ });

If you were previously using localOnly, this should be replaced with a handler that does nothing, like so:

- client.logout({
-   localOnly: true
- });
+ await client.logout({
+    onRedirect: async (url) => { }
+ });

Internally, this also removes the clearSync function on CacheManager as given that the logout now always returns a Promise we no longer need to switch to a synchronous method to remove the keys and prevent race conditions

Checklist

@ewanharris ewanharris requested a review from a team as a code owner September 13, 2022 11:20
@ewanharris ewanharris changed the title Make buildLogoutUrl internal only and replace with onRedirect handler on logout [SDK-3627] Make buildLogoutUrl internal only and replace with onRedirect handler on logout Sep 13, 2022
@ewanharris ewanharris merged commit 736c47b into v2 Sep 14, 2022
@ewanharris ewanharris deleted the feat/SDK-3627 branch September 14, 2022 09:09
@frederikprijck frederikprijck mentioned this pull request Oct 25, 2022
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.

2 participants