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

openUrl requires a returned promise, but why? #521

Closed
eddiemonge opened this issue Mar 7, 2023 · 3 comments · Fixed by auth0/auth0-spa-js#1087
Closed

openUrl requires a returned promise, but why? #521

eddiemonge opened this issue Mar 7, 2023 · 3 comments · Fixed by auth0/auth0-spa-js#1087

Comments

@eddiemonge
Copy link

Describe the problem

In version 2.x.x, why does openUrl as a function, require a returned promise? The example shows it returning void, which doesn't need to be resolved in a promise.

* @example
   * await auth0.logout({
   *   async openUrl(url) {
   *     window.location.replace(url);
   *   }
   * });
   */
  openUrl?: false | ((url: string) => Promise<void>);
@frederikprijck
Copy link
Member

frederikprijck commented Mar 7, 2023

Hello,

This is because some integrations will return a promise, such as Capacitor's Browser.open.

Typically, opening a browser is an async operation, but not when using window.location.replace.
However, when you take control over opening the URL, our SDK will wait until the returned promise is resolved.

The example shows it returning void, which doesn't need to be resolved in a promise.

An async function that does not return anything will return a Promise<void>, not void.

image

This is exactly what is happening in the example you linked, it's an async function not returning anything.

@eddiemonge
Copy link
Author

I meant the example does not need to be an async function since it returns void. By changing the type to

openUrl?: false | ((url: string) => Promise<void> | void)

and then the implementation to support that (which might be as simple as openUrl === false || Promise.resolve(openUrl), it would make the consumer's leaves slightly easier since they can do a normal callback or a promise based callback.

@eddiemonge
Copy link
Author

Thank you all!

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 a pull request may close this issue.

2 participants