Skip to content

refactor(clerk-js,nextjs,types): Add Autocomplete generic [SDK-900] - #2132

Merged
tmilewski merged 1 commit into
mainfrom
sdk-900-union-literal-autocomplete
Nov 14, 2023
Merged

refactor(clerk-js,nextjs,types): Add Autocomplete generic [SDK-900]#2132
tmilewski merged 1 commit into
mainfrom
sdk-900-union-literal-autocomplete

Conversation

@tmilewski

Copy link
Copy Markdown
Member

Description

Adds Autocomplete Typescript generic util.

SDK-900

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

@tmilewskitmilewski self-assigned this Nov 14, 2023
@tmilewski
tmilewski requested a review from a team as a code ownerNovember 14, 2023 14:52
@changeset-bot

changeset-botBot commented Nov 14, 2023

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 54171c2

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

This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsPatch
@clerk/nextjsPatch
@clerk/typesPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
gatsby-plugin-clerkPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch

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

Comment threadpackages/clerk-js/package.json Outdated
@tmilewski
tmilewskiforce-pushed the sdk-900-union-literal-autocomplete branch from 39bf95a to 54171c2CompareNovember 14, 2023 16:02
[K in keyof T]: T[K] extends string | boolean | number | null ? K : never;
}[keyof T];

type LooseExtractedParams<T extends string> = `:${T}` | (string & NonNullable<unknown>);

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.

Is NonNullable<unknown> the same as Record<never, never> ?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It ultimately produces the same result.

Comment on lines +43 to +52
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>;

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.

How come this is a necessary change ? I believe you are using this logic in more places, shall we update the rest as well ?

Copy link
Copy Markdown
MemberAuthor

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 _fetch returns (and its generic extends) ClerkResourceJSON | DeletedObjectJSON | null which doesn't fit the desired ClerkPaginatedResponse<OrganizationMembershipJSON> type.

We might want to expand upon this, however, I didn't feel this was the PR to do it.

Comment on lines +109 to +115

if (!json?.response) {
return this.fromJSON(null);
}

// TODO: Fix typing
const currentMembership = (json.response as unknown as OrganizationMembershipJSON[]).find(

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.

ditto

@tmilewski
tmilewski added this pull request to the merge queue Nov 14, 2023
Merged via the queue into main with commit f77e8cdNov 14, 2023
@tmilewski
tmilewski deleted the sdk-900-union-literal-autocomplete branch November 14, 2023 18:34
Sign up for freeto 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.

4 participants

@tmilewski@dimkl@panteliselef@clerk-cookie