-
Notifications
You must be signed in to change notification settings - Fork 280
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
chore(backend): Drop deprecations #1899
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
latest
orcanary
tag 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:
createClerkClient
instead of__unstable_options
publishableKey
instead offrontendApi
clockSkewInMs
instead ofclockSkewInSeconds
apiKey
instead ofsecretKey
httpOptions
*.image
instead ofExternalAccount.picture
ExternalAccountJSON.avatar_url
Organization.logoUrl
OrganizationJSON.logo_url
User.profileImageUrl
UserJSON.profile_image_url
OrganizationMembershipPublicUserData.profileImageUrl
OrganizationMembershipPublicUserDataJSON.profile_image_url
pkgVersion
Organization.getOrganizationInvitationList
withstatus
instead ofgetPendingOrganizationInvitationList
orgs
claim (if required, can be manually added by usinguser.organizations
in a jwt template)Internal changes:
SetClerkSecretKeyOrAPIKey
withSetClerkSecretKey
TODO
Checklist
npm test
runs as expected.npm run build
runs 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-extension
gatsby-plugin-clerk
build/tooling/chore