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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): allow number index for hit attribute #1261 #1262

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

aldenquimby
Copy link
Contributor

Summary

Result

  • I am unable to run this project on my Apple Silicon Mac, so I am unable to confirm this works. If a maintainer could help verify, that would be much appreciated

@@ -11,7 +11,7 @@ type HighlightHitParams<THit> = {
*
* You can use the array syntax to reference nested attributes.
*/
attribute: keyof THit | string[];
attribute: keyof THit | (string | number)[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative would be to add @algolia/autocomplete-shared as a dependency in this example project, and import the existing type, so this doesn't need to be duplicated

Copy link

codesandbox-ci bot commented Jul 8, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0080a87:

Sandbox Source
@algolia/autocomplete-example-github-repositories-custom-plugin Configuration
@algolia/autocomplete-example-instantsearch Configuration
@algolia/autocomplete-example-playground Configuration
@algolia/autocomplete-example-preview-panel-in-modal Configuration
@algolia/autocomplete-example-starter-algolia Configuration
@algolia/autocomplete-example-starter Configuration
@algolia/autocomplete-example-reshape Configuration
@algolia/autocomplete-example-vue Configuration
@algolia/autocomplete-example-playground Issue #1261

@@ -1,4 +1,4 @@
export type ParseAlgoliaHitParams<TItem> = {
hit: TItem;
attribute: keyof TItem | string[];
attribute: keyof TItem | (string | number)[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative, if preferred, could be to change this type to:

export type ParseAlgoliaHitParams<TItem> = Pick<HighlightHitParams<TItem>, 'hit' | 'attribute'>;

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

let's go with this solution! ideally we'd do a proper traversal of the Hit to create the highlight types, but in the past that caused issues with types that got too long. This is a good stopgap solution

@Haroenv Haroenv merged commit 0ae0c5c into algolia:next Jul 8, 2024
1 check passed
@aldenquimby aldenquimby deleted the fix/1261 branch July 11, 2024 12:59
@aldenquimby
Copy link
Contributor Author

Thank you @Haroenv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HighlightHitParams attribute type should allow number for array indexing
2 participants