Skip to content

Conversation

desiprisg
Copy link
Contributor

@desiprisg desiprisg commented May 27, 2024

Description

When installing a Clerk package that has @clerk/types as a dev dep, the types packages is usually not installed (depending on the package manager/ or the user's setup). I think this should be a normal dependency similar to how "@clerk/shared" is used across the monorepo.

Seems to fix the issue I noticed:
With latest:
image

WIth snapshot of this PR:
image

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:

@desiprisg desiprisg self-assigned this May 27, 2024
Copy link

changeset-bot bot commented May 27, 2024

🦋 Changeset detected

Latest commit: 727f01d

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

This PR includes changesets to release 14 packages
Name Type
@clerk/localizations Patch
@clerk/elements Patch
@clerk/clerk-sdk-node Patch
@clerk/backend Patch
@clerk/express Patch
@clerk/nextjs Patch
@clerk/shared Patch
@clerk/remix Patch
@clerk/clerk-expo Patch
@clerk/clerk-js Patch
@clerk/fastify Patch
@clerk/chrome-extension Patch
@clerk/clerk-react Patch
@clerk/testing 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

@LekoArts
Copy link
Contributor

What's the reason for doing this?

@nikosdouvlis
Copy link
Member

What's the reason for doing this?

@desiprisg noticed that when installing a Clerk package that has @clerk/types as a dev dep, the types packages is usually not installed (depending on the package manager/ or the user's setup). I think this should be a normal dependency similar to how "@clerk/shared" is used across the monorepo

@LekoArts
Copy link
Contributor

Makes sense 👍

Copy link
Member

@nikosdouvlis nikosdouvlis left a comment

Choose a reason for hiding this comment

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

Lets add a short description, fix the conflicts and merge :)

@dimkl
Copy link
Contributor

dimkl commented May 29, 2024

@nikosdouvlis Let's proceed with this but the @clekr/types does not seem to be the same case as the @clerk/shared since the types are only used in package development and the shared contains functionality meant to be used in the released package.
Also, can we add some examples of package managers for the @clerk/types fails when it's set as a dev dependency for posterity.

@LekoArts
Copy link
Contributor

LekoArts commented May 29, 2024

since the types are only used in package development and the shared contains functionality meant to be used in the released package.

This is only partially true. In some instances, yes, the TypeScript types from @clerk/types are solely for development purposes. But we export components and types that rely on @clerk/types.

For example:

export type SignInStrategyProps = { name: SignInStrategyName; children: React.ReactNode };

Source:

export type SignInStrategyName = SignInStrategy | 'oauth' | 'web3';

And that uses a type from @clerk/types

@desiprisg desiprisg marked this pull request as ready for review May 30, 2024 08:43
@desiprisg
Copy link
Contributor Author

!snapshot

@clerk-cookie
Copy link
Collaborator

Hey @desiprisg - the snapshot version command generated the following package versions:

Package Version
@clerk/backend 1.2.2-snapshot.v727f01d
@clerk/chrome-extension 1.0.17-snapshot.v727f01d
@clerk/clerk-js 5.5.4-snapshot.v727f01d
@clerk/elements 0.5.3-snapshot.v727f01d
@clerk/clerk-expo 1.1.9-snapshot.v727f01d
@clerk/express 0.0.11-snapshot.v727f01d
@clerk/fastify 1.0.13-snapshot.v727f01d
gatsby-plugin-clerk 5.0.0-beta.45
@clerk/localizations 2.4.4-snapshot.v727f01d
@clerk/nextjs 5.1.4-snapshot.v727f01d
@clerk/clerk-react 5.2.3-snapshot.v727f01d
@clerk/remix 4.0.14-snapshot.v727f01d
@clerk/clerk-sdk-node 5.0.10-snapshot.v727f01d
@clerk/shared 2.2.2-snapshot.v727f01d
@clerk/testing 1.1.6-snapshot.v727f01d

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/backend

npm i @clerk/backend@1.2.2-snapshot.v727f01d --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@1.0.17-snapshot.v727f01d --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.5.4-snapshot.v727f01d --save-exact

@clerk/elements

npm i @clerk/elements@0.5.3-snapshot.v727f01d --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@1.1.9-snapshot.v727f01d --save-exact

@clerk/express

npm i @clerk/express@0.0.11-snapshot.v727f01d --save-exact

@clerk/fastify

npm i @clerk/fastify@1.0.13-snapshot.v727f01d --save-exact

gatsby-plugin-clerk

npm i gatsby-plugin-clerk@5.0.0-beta.45 --save-exact

@clerk/localizations

npm i @clerk/localizations@2.4.4-snapshot.v727f01d --save-exact

@clerk/nextjs

npm i @clerk/nextjs@5.1.4-snapshot.v727f01d --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.2.3-snapshot.v727f01d --save-exact

@clerk/remix

npm i @clerk/remix@4.0.14-snapshot.v727f01d --save-exact

@clerk/clerk-sdk-node

npm i @clerk/clerk-sdk-node@5.0.10-snapshot.v727f01d --save-exact

@clerk/shared

npm i @clerk/shared@2.2.2-snapshot.v727f01d --save-exact

@clerk/testing

npm i @clerk/testing@1.1.6-snapshot.v727f01d --save-exact

@desiprisg
Copy link
Contributor Author

Seems to fix the issue I noticed:
With latest:
image

WIth snapshot of this PR:
image

@desiprisg desiprisg merged commit 02bed2e into main May 30, 2024
@desiprisg desiprisg deleted the clerk_types_dep branch May 30, 2024 08:58
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.

5 participants