-
Notifications
You must be signed in to change notification settings - Fork 462
refactor(clerk-js,nextjs,types): Add Autocomplete generic [SDK-900] #2132
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@clerk/clerk-js': patch | ||
| '@clerk/nextjs': patch | ||
| '@clerk/types': patch | ||
| --- | ||
|
|
||
| Add Autocomplete TS generic for union literals |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import type { | ||
| Autocomplete, | ||
| ClerkPaginatedResponse, | ||
| ClerkResourceReloadParams, | ||
| GetUserOrganizationMembershipParams, | ||
|
|
@@ -20,9 +21,7 @@ export class OrganizationMembership extends BaseResource implements Organization | |
| /** | ||
| * @experimental The property is experimental and subject to change in future releases. | ||
| */ | ||
| // Adding (string & {}) allows for getting eslint autocomplete but also accepts any string | ||
| // eslint-disable-next-line | ||
| permissions: (OrganizationPermission | (string & {}))[] = []; | ||
| permissions: Autocomplete<OrganizationPermission>[] = []; | ||
| role!: MembershipRole; | ||
| createdAt!: Date; | ||
| updatedAt!: Date; | ||
|
|
@@ -41,8 +40,16 @@ export class OrganizationMembership extends BaseResource implements Organization | |
| search: convertPageToOffset({ ...retrieveMembershipsParams, paginated: true }) as any, | ||
| }) | ||
| .then(res => { | ||
| if (!res?.response) { | ||
| return { | ||
| total_count: 0, | ||
| data: [], | ||
| }; | ||
| } | ||
|
|
||
| // TODO: Fix typing | ||
| const { data: suggestions, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; | ||
| res.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; | ||
|
|
||
| return { | ||
| total_count, | ||
|
|
@@ -99,9 +106,16 @@ export class OrganizationMembership extends BaseResource implements Organization | |
| }, | ||
| { forceUpdateClient: true }, | ||
| ); | ||
| const currentMembership = (json?.response as unknown as OrganizationMembershipJSON[]).find( | ||
|
|
||
| if (!json?.response) { | ||
| return this.fromJSON(null); | ||
| } | ||
|
|
||
| // TODO: Fix typing | ||
| const currentMembership = (json.response as unknown as OrganizationMembershipJSON[]).find( | ||
|
Comment on lines
+109
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
| orgMem => orgMem.id === this.id, | ||
| ); | ||
|
|
||
| return this.fromJSON(currentMembership as OrganizationMembershipJSON); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ import type { OrganizationResource } from './organization'; | |
| import type { MembershipRole } from './organizationMembership'; | ||
| import type { ActiveSessionResource } from './session'; | ||
| import type { UserResource } from './user'; | ||
| import type { DeepPartial, DeepSnakeToCamel } from './utils'; | ||
| import type { Autocomplete, DeepPartial, DeepSnakeToCamel } from './utils'; | ||
|
|
||
| export type InstanceType = 'production' | 'development'; | ||
|
|
||
|
|
@@ -805,7 +805,7 @@ type PrimitiveKeys<T> = { | |
| [K in keyof T]: T[K] extends string | boolean | number | null ? K : never; | ||
| }[keyof T]; | ||
|
|
||
| type LooseExtractedParams<T extends string> = `:${T}` | (string & NonNullable<unknown>); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It ultimately produces the same result. |
||
| type LooseExtractedParams<T extends string> = Autocomplete<`:${T}`>; | ||
|
|
||
| export type OrganizationSwitcherProps = { | ||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come this is a necessary change ? I believe you are using this logic in more places, shall we update the rest as well ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@panteliselef I was running into build issues, per typing. The
_fetchreturns (and its generic extends)ClerkResourceJSON | DeletedObjectJSON | nullwhich doesn't fit the desiredClerkPaginatedResponse<OrganizationMembershipJSON>type.We might want to expand upon this, however, I didn't feel this was the PR to do it.