-
Notifications
You must be signed in to change notification settings - Fork 371
feat(remix): Add Auth-Result response header #174
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,9 @@ | ||
import { json } from '@remix-run/server-runtime'; | ||
import cookie from 'cookie'; | ||
|
||
import { AuthData } from './getAuthData'; | ||
import { LoaderFunctionArgs, LoaderFunctionArgsWithAuth } from './types'; | ||
|
||
/** | ||
* Wraps obscured clerk internals with a readable `clerkState` key. | ||
* This is intended to be passed by the user into <ClerkProvider> | ||
* | ||
* @internal | ||
*/ | ||
export const wrapClerkState = (data: any) => { | ||
return { clerkState: { __internal_clerk_state: { ...data } } }; | ||
}; | ||
|
||
/** | ||
* Inject `auth`, `user` and `session` properties into `request` | ||
* @internal | ||
|
@@ -75,3 +66,46 @@ export function assertObject(val: any, error?: string): asserts val is Record<st | |
throw new Error(error || ''); | ||
} | ||
} | ||
|
||
/** | ||
* @internal | ||
*/ | ||
export const throwInterstitialJsonResponse = (opts: { frontendApi: string; errorReason: string | undefined }) => { | ||
throw json( | ||
wrapWithClerkState({ | ||
__clerk_ssr_interstitial: true, | ||
__frontendApi: opts.frontendApi, | ||
__lastAuthResult: opts.errorReason, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point! I think we should go with something like |
||
}), | ||
{ status: 401 }, | ||
); | ||
}; | ||
|
||
/** | ||
* @internal | ||
*/ | ||
export const returnLoaderResultJsonResponse = (opts: { | ||
authData: AuthData | null; | ||
frontendApi: string; | ||
errorReason: string | undefined; | ||
callbackResult?: any; | ||
}) => { | ||
return json({ | ||
...(opts.callbackResult || {}), | ||
...wrapWithClerkState({ | ||
__clerk_ssr_state: opts.authData, | ||
__frontendApi: opts.frontendApi, | ||
__lastAuthResult: opts.errorReason || '', | ||
}), | ||
}); | ||
}; | ||
|
||
/** | ||
* Wraps obscured clerk internals with a readable `clerkState` key. | ||
* This is intended to be passed by the user into <ClerkProvider> | ||
* | ||
* @internal | ||
*/ | ||
export const wrapWithClerkState = (data: any) => { | ||
return { clerkState: { __internal_clerk_state: { ...data } } }; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓
Is the
throw
needed here sincethrowInterstitialJsonResponse
throws on its own ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it's not needed per se, but I used it to further emphasize that we need to throw in order for the catch boundary to work - otherwise the SSR flow will break. It also helps TS infer that the execution stops there if the if is true, without using
return
I was worried that wrapping the
throwInterstitialJsonResponse
into another function that uses a try/catch block during a future refactor could potentially break the flow.Afaik immediately throwing a thrown error works as expected - but let me know if you have any objections