Skip to content

chore(backend): Drop deprecations#1899

Merged
dimkl merged 15 commits into
mainfrom
sdk-782-drop-deprecations-backend
Nov 16, 2023
Merged

chore(backend): Drop deprecations#1899
dimkl merged 15 commits into
mainfrom
sdk-782-drop-deprecations-backend

Conversation

@dimkl

@dimkl dimkl commented Oct 17, 2023

Copy link
Copy Markdown
Contributor

Description

Drop deprecated properties. Migration steps:

  • use createClerkClient instead of __unstable_options
  • use publishableKey instead of frontendApi
  • use clockSkewInMs instead of clockSkewInSeconds
  • use apiKey instead of secretKey
  • drop httpOptions
  • use *.image instead of
    • ExternalAccount.picture
    • ExternalAccountJSON.avatar_url
    • Organization.logoUrl
    • OrganizationJSON.logo_url
    • User.profileImageUrl
    • UserJSON.profile_image_url
    • OrganizationMembershipPublicUserData.profileImageUrl
    • OrganizationMembershipPublicUserDataJSON.profile_image_url
  • drop pkgVersion
  • use Organization.getOrganizationInvitationList with status instead of getPendingOrganizationInvitationList
  • drop orgs claim (if required, can be manually added by using user.organizations in a jwt template)

Internal changes:

  • replaced error enum (and it's) SetClerkSecretKeyOrAPIKey with SetClerkSecretKey

TODO

  • drop Interstitial deprecation (will be done in a separate PR since we will probably keep only the local interstitial based on the SDK public APIs for v5 RFC)

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

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

@dimkl dimkl self-assigned this Oct 17, 2023
@changeset-bot

changeset-bot Bot commented Oct 17, 2023

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 53bef9c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@clerk/backend Major
@clerk/fastify Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch

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

@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch from 505650d to 57684e7 Compare October 19, 2023 13:32
@dimkl
dimkl marked this pull request as ready for review October 19, 2023 13:33
@dimkl
dimkl requested a review from a team as a code owner October 19, 2023 13:33
@dimkl
dimkl marked this pull request as draft October 19, 2023 13:33
@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch 3 times, most recently from 4d64e56 to f95ab26 Compare October 26, 2023 21:24
@dimkl dimkl changed the title chore(backend): Drop clerk@v4 deprecations chore(backend): Drop deprecations Oct 31, 2023
@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch from f95ab26 to 526a49c Compare November 7, 2023 12:52
@dimkl
dimkl marked this pull request as ready for review November 7, 2023 12:57
@dimkl dimkl closed this Nov 7, 2023
@dimkl dimkl reopened this Nov 7, 2023
@dimkl dimkl closed this Nov 7, 2023
@dimkl
dimkl deleted the sdk-782-drop-deprecations-backend branch November 7, 2023 13:27
@dimkl
dimkl restored the sdk-782-drop-deprecations-backend branch November 7, 2023 13:27
@dimkl
dimkl deleted the sdk-782-drop-deprecations-backend branch November 7, 2023 13:27
@dimkl
dimkl restored the sdk-782-drop-deprecations-backend branch November 7, 2023 13:28
@dimkl dimkl reopened this Nov 7, 2023
@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch 3 times, most recently from c9f2381 to 18dabbc Compare November 8, 2023 13:36
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`;

Copy link
Copy Markdown
Contributor Author

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 or canary tag or
  • the explicit version passed

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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

@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch 6 times, most recently from a6d5666 to 6b8d70b Compare November 15, 2023 12:53

@nikosdouvlis nikosdouvlis left a comment

Copy link
Copy Markdown
Member

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!

Comment thread packages/backend/src/tokens/factory.ts
Comment thread packages/backend/src/tokens/interstitialRule.ts Outdated
Comment thread packages/backend/src/tokens/interstitialRule.ts Outdated
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`;

Copy link
Copy Markdown
Member

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?

@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch from 6b8d70b to 92bbeb9 Compare November 15, 2023 14:16
dimkl added 15 commits November 16, 2023 12:46
- `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.
@dimkl
dimkl force-pushed the sdk-782-drop-deprecations-backend branch from 92bbeb9 to 53bef9c Compare November 16, 2023 10:47
@dimkl
dimkl added this pull request to the merge queue Nov 16, 2023
Merged via the queue into main with commit cace853 Nov 16, 2023
@dimkl
dimkl deleted the sdk-782-drop-deprecations-backend branch November 16, 2023 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants