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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

desiprisg
Copy link
Member

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:

Copy link

changeset-bot bot commented Mar 11, 2024

馃 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
Member 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
Member

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

@desiprisg
Copy link
Member 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
Member

@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
Copy link
Member

dimkl commented Mar 11, 2024

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.

/**
* JSON types
*/
export type * from './api/resources/JSON';
Copy link
Member

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

Copy link
Member Author

@desiprisg desiprisg Mar 12, 2024

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
9 checks passed
@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.

None yet

5 participants