"Two custom restoreOriginalUri callbacks are detected" warning

[CruseCtrl]

Describe the bug?

I'm getting a warning saying that "Two custom restoreOriginalUri callbacks are detected. The one from the OktaAuth configuration will be overridden by the provided restoreOriginalUri prop from the Security component."

What is expected to happen?

I shouldn't get a warning

What is the actual behavior?

I get a warning

Reproduction Steps?

const oktaAuth = new OktaAuth({})

type Props = { children: React.ReactNode }

export const OktaSecurity = ({ children }: Props) => {
  const navigate = useNavigate()

  const restoreOriginalUri = useCallback(
    (oktaAuth: OktaAuth, originalUri: string) => {
      navigate(toRelativeUrl(originalUri || '/', window.location.origin), {
        replace: true,
      })
    },
    [navigate],
  )

  return (
    <Security oktaAuth={oktaAuth} restoreOriginalUri={restoreOriginalUri}>
      {children}
    </Security>
  )
}

SDK Versions

@okta/okta-auth-js@6.3.0
@okta/okta-react@6.4.3

Execution Environment

MacOS Big Sur 11.6

Additional Information?

It looks like this is caused in Security.tsx on lines 51-56.

In the first render, oktaAuth.options.restoreOriginalUri won't exist and so it won't log a warning. But then on line 54 it sets a value to oktaAuth.options.restoreOriginalUri. So on the next render when it checks if (oktaAuth.options.restoreOriginalUri && restoreOriginalUri) it will show the warning, because it's just added a value! (And the useEffect will run again because you've just changed one of the properties of oktaAuth.)

This problem is also mentioned in #170, where the accepted solution was to create a new oktaAuth for each render. This will get rid of the error because it doesn't reuse the oktaAuth which now has a restoreOriginalUri, but it's bad practice to create a new object on every render.

It's a bit weird that you're modifying the oktaAuth object that's passed in. Some options of how to fix this are:

1. Create a copy of oktaAuth and modify the copy, leaving the original unchanged
2. Pass the restoreOriginalUri into the OktaContext.Provider so that we can keep the original oktaAuth
3. Remove the option to pass restoreOriginalUri as a separate parameter to Security, instead requiring that the user sets the restoreOriginalUri directly on the oktaAuth.options themselves (and add a suitable error message if the user hasn't done this)
4. Or just remove this warning entirely, since it's broken

I think 3 is my favourite, but it would require a breaking change so I'd welcome any feedback. I'm also happy to raise a pull request once we've agreed upon a solution

[RikkiMasters]

Same issue since upgrading to React 18

[denysoblohin-okta]

@CruseCtrl What are the reasons of re-rendering component?

[CruseCtrl]

@denysoblohin-okta every time anything on the page changes, React re-renders the components. I'm not sure I fully understand the question

[denysoblohin-okta]

Security component should re-render and call its useEffect hook if restoreOriginalUri or oktaAith changes. oktaAuth should not change as it's being constructed outside of OktaSecurity. So seems like restoreOriginalUri changes, probably due to change of navigate. Is it correct? You can use React DevTools to investigate.

[CruseCtrl]

I'm not changing restoreOriginalUri. The Okta code is changing the value of restoreOriginalUri on the oktaAuth object that I'm passing in on line 54 of Security.tsx.

Edit: To be clear, the restoreOriginalUri prop that I'm passing into <Security restoreOriginalUri={restoreOriginalUri} /> isn't changing. The only thing that is changing is the restoreOriginalUri on oktaAuth.options.restoreOriginalUri, which is being changed by the Okta Security component

[denysoblohin-okta]

Unlike react-router v5, react-router v6 will re-render router on each location change. react-router v6 will change navigate function (returned from useNavigate hook) after each location change. See https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/hooks.tsx#L190 Thus callbacks passed to <Security> (restoreOriginalUri and onAuthRequired) will change too, if memoized with dependency on navigate. This leads to unnecessary side effects like multiple calls to useEffect of <Security>

It's safe to remove dependency from navigate.

  const restoreOriginalUri = useCallback(
    (oktaAuth: OktaAuth, originalUri: string) => {
      navigate(toRelativeUrl(originalUri || '/', window.location.origin), {
        replace: true,
      })
    },
    [],
  )

Some explanation of why it's safe: In fact, useNavigate callback depends on several contexts - navigation context, route context and location context. Location context value change should not affect on navigation to originalUri because it's absolute (starts with /). And navigation context value (navigator, basepath) should not change in real application.

[denysoblohin-okta]

Internal ref: OKTA-500147

[vdzerakh]

Unlike react-router v5, react-router v6 will re-render router on each location change. react-router v6 will change navigate function (returned from useNavigate hook) after each location change. See https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/hooks.tsx#L190 Thus callbacks passed to <Security> (restoreOriginalUri and onAuthRequired) will change too, if memoized with dependency on navigate. This leads to unnecessary side effects like multiple calls to useEffect of <Security>

It's safe to remove dependency from navigate.

  const restoreOriginalUri = useCallback(
    (oktaAuth: OktaAuth, originalUri: string) => {
      navigate(toRelativeUrl(originalUri || '/', window.location.origin), {
        replace: true,
      })
    },
    [],
  )

Some explanation of why it's safe: In fact, useNavigate callback depends on several contexts - navigation context, route context and location context. Location context value change should not affect on navigation to originalUri because it's absolute (starts with /). And navigation context value (navigator, basepath) should not change in real application.

Hi, even though I've added a useCallback to restoreOriginalUri, and removed navigate from dependencies, the warning in the console still crops up. The creating of oktaAuth is located in the main index.ts file

Do you have any ideas why the recommendations don't work?

[RikkiMasters]

I fixed it by disabling React strict mode. (Remove from index.js)

Strict mode will intentionally render every component twice.

[ryanmr]

I noticed the same thing as vdzerakh mentioned above. Even with the useCallback around the restore function, and without the navigate in the dependnency list, the warning still comes back up. With React 18 and Strict Mode, these future docs are incomplete. Disabling StrictMode isn't ideal.

I think the problem here is that Strict Mode enforces double renders in development so that you can catch side effects.

  React.useEffect(() => {
    if (!oktaAuth || !restoreOriginalUri) {
      return;
    }

    // Add default restoreOriginalUri callback
    if (oktaAuth.options.restoreOriginalUri && restoreOriginalUri) {
      console.warn('Two custom restoreOriginalUri callbacks are detected. The one from the OktaAuth configuration will be overridden by the provided restoreOriginalUri prop from the Security component.');
    }
    oktaAuth.options.restoreOriginalUri = (async (oktaAuth: unknown, originalUri: string) => {
      restoreOriginalUri(oktaAuth as OktaAuth, originalUri);
    }) as ((oktaAuth: OktaAuth, originalUri?: string) => Promise<void>);

  }, [oktaAuth, restoreOriginalUri]);

This useEffect triggers the first time, and the warning does not trigger, since the condition isn't true yet. But then on the second render (the one from Strict Mode), it triggers because the condition oktaAuth.options.restoreOriginalUri && restoreOriginalUri is true now, from having been set one useEffect prior, oktaAuth.options.restoreOriginalUri =.

I'm still thinking about alternative approaches to this.

Follow up

I did some rudimentary testing, and I came up with this:

return () => {
      oktaAuth.options.restoreOriginalUri = undefined;
};

This is a clean up function from the return of the useEffect. Whenever the <Security> re-renders now, either because its parent changes, its props change, or its internal state change, the useEffect will run, and so will this cleanup. When the cleanup runs, it sets the options.restoreOriginalUri back to undefined, so that on the next render the warning conditions are not true. Unfortunately, this does not fix the case where a user supplies an options.restoreOriginalUri callback in the OktaAuth config. I think that is OK given that with okta-react and how the <Security> component blindly overwrites the options.restoreOriginalUri anyway, the prop is the source of truth.

[kellengreen]

Temp solution I'm using in the meantime:

import { Security } from '@okta/okta-react'
import { useCallback, useEffect, useRef } from 'react'
import { useNavigate } from 'react-router-dom'
import { toRelativeUrl } from '@okta/okta-auth-js'

export default function Root() {
  const navigate = useNavigate()
  const navigateRef = useRef(navigate)

  // Allow for "stale" navigate references since originalUri will be an absolute URL.
  const restoreOriginalUri = useCallback((_, originalUri = '/') => {
    const url = toRelativeUrl(originalUri, globalThis.location.origin)
    navigateRef.current(url)
  }, [])

  useEffect(() => {
    return () => {
      oktaAuth.options.restoreOriginalUri = undefined
    }
  }, [])

  return (
    <Security oktaAuth={oktaAuth} restoreOriginalUri={restoreOriginalUri}>
      ...
    </Security>
  )
}

[pzi]

https://github.com/okta/okta-react/pull/239 seems to fix this.

[kellengreen]

If I remove the above fix:

useEffect(() => {
  return () => {
    oktaAuth.options.restoreOriginalUri = undefined
  }
}, [])

I'm still seeing this error with @okta/okta-react=6.6.0, @okta/okta-auth-js=6.7.2, and react-router-dom=6.3.0:

Two custom restoreOriginalUri callbacks are detected. The one from the OktaAuth configuration will be overridden by the provided restoreOriginalUri prop from the Security component.

(anonymous) @ Security.tsx:54
commitHookEffectListMount @ react-dom.development.js:23150
invokePassiveEffectMountInDEV @ react-dom.development.js:25154
invokeEffectsInDev @ react-dom.development.js:27351
commitDoubleInvokeEffectsInDEV @ react-dom.development.js:27330
flushPassiveEffectsImpl @ react-dom.development.js:27056
flushPassiveEffects @ react-dom.development.js:26984
performSyncWorkOnRoot @ react-dom.development.js:26076
flushSyncCallbacks @ react-dom.development.js:12042
commitRootImpl @ react-dom.development.js:26959
commitRoot @ react-dom.development.js:26682
finishConcurrentRender @ react-dom.development.js:25981
performConcurrentWorkOnRoot @ react-dom.development.js:25809
workLoop @ scheduler.development.js:266
flushWork @ scheduler.development.js:239
performWorkUntilDeadline @ scheduler.development.js:533

I've also updated my "SecureRoute" to reflect the useEffect changes made in 6.6.0

export default function SecureRoute() {
  const { oktaAuth, authState } = useOktaAuth()

  useEffect(() => {
    if (authState?.isAuthenticated === false) {
      const originalUri = toRelativeUrl(
        window.location.href,
        window.location.origin,
      )
      oktaAuth.setOriginalUri(originalUri)
      oktaAuth.signInWithRedirect()
    }
  }, [oktaAuth, authState?.isAuthenticated])

  if (authState?.isAuthenticated === true) {
    return <Outlet />
  } else {
    return <FullPageLoading />
  }
}

[sridharan-santhanam]

@kellengreen @ryanmr - Thanks for the solutions.

[phani1585]

@kellengreen / @ryanmr , do you have any idea whats need to be done in the react app when we doing the single sign out from the okta dashboard

[jaredperreault-okta]

@phani1585 Can you please create a new issue, your question doesn't seem relevant to this thread

[kellengreen]

@phani1585 sorry I'm not sure if I understand your question.

For what it's worth, I don't recall seeing this warning in recent releases.