feat(backend): Introduce createIsomorphicRequest#1393
Conversation
🦋 Changeset detectedLatest commit: ee55518 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
anagstef
changed the title
createIsomorphicRequestcreateIsomorphicRequest
| * @experimental | ||
| */ | ||
| signInUrl?: string; | ||
| request?: Request; |
There was a problem hiding this comment.
This needs to be turned into a new signature - lets take this offline
anagstef
changed the title
createIsomorphicRequestcreateIsomorphicRequest
048725d to
33ae931
Compare
| forwardedProto: | ||
| options.forwardedProto || isomorphicRequest?.headers?.get(constants.Headers.ForwardedProto) || undefined, | ||
| referrer: options.referrer || isomorphicRequest?.headers?.get(constants.Headers.Referrer) || undefined, | ||
| userAgent: options.userAgent || isomorphicRequest?.headers?.get(constants.Headers.UserAgent) || undefined, |
There was a problem hiding this comment.
🙃 could we create a helper method for the repetitive parts to reduce the length of each size and make it more readable?
Example:
const getFromReq = (req, key) => req?.headers?.get(key) || undefined ;
// usage
options = {
// ...
forwardedProto: options.forwardedProto || getFromReq(isomorphicRequest, constants.Headers.ForwardedProto)
//...
}| return value?.split(',')[0]?.trim() || ''; | ||
| } | ||
|
|
||
| const isRelativeUrl = (url: string) => { |
There was a problem hiding this comment.
🙃 let's add a comment for this one to move it to shared package or somewhere else and re-use it from there. I believe we have that code somewhere too.
|
|
||
| type IsomorphicRequestOptions = (Request: Request, Headers: Headers) => Request; | ||
|
|
||
| export const createIsomorphicRequest = (cb: IsomorphicRequestOptions): Request => { |
There was a problem hiding this comment.
❓ Why are we calling this an IsomorphicRequest since it's just the global.Request?
There was a problem hiding this comment.
I recommended the IsomorphicRequest name because it's a Request that can run on any platform, even platforms that natively do not support global.Request (older node runtimes etc).
Would createRequest make more sense to you? I didn't choose this one because it doesn't make the difference between new Request and createRequest apparent. Happy to change the name though :)
| import { clerkClient } from './clerkClient'; | ||
| import * as constants from './constants'; | ||
| import type { ClerkFastifyOptions } from './types'; | ||
| import { getSingleValueFromArrayHeader } from './utils'; |
There was a problem hiding this comment.
🔧 This also feels it belongs to the request utility
There was a problem hiding this comment.
What do you mean exactly? This is the fastify package importing the utility from backend as we have designed with @nikosdouvlis
| domain, | ||
| searchParams: new URL(request.url).searchParams, | ||
| signInUrl, | ||
| request: createIsomorphicRequest((Request: any, Headers: any) => { |
There was a problem hiding this comment.
❓ Similar to my other comment, why don't we just pass req directly?
There was a problem hiding this comment.
We do some sanitization actions first for the Request URL. That's why we need to create it ourselves.
8fc6fed to
a06196f
Compare
0473ca4 to
c34134e
Compare
![jit-ci[bot]](https://avatars.githubusercontent.com/in/142441?s=60&v=4){kind=small}
c34134e to
04045b1
Compare
Introducing the new utility `createIsomorphicRequest` for the `@clerk/backend` package, so that the `authenticateRequest` signature will be more simplified, and it will be easier to integrate with more frameworks.
… createIsomorphicRequest chore(backend): Add changeset for `createIsomorphicRequest` refactor(clerk-sdk-node,backend): Remove sdk-node cookie dependency Also refactor authenticateRequest options fix(backend): Refactor the new IsomorphicRequest utilities Also, manipulate headers objects to be compatible with Headers constructor chore(repo): Revert `package-lock.json` changes
04045b1 to
ee55518
Compare
|
@anagstef, this change broke my setup (AWS amplify serverless express lambdas) I rolled back to previous version (4.10.15) and everything worked again.
|
|
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
6 participants
Type of change
Packages affected
@clerk/clerk-js@clerk/clerk-react@clerk/nextjs@clerk/remix@clerk/types@clerk/themes@clerk/localizations@clerk/clerk-expo@clerk/backend@clerk/clerk-sdk-node@clerk/shared@clerk/fastify@clerk/chrome-extensiongatsby-plugin-clerkbuild/tooling/choreDescription
npm testruns as expected.npm run buildruns as expected.Introducing the new utility
createIsomorphicRequestfor the@clerk/backendpackage, so that theauthenticateRequestsignature will be more simplified, and it will be easier to integrate with more frameworks.