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

restoreOriginalUri warning & routing samples #239

Closed
wants to merge 3 commits into from

Conversation

jaredperreault-okta
Copy link
Contributor

Fixes 'Two custom restoreOriginalUri callbacks are detected' warning
Updates routing samples (v6) to fix unnecessary calls to /authorize

eng-prod-CI-bot-okta pushed a commit that referenced this pull request Jun 30, 2022
OKTA-500147
<<<Jenkins Check-In of Tested SHA: 9f2a625 for eng_productivity_ci_bot_okta@okta.com>>>
Artifact: okta-react
Files changed count: 27
PR Link: #239
@code-bode
Copy link

code-bode commented Jul 20, 2022

I clone the latest code and try to run locally React-Router-DOM-v6 project and still seeing this error.

image

I found this #227 (comment) temp solution.

@jaredperreault-okta
Copy link
Contributor Author

@code-bode can you provide more details? I just ran the cd samples/routing/react-router-dom-v6 && yarn dev and I got no error.

Did you run yarn before trying the sample?

@code-bode
Copy link

@jaredperreault-okta Yes, I ran yarn. This error appearing when I started clicking protected link multiple time on home page.

@jaredperreault-okta
Copy link
Contributor Author

I assume this is when your unauthenticated? Are you redirected to your Okta Org after clicking protected?

I still cannot reproduce this

@code-bode
Copy link

I logged in with my Okta Org. Then I clicking on protected link multiple time and saw that error. Not sure if that matter but I am using chrome Version 103.0.5060.114 for testing.

@jaredperreault-okta
Copy link
Contributor Author

I just did a completely fresh test.

  1. Cloned the repo into a separate directory
  2. ran yarn
  3. setup testenv file
  4. cd samples/routing/react-router-dom-6
  5. yarn start

Tested in Chrome 103 and FF 102. No errors

@code-bode
Copy link

You are correct. I think issue is on my side. I have manually change below two packages "@okta/okta-auth-js": "6.6.1" , "@okta/okta-react": "^6.4.3" with different version thinking it's latest version. Even updating react-router-dom to version 6.3.0 app is working fine.

Sorry for all the confusion.

@jaredperreault-okta
Copy link
Contributor Author

@code-bode No problem at all! We can only fix the bugs we know about. Thank you for the report

@code-bode
Copy link

I know using * packages.json means use the latest version for that package, but I updated these two packages from * to their latest version "@okta/okta-auth-js": "6.7.2" , "@okta/okta-react": "6.5.0" I am seeing that error. Do you think am i still doing something wrong?

@jaredperreault-okta
Copy link
Contributor Author

jaredperreault-okta commented Jul 20, 2022

Ok, I see what is happening.

So we use * in the package.json because the samples are all yarn workspaces. (* will install the latest, if there is no other "version" of the module are installed, from another workspace or otherwise). Therefore @okta/okta-react: "*" will pull from the locally built package, the top-level workspace (this is built as a result of running yarn). master is "at" 6.6.0 and includes this PR which specifically fixes this warning.

So when you specify @okta/okta-react: "6.5.0", you'll actually pull the module from npm and 6.5.0 does not include the restoreOriginalUri fix.

I'll work on getting 6.6.0 released

@code-bode
Copy link

code-bode commented Jul 20, 2022

Thanks for quick response and working on releasing new version.

@pzi
Copy link

pzi commented Jul 21, 2022

Great, thanks for this. Looking forward to 6.6

@jaredperreault-okta
Copy link
Contributor Author

@code-bode @pzi 6.6.0 has been released

@pzi
Copy link

pzi commented Jul 25, 2022

Works 👍🏼

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.

5 participants