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) #282

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Support react-router 6 (#275) #282

wants to merge 2 commits into from

Conversation

jaredperreault-okta
Copy link
Contributor

OKTA-676780 Support react-router 6

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: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Reviewers

OKTA-676780 Support react-router 6
@Spartan-Hex-Shadow
Copy link

Hi @jaredperreault-okta , is this PR going through your tests before being merged for a new okta-react version?

@jaredperreault-okta
Copy link
Contributor Author

@Spartan-Hex-Shadow Yes. This will become a new major version release

@Spartan-Hex-Shadow
Copy link

@Spartan-Hex-Shadow Yes. This will become a new major version release

Wonderful, thanks!

@Spartan-Hex-Shadow
Copy link

@jaredperreault-okta any E.T.A on release?

@jaredperreault-okta
Copy link
Contributor Author

@Spartan-Hex-Shadow I apologize for the delay on this, however I am trying to determine what the best course of action for this SDK is. react-router seems to be moving towards the use of action and loader functions on <Route />s rather than the component-based approach. Even this example holds the "auth state" outside of the React state foregoing the previously common pattern of useState/useContext. This something I have been pondering for a while; holding the auth state inside the app (React) state never quite seemed necessary to me. Ultimately a token's lifecycle is a function of time, therefore any value placed within the app state to represent the auth state is essentially a cached value of a point in time.
I am trying to avoid positioning this SDK in a way which forces consumers to use/implement react-router in a certain way. Ideally this SDK should no influence on how you manage your <Route />s within your application. Currently I am considering exposing some Loader function(s) rather than moving forward with SecureOutlet as the loader approach seems more future-proof.

I'm curious, have you experimented with the use of action / loader in your React app(s)?

@Spartan-Hex-Shadow
Copy link

@Spartan-Hex-Shadow I apologize for the delay on this, however I am trying to determine what the best course of action for this SDK is. react-router seems to be moving towards the use of action and loader functions on <Route />s rather than the component-based approach. Even this example holds the "auth state" outside of the React state foregoing the previously common pattern of useState/useContext. This something I have been pondering for a while; holding the auth state inside the app (React) state never quite seemed necessary to me. Ultimately a token's lifecycle is a function of time, therefore any value placed within the app state to represent the auth state is essentially a cached value of a point in time. I am trying to avoid positioning this SDK in a way which forces consumers to use/implement react-router in a certain way. Ideally this SDK should no influence on how you manage your <Route />s within your application. Currently I am considering exposing some Loader function(s) rather than moving forward with SecureOutlet as the loader approach seems more future-proof.

I'm curious, have you experimented with the use of action / loader in your React app(s)?

Hi Jarred. We have not. Our app is heavily based around components and we have began the process of addressing technical debt by upgrading to the latest versions of our third party packages but cannot do so for some as this package does not support react router 6. The SecureOutlet approach would work very well for us and would be a seamless code transition from SecureRoute So we would personally prefer that but ultimately recognize it is not up to us to decide if that happens or not.

OKTA-721854 merge master into dev7
@Spartan-Hex-Shadow
Copy link

Hi @jaredperreault-okta , any updates?

@jaredperreault-okta
Copy link
Contributor Author

@Spartan-Hex-Shadow Some other tasks have taken my attention. I'll be revisiting this soon

@Spartan-Hex-Shadow
Copy link

@jaredperreault-okta Much appreciate, thank you so much.

@Spartan-Hex-Shadow
Copy link

@jaredperreault-okta any updates?

@krook1024
Copy link

Hey @jaredperreault-okta

Any update on react-router 6 support?

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.

4 participants