- 
                Notifications
    You must be signed in to change notification settings 
- Fork 403
chore(backend): Drop deprecations #1899
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
Conversation
| 🦋 Changeset detectedLatest commit: 53bef9c 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 | 
505650d    to
    57684e7      
    Compare
  
    4d64e56    to
    f95ab26      
    Compare
  
    f95ab26    to
    526a49c      
    Compare
  
    c9f2381    to
    18dabbc      
    Compare
  
    | const noSchemeFrontendApi = frontendApi.replace(/http(s)?:\/\//, ''); | ||
| const major = getClerkJsMajorVersionOrTag(frontendApi, pkgVersion); | ||
| const major = getClerkJsMajorVersionOrTag(frontendApi, clerkJSVersion); | ||
| return `https://${noSchemeFrontendApi}/npm/@clerk/clerk-js@${clerkJSVersion || major}/dist/clerk.browser.js`; | 
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.
@nikosdouvlis Since we are dropping the pkgVersion we won't be able to load the major version of a version provided implicitly because the clerkJSVersion || major where the major version is being calculated using the clerkJSVersion.
This is the reason behind dropping the test in packages/shared/src/__tests__/url.test.ts.
We will be able to use:
- the latestorcanarytag or
- the explicit version passed
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.
I'm not sure I undertand why we are making this change. Could you please share more context? We need to be able to load version 4 for example, otherwise each clerk-react major version will not be able to load the correct clerk-js package.
We cannot rely on latest for this - what will happen when we release v6?
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.
@nikosdouvlis  the clerk-react does not use this script to load the clerk-js. This is used to load ClerkJS in interstitial and it seems to be an existing issue that needs to be solved.
I will merge this an open a ticket to validate and handle the clerkJS version loaded in the interstitial
a6d5666    to
    6b8d70b      
    Compare
  
    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.
A few minor comments and a question - appart from that, everything looks good. Approving in order to unblock!
| const noSchemeFrontendApi = frontendApi.replace(/http(s)?:\/\//, ''); | ||
| const major = getClerkJsMajorVersionOrTag(frontendApi, pkgVersion); | ||
| const major = getClerkJsMajorVersionOrTag(frontendApi, clerkJSVersion); | ||
| return `https://${noSchemeFrontendApi}/npm/@clerk/clerk-js@${clerkJSVersion || major}/dist/clerk.browser.js`; | 
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.
I'm not sure I undertand why we are making this change. Could you please share more context? We need to be able to load version 4 for example, otherwise each clerk-react major version will not be able to load the correct clerk-js package.
We cannot rely on latest for this - what will happen when we release v6?
6b8d70b    to
    92bbeb9      
    Compare
  
    … `SetClerkSecretKey`
- `ExternalAccount.picture` - `ExternalAccountJSON.avatar_url` - `Organization.logoUrl` - `OrganizationJSON.logo_url` - `User.profileImageUrl` - `UserJSON.profile_image_url` - `OrganizationMembershipPublicUserData.profileImageUrl` - `OrganizationMembershipPublicUserDataJSON.profile_image_url`
The `as string` silents the typescript error but in the runtime the code fails if the value of `null` or `undefined` is passed in a function that expects a string to be passed. Failures may be caused by invoking string specific methods (eg `startsWith`) which raise a `Cannot read properties of undefined` error.
92bbeb9    to
    53bef9c      
    Compare
  
    
Description
Drop deprecated properties. Migration steps:
createClerkClientinstead of__unstable_optionspublishableKeyinstead offrontendApiclockSkewInMsinstead ofclockSkewInSecondsapiKeyinstead ofsecretKeyhttpOptions*.imageinstead ofExternalAccount.pictureExternalAccountJSON.avatar_urlOrganization.logoUrlOrganizationJSON.logo_urlUser.profileImageUrlUserJSON.profile_image_urlOrganizationMembershipPublicUserData.profileImageUrlOrganizationMembershipPublicUserDataJSON.profile_image_urlpkgVersionOrganization.getOrganizationInvitationListwithstatusinstead ofgetPendingOrganizationInvitationListorgsclaim (if required, can be manually added by usinguser.organizationsin a jwt template)Internal changes:
SetClerkSecretKeyOrAPIKeywithSetClerkSecretKeyTODO
Checklist
npm testruns as expected.npm run buildruns as expected.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/chore