Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Improve Templates type #2499

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Conversation

LucasGarcez
Copy link
Contributor

What

The predefined template has a generic type, so the TypeScript and autocomplete can't get a typo issue. This PR defines a literal type to register all keys and help users when using Templates.

Changes

This PR changes the Templates const type.

Tested on

image

Related issues

Copy link

vercel bot commented Feb 2, 2024

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

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 2, 2024 9:24am

Copy link
Owner

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

Ah, lol whoops, nice catch!! thanks for the PR.

One question; could we maybe use as const at the end of the Templates declaration instead of your approach?

I'm trying to find a way that would not require me to write the keys (Video, Video60Fps, ...) twice, but instead only once and have the value type (FormatFilter[]) still be typed...?

@LucasGarcez
Copy link
Contributor Author

Ah, lol whoops, nice catch!! thanks for the PR.

One question; could we maybe use as const at the end of the Templates declaration instead of your approach?

I'm trying to find a way that would not require me to write the keys (Video, Video60Fps, ...) twice, but instead only once and have the value type (FormatFilter[]) still be typed...?

Thanks for the quick answer. It would be great to declare the keys just once, but as far as I know, there is no resource on TypeScript to do it. If you check the TypeScript documentation for the Record utility the example is exactly the same as our use case here. So, I assume Record<Keys, Type> is the best approach.

@mrousavy
Copy link
Owner

mrousavy commented Feb 5, 2024

Great, LGTM then, thank you Lucas! ❤️

@mrousavy mrousavy enabled auto-merge (squash) February 5, 2024 11:36
@mrousavy mrousavy disabled auto-merge February 5, 2024 11:37
@mrousavy mrousavy merged commit cd5fdd4 into mrousavy:main Feb 5, 2024
4 checks passed
@LucasGarcez LucasGarcez deleted the PredefinedTemplates branch February 5, 2024 12:07
isaaccolson pushed a commit to isaaccolson/deliveries-mobile that referenced this pull request Oct 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants