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

Support react-router 6 #275

Merged
merged 10 commits into from
Feb 27, 2024

Conversation

denysoblohin-okta
Copy link
Contributor

@denysoblohin-okta denysoblohin-okta commented Feb 26, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Adding Tests
  • Build related changes
  • CI related changes
  • Documentation changes
  • Other... Please describe:

What is the current behavior?

Issue Number: OKTA-676780

Resolves #178

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Reviewers

src/Security.tsx Outdated
@@ -60,6 +61,10 @@ const Security: React.FC<{
restoreOriginalUri(oktaAuth as OktaAuth, originalUri);
}) as ((oktaAuth: OktaAuth, originalUri?: string) => Promise<void>);

return () => {
// Restore original callback
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes #227 for React 18 StrictMode

src/AuthRequired.tsx Outdated Show resolved Hide resolved
src/AuthRequired.tsx Outdated Show resolved Hide resolved
src/Security.tsx Outdated
@@ -60,6 +61,10 @@ const Security: React.FC<{
restoreOriginalUri(oktaAuth as OktaAuth, originalUri);
}) as ((oktaAuth: OktaAuth, originalUri?: string) => Promise<void>);

return () => {
// Restore original callback
oktaAuth.options.restoreOriginalUri = originalCallback;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is necessary, the Security component should never unmount

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cleanup function fixes the issue with React 18 StrictMode.
If you run test/apps/v6-app sample app without it, you can notice the warning Two custom restoreOriginalUri callbacks are detected. The one from the OktaAuth configuration will be overridden by the provided restoreOriginalUri prop from the Security component.
It happens because effects run twice

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting, that's probably because StrictMode was changed in React18 to do a double mount

I wonder if we should use this approach instead: https://react.dev/learn/you-might-not-need-an-effect#initializing-the-application

Trevor used it as well for something similar: #266

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to global var approach.
This required a workaround for tests: 0132a0e

@denysoblohin-okta denysoblohin-okta marked this pull request as ready for review February 26, 2024 15:50
@denysoblohin-okta denysoblohin-okta changed the base branch from master to dev7 February 26, 2024 17:04
@eng-services-aperture-github-bot-okta eng-services-aperture-github-bot-okta merged commit 24c6370 into dev7 Feb 27, 2024
1 check passed
@eng-services-aperture-github-bot-okta eng-services-aperture-github-bot-okta deleted the od-router-OKTA-676780-2 branch February 27, 2024 09:47
@rodalcala
Copy link

this is really useful, even the updates to the docs are super convenient and helped me a lot.
do you know when is this going to reach a stable release?

@Spartan-Hex-Shadow
Copy link

Hi everyone

We are upgrading an old application to the latest version okta-react and are encountering the same issue of SecureRoute no longer working after going to version Version 6. Will this update be released soon?

@jaredperreault-okta
Copy link
Contributor

We don't have an official timeline, this is a major version release, so we are proceeding carefully

@Spartan-Hex-Shadow
Copy link

We don't have an official timeline, this is a major version release, so we are proceeding carefully

Ok, thank you for the clarification!

jaredperreault-okta pushed a commit that referenced this pull request May 30, 2024
OKTA-676780 Support react-router 6
jaredperreault-okta pushed a commit that referenced this pull request May 30, 2024
OKTA-676780 Support react-router 6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants