feat/Autocomplete Generics

[HugeLetters]

Linked Issue

Closes #1188

Description

Fixes generics on values and other props in Autocomplete component

Changsets

feat: autocomplete generics

Instructions: Changesets automate our changelog. If you modify files in /packages/skeleton, run pnpm changeset in the root of the monorepo, follow the prompts, then commit the markdown file. Changes that add features should be minor while chores and bugfixes should be patch. Please prefix the changeset message with feat:, bugfix: or chore:.

Checklist

Please read and apply all contribution requirements.

• This PR targets the dev branch (NEVER master)
• Documentation reflects all relevant changes
• Branch is prefixed with: docs/, feat/, chore/, bugfix/
• Ensure Svelte and Typescript linting is current - run pnpm check
• Ensure Prettier linting is current - run pnpm format
• All test cases are passing - run pnpm test
• Includes a changeset (if relevant; see above)

[changeset-bot]

⚠️ No Changeset found

Latest commit: bbaa2bf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

@HugeLetters is attempting to deploy a commit to the Skeleton Labs Team on Vercel.

A member of the Team first needs to authorize it.

[endigo9740]

Thanks for submitting the draft @HugeLetters.

@AdrianGonz97 and @ryceg are our resident Typescript experts, so I'm going to ping them here to provide feedback asap.

To make sure we're on the same page, I mentioned in the main thread this approach might be something we want to extend to Modals and other Skeleton features too. I'm fine if we want to take a piecemeal approach to that, as long as these aren't lost in the shuffle.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
skeleton-docs	✅ Ready (Inspect)	Visit Preview	Sep 11, 2023 6:40pm

[HugeLetters]

Thanks for submitting the draft @HugeLetters.

@AdrianGonz97 and @ryceg are our resident Typescript experts, so I'm going to ping them here to provide feedback asap.

To make sure we're on the same page, I mentioned in the main thread this approach might be something we want to extend to Modals and other Skeleton features too. I'm fine if we want to take a piecemeal approach to that, as long as these aren't lost in the shuffle.

Sure, once this gets approved and I get a better understanding of kind of code style is appreciated here I'll try to work on other components too.
Btw, what should the procedure be on minor improvements? Like here there's an unnecessary array copying or here when there's no limit set.

[endigo9740]

Btw, what should the procedure be on minor improvements? ...

We keep scope small and limited in PRs. This helps keep changes localized and not grow too large. For additional issues it's worth creating new issues and dedicated PRs for each change.

I will also advise you to browse our issue backlog. There's a number of issues folks have with the autocomplete component specifically. It's a feature implemented by me, but not one I've had a lot of prior experience with, so I think there's definitely some room for improvement. At this point we're reaching a point a more consorted effort to improve the feature top to bottom might be in order.

If you're not already on Discord, I might recommend joining and jumping into the #contributor channel. This is a great place to discuss and plan bigger ideas. That or creating a Discussions section here on GitHub. Once a set plan is established we can generate one or more issues against that.

[AdrianGonz97]

I love the proposed changes, great job! I pushed a couple of changes when it came to annotating types and some variable names.

One thing I wanted to mention is that since we're already modifying the type of value into a generic, we may as well do the same for meta.

Additionally, keywords looks to accept a string type, despite being labeled as any. Note the comment for it: /** Provide a comma separated list of keywords. */. What are your thoughts on this @endigo9740? I looked back in the original PR that introduced the component and didn't see any conversations about keeping keywords as any, so I'm assuming this is an any type that slipped between the cracks.

[endigo9740]

@AdrianGonz97 Per keywords, yes those should be a string ideally. Offhand I can't recall if we're parsing those, but they will be read as a string in the end.

[AdrianGonz97]

@endigo9740 Thanks! I thought as much. Looking at the code, we're not parsing it at all and the only time it's used is when everything is being strinigified for search - see here.

In that case, I think it's safe to change that any to a string then.

[endigo9740]

Hey @AdrianGonz97 not sure if you saw but @HugeLetters had a few follow up questions above. If we can get these addressed and give me a thumbs up when this is approved and ready to merge.

[AdrianGonz97]

Excellent changes and thanks for the update!

I've pushed a small change that adjusts the type for options so that it's appearance in the docs site makes a little more sense. And with that, everything else looks great and I think this PR is ready to be merged.

[HugeLetters]

Excellent changes and thanks for the update!

I've pushed a small change that adjusts the type for options so that it's appearance in the docs site makes a little more sense. And with that, everything else looks great and I think this PR is ready to be merged.

Great! Somebody needs to authorize Vercel's deployment to follow through.