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

Provide a way for developers to override the navigation methods used #2985

Merged
merged 26 commits into from Feb 26, 2021

Conversation

tnorling
Copy link
Collaborator

@tnorling tnorling commented Feb 6, 2021

Context/Motivation: React routers, such as react-router-dom or next/router provide the ability to do "client side routing" using the HTML5 History API which essentially means that you can navigate to other "pages" in your app without doing a hard redirect and reloading the entire webpage. In Angular you can use the built-in router to do this by calling Router.navigateByUrl()

Behavior Now: When using the redirect flow msal will attempt to send you back to the page you started on by reassigning window.location. This causes the whole SPA to reload and you gain no performance benefit from using a router.

Proposed Change: This PR removes the navigateWindow API and replaces it with a NavigationClient interface which the developer can override by providing their own implementations of navigateInternal and/or navigateExternal. Also adds a setNavigationClient API developers can use to set the NavigationClient after initializing PublicClientApplication (needed for react and angular)

Addresses #2869

Follow up enhancements:

  • Utilize this interface for popup scenarios

@tnorling tnorling linked an issue Feb 6, 2021 that may be closed by this pull request
10 tasks
@github-actions github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package msal-react Related to @azure/msal-react samples Related to the samples apps for the library. labels Feb 6, 2021
@coveralls
Copy link

coveralls commented Feb 9, 2021

Coverage Status

Coverage increased (+4.8%) to 87.472% when pulling d09cd86 on client-side-navigation into 30b5ff9 on dev.

@tnorling tnorling marked this pull request as ready for review February 10, 2021 17:32
@tnorling tnorling marked this pull request as draft February 12, 2021 20:59
@github-actions github-actions bot removed the msal-common Related to msal-common package label Feb 18, 2021
@tnorling tnorling changed the title Add the ability to use client-side navigation when redirecting back to start page Provide a way for developers to override the navigation methods used Feb 18, 2021
@tnorling tnorling marked this pull request as ready for review February 18, 2021 21:31
Copy link
Collaborator

@jo-arroyo jo-arroyo left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few nits for consistency.

lib/msal-browser/docs/navigation.md Outdated Show resolved Hide resolved
lib/msal-react/docs/performance.md Outdated Show resolved Hide resolved
samples/msal-react-samples/nextjs-sample/pages/_app.js Outdated Show resolved Hide resolved
samples/msal-react-samples/react-router-sample/src/App.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation. msal-browser Related to msal-browser package msal-react Related to @azure/msal-react samples Related to the samples apps for the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to redirect using router
5 participants