Skip to content

feat(backend): Export the JSON types for clerk resources - #2965

Merged
nikosdouvlis merged 1 commit into
mainfrom
george/sdk-1489-export-the-json-types-from-clerkbackend
Mar 12, 2024
Merged

feat(backend): Export the JSON types for clerk resources#2965
nikosdouvlis merged 1 commit into
mainfrom
george/sdk-1489-export-the-json-types-from-clerkbackend

Conversation

@desiprisg

Copy link
Copy Markdown
Contributor

Description

Export the JSON types for clerk resources

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 requested review from dimkl and nikosdouvlis March 11, 2024 09:44
@changeset-bot

changeset-bot Bot commented Mar 11, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5bf1f61

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 Patch
@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

@desiprisg

Copy link
Copy Markdown
Contributor Author

Question for reviewers: Is there anything in JSON.ts that might not be beneficial to export?

@desiprisg
desiprisg force-pushed the george/sdk-1489-export-the-json-types-from-clerkbackend branch from fcaa0fd to 740f810 Compare March 11, 2024 10:09
@panteliselef

Copy link
Copy Markdown
Contributor

Should these be re-exported from the rest of "server" packages ? next, fastify, etc.

@desiprisg

Copy link
Copy Markdown
Contributor Author

Should these be re-exported from the rest of "server" packages ? next, fastify, etc.

I think we re-export everything by default, but I'll check,

@panteliselef

Copy link
Copy Markdown
Contributor

@desiprisg we are not. Check this we are very explicit about which types we return. So far we are just returning the classes as types. We probably need to do the same for the JSON types.

@dimkl

dimkl commented Mar 11, 2024

Copy link
Copy Markdown
Contributor

I would suggest we export explicitly only the types we want and avoid * top level exports from the @clerk/backend since it may pollute all of our other SDKs with internal types / utils exported by the JSON.ts meant ONLY to be used internally.
Let's stick to the more disciplined approach for now.

Comment thread packages/backend/src/index.ts Outdated
/**
* JSON types
*/
export type * from './api/resources/JSON';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔧 Let's export ONLY the types we want to and avoid * as it may result in unwanted exports in the future since the api/resources/JSON exports are considered to be consumed internally.

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.

@desiprisg PTAL at this one as we'd like to release today

@desiprisg desiprisg Mar 12, 2024

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.

Done, PTAL and let me know if there's anything we want to avoid exporting for now.

@desiprisg
desiprisg force-pushed the george/sdk-1489-export-the-json-types-from-clerkbackend branch from 740f810 to 5bf1f61 Compare March 12, 2024 12:06
@nikosdouvlis
nikosdouvlis merged commit 9272006 into main Mar 12, 2024
@nikosdouvlis
nikosdouvlis deleted the george/sdk-1489-export-the-json-types-from-clerkbackend branch March 12, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants