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

App state interface #240

Merged
merged 12 commits into from
Dec 1, 2021
Merged

Conversation

4javier
Copy link
Contributor

@4javier 4javier commented Nov 11, 2021

This little PR add two short interfaces to type the appState that can be passed to login command to be stored before redirect.
At the moment the only typed parameter is optional target but other ones could be added in the future, and custom ones are allowed by interface signature.

This way, when writing a loginWithRedirect(...) call, both appState object both its string target property will be suggested by autocompletion.

To get this working two interfaces have been created

/**
 * Extends auth0-spa-js RedirectLoginOptions narrowing
 * appState property type to NgAppState
 */
export interface NgRedirectLoginOptions extends RedirectLoginOptions {
  appState?: NgAppState;
}

and

/**
 * Angular specific state to be stored before redirect
 */
export interface NgAppState {
  /**
   * Target path the app gets routed to after
   * callback from login (defaults to '/')
   */
  target?: string;

  /**
   * For any custom parameter the user would store in appState
   */
  [key: string]: any;
}

With this setup, future angular specific options can be kept separated from auth0-spa-js code without interfering with it, since the assertions it issues on the extended RedirectLoginOptions only care about it being an object and about its basic properties types.

@4javier 4javier requested a review from a team as a code owner November 11, 2021 12:17
@frederikprijck
Copy link
Member

frederikprijck commented Nov 30, 2021

Thanks for the PR. We believe having a typed appState is an improvement, however we want to make a change to our Auth0-SPA-JS SDK to ensure other wrapping SDKs (Angular, React, ...) benefit from that same typed AppState.

Once auth0/auth0-spa-js#846 is merged and released, we should be able to update the Angular SDK by mostly adding some generic types to AuthService, something along the lines of:

export interface NgAppState {
  target?: string;
  [key: string]: any;
}

export class AuthService<TAppState extends NgAppState = NgAppState> implements OnDestroy {
  private appStateSubject$ = new ReplaySubject<TAppState>(1);

  loginWithRedirect(options?: RedirectLoginOptions<TAppState>): Observable<void> {
    return from(this.auth0Client.loginWithRedirect(options));
  }
  
  handleRedirectCallback(url?: string): Observable<RedirectLoginResult<TAppState>> {
    return defer(() => this.auth0Client.handleRedirectCallback<TAppState>(url)).pipe(
      withLatestFrom(this.authState.isLoading$),
      tap(([result, isLoading]) => {
        if (!isLoading) {
          this.authState.refresh();
        }
        const appState = result?.appState;
        const target = appState?.target ?? '/';

        if (appState) {
          this.appStateSubject$.next(appState);
        }

        this.navigator.navigateByUrl(target);
      }),
      map(([result]) => result)
    );
  }
}

@frederikprijck frederikprijck added the CH: Changed PR is changing something label Dec 1, 2021
@frederikprijck frederikprijck merged commit 1541565 into auth0:master Dec 1, 2021
@frederikprijck frederikprijck added this to the v1.8.1 milestone Dec 7, 2021
@frederikprijck frederikprijck mentioned this pull request Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Changed PR is changing something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants