feat(remix): Support new redirect urls as environment variables or options to rootAuthLoader #3442
Conversation
…tions to rootAuthLoader As options - signInForceRedirectUrl - signUpForceRedirectUrl - signInFallbackRedirectUrl - signUpFallbackRedirectUrl As environment variables - CLERK_SIGN_IN_FORCE_REDIRECT_URL - CLERK_SIGN_UP_FORCE_REDIRECT_URL - CLERK_SIGN_IN_FALLBACK_REDIRECT_URL - CLERK_SIGN_UP_FALLBACK_REDIRECT_URL
🦋 Changeset detectedLatest commit: 5753a31 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const _requestState = await authenticateRequest(args, loadedOptions); | ||
| const requestState = { ...loadedOptions, ..._requestState }; |
There was a problem hiding this comment.
❓Should @clerk/backend be aware of these new redirect urls ? Seems like afterXUrl are being returned as part of the SignedIn/OutState
There was a problem hiding this comment.
❓ Aren't those properties only available in the client? I believe that the only reason the afterXUrl exist in the backend is for debugging.
@nikosdouvlis Do you have more context?
There was a problem hiding this comment.
Yes, i would expect them to only be used in the client as well.
Do you think we should update add the new urls in the request state for usage in debugging as well ? Or is it ok to leave the PR as it is now ?
There was a problem hiding this comment.
Why wouldn't we want them in loadedOptions? I think we can safely add them - improved debuggability is a welcome side effect
| @@ -80,6 +84,10 @@ export function ClerkProvider({ children, ...rest }: ClerkProviderPropsWithState | |||
| signUpUrl: __signUpUrl, | |||
| afterSignInUrl: __afterSignInUrl, | |||
| afterSignUpUrl: __afterSignUpUrl, | |||
There was a problem hiding this comment.
❓ Do we still need these or can we mark them as deprecated?
| const _requestState = await authenticateRequest(args, loadedOptions); | ||
| const requestState = { ...loadedOptions, ..._requestState }; |
There was a problem hiding this comment.
Why wouldn't we want them in loadedOptions? I think we can safely add them - improved debuggability is a welcome side effect
nikosdouvlis
deleted the
elef/sdk-1719-support-the-new-redirect-url-envs-in-remix
branch
Description
As options
As environment variables
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change