Skip to content
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(clerk-js): Drop deprecations #2082

Merged
merged 23 commits into from
Nov 10, 2023
Merged

Conversation

dimkl
Copy link
Member

@dimkl dimkl commented Nov 8, 2023

Description

This PR contains also changes in react, expo, chrome-extension, types, shared, nextjs packages due to types being changed or to code being exported or re-used by other packages.

Drop deprecations. Migration steps:

  • use publishableKey instead of frontendApi
  • use Clerk.handleEmailLinkVerification() instead of Clerk.handleMagicLinkVerification()
  • use EmailLinkError instead of MagicLinkError
  • use isEmailLinkError instead of isMagicLinkError
  • use EmailLinkErrorCode instead of MagicLinkErrorCode
  • use useEmailLink instead of useMagicLink
  • drop orgs jwt claim from session token
  • use ExternalAccount.imageUrl instead of ExternalAccount.avatarUrl
  • use Organization.imageUrl instead of Organization.logoUrl
  • use User.imageUrl instead of User.profileImageUrl
  • use OrganizationMembershipPublicUserData.imageUrl instead of OrganizationMembershipPublicUserData.profileImageUrl
  • use useOrganizationList instead of useOrganizations
  • use userProfileProps instead of userProfile in Appearance
  • use Clerk.setActive() instead of Clerk.setSession()
  • drop password param in User.update()
  • use afterSelectOrganizationUrl instead of afterSwitchOrganizationUrl in OrganizationSwitcher
  • drop Clerk.experimental_canUseCaptcha / Clerk.Clerk.experimental_captchaSiteKey / Clerk.experimental_captchaURL (were meant for internal use)
  • use User.getOrganizationMemberships() instead of Clerk.getOrganizationMemberships()
  • drop lastOrganizationInvitation / lastOrganizationMember from Clerk emitted events
  • drop Clerk.__unstable__invitationUpdate / Clerk.__unstable__membershipUpdate
  • drop support for string param in Organization.create()
  • use Organization.getInvitations() instead of Organization.getPendingInvitations()
  • use pageSize instead of limit in OrganizationMembership.retrieve()
  • use initialPage instead of offset in OrganizationMembership.retrieve()
  • drop lastOrganizationInvitation / lastOrganizationMember from ClerkProvider
  • use invitations instead of invitationList in useOrganization
  • use memberships instead of membershipList in useOrganization
  • use redirectUrl instead of redirect_url in User.createExternalAccount()
  • use signature instead of generatedSignature in Signup.attemptWeb3WalletVerification()

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/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore

@dimkl dimkl self-assigned this Nov 8, 2023
@dimkl dimkl requested a review from a team as a code owner November 8, 2023 12:53
Copy link

changeset-bot bot commented Nov 8, 2023

🦋 Changeset detected

Latest commit: c195500

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

This PR includes changesets to release 12 packages
Name Type
@clerk/chrome-extension Major
@clerk/clerk-js Major
@clerk/nextjs Major
@clerk/shared Major
@clerk/clerk-react Major
@clerk/types Major
@clerk/clerk-expo Major
@clerk/backend Patch
@clerk/fastify Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
gatsby-plugin-clerk 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
Copy link
Member Author

dimkl commented Nov 8, 2023

@panteliselef The only deprecations still in the clerk-js package are related to the following components:

  • Form.tsx: Control is deprecated in favor of Form.PlainInput
  • RadioGroup.tsx: RadioGroup and RadioGroupItem are being deprecated
  • FormControl.tsx: FormControl is deprecated in favor of Field.Root
  • useFormControl.tsx: has some deprecations

How should we proceed with those?

@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch from 1e4821b to 879bf78 Compare November 8, 2023 13:01
@panteliselef
Copy link
Member

@panteliselef The only deprecations still in the clerk-js package are related to the following components:

  • Form.tsx: Control is deprecated in favor of Form.PlainInput
  • RadioGroup.tsx: RadioGroup and RadioGroupItem are being deprecated
  • FormControl.tsx: FormControl is deprecated in favor of Field.Root
  • useFormControl.tsx: has some deprecations

How should we proceed with those?

@dimkl you are correct, these are marked as deprecated as removing them completely and introducing new ones would result an a PR with a lot of changes what would make the reviewal process almost impossible.

I hope i will tackle this by end of this week, but also want to mention that this does not really affect external APIs, and anything that you listed as deprecated is for internal use only

@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch 2 times, most recently from 95213c4 to 85aec82 Compare November 9, 2023 09:37
@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch from 85aec82 to c735882 Compare November 9, 2023 10:54
@@ -11,7 +11,7 @@ export interface JWTHeader {
alg: string | Algorithm;
typ?: string;
cty?: string;
crit?: Array<string | Exclude<keyof JWTHeader, 'crit'>>;
crit?: Array<string | Exclude<JWTHeader, 'crit'>>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean previously this was broken ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@panteliselef based on the RFC: https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.11
The crit header parameter

  • should NOT be an empty array
  • should not include crit (not sure about this but it's already defined)

The above change does not fix it, so i will update it to crit: NonEmptyArray<Exclude<keyof JWTHeader, 'crit'>>; to resolve this. I did some tests and it passes.

PS: type NonEmptyArray<T> = [T, ...T[]];

@@ -8,8 +8,8 @@ describe('Organization', () => {
name: 'test_name',
public_metadata: { public: 'metadata' },
slug: 'test_slug',
logo_url: 'https://url-for-logo.png',
image_url: 'https://clerk.com',
image_url:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Formatting

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is made by the npm run format script based on our rules.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems weird 🤔

packages/clerk-js/README.md Show resolved Hide resolved
packages/shared/src/keys.ts Show resolved Hide resolved
@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch from c735882 to fda0693 Compare November 9, 2023 17:40
@dimkl dimkl enabled auto-merge November 9, 2023 17:40
@dimkl
Copy link
Member Author

dimkl commented Nov 9, 2023

@LekoArts , @panteliselef i have addressed your comments. PTAL (also review & resolve the comments)

@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch 2 times, most recently from 8231284 to 0ebecdf Compare November 9, 2023 19:18
@dimkl dimkl mentioned this pull request Nov 9, 2023
24 tasks
- `ExternalAccount.avatarUrl`
- `ExternalAccountJSON.avatar_url`
- `Organization.logoUrl`
- `OrganizationJSON.logo_url`
- `User.profileImageUrl`
- `UserJSON.profile_image_url`
- `OrganizationMembershipPublicUserData.profileImageUrl`
- `OrganizationMembershipPublicUserDataJSON.profile_image_url`
@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch from 0ebecdf to defbbd0 Compare November 10, 2023 08:53
@dimkl dimkl force-pushed the sdk-782-drop-deprecations-clerk-js branch from defbbd0 to c195500 Compare November 10, 2023 08:57
Copy link
Member

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not run/test this locally but looking over the changes in GitHub this LGTM

@dimkl dimkl added this pull request to the merge queue Nov 10, 2023
Merged via the queue into main with commit 7f833da Nov 10, 2023
6 checks passed
@dimkl dimkl deleted the sdk-782-drop-deprecations-clerk-js branch November 10, 2023 10:10
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.

None yet

4 participants