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

feat: allow extra arguments in Insights API #1210

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

sarahdayan
Copy link
Member

@sarahdayan sarahdayan commented Oct 19, 2023

Summary

This types and tests the exposed Insights API to allow passing extra arguments in the payload.

This was already possible, but would have resulted in typing violations for TypeScript users.

FX-2652

@@ -6,6 +6,9 @@ export type AutocompleteInsightsApi = ReturnType<
typeof createSearchInsightsApi
>;

export type WithArbitraryParams<TParams extends Record<string, unknown>> =
Copy link
Member Author

Choose a reason for hiding this comment

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

It's technically an all-purpose utility type, but since it's only being used here for now, it seemed like an unnecessary hassle to move it to shared.

@codesandbox-ci
Copy link

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 0b43fc9:

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-react-renderer Configuration
@algolia/autocomplete-example-starter-algolia Configuration
@algolia/autocomplete-example-starter Configuration
@algolia/autocomplete-example-reshape Configuration
@algolia/autocomplete-example-vue Configuration

>
) {
if (params.length > 0) {
sendToInsights(
'clickedObjectIDsAfterSearch',
mapToInsightsParamsApi(params),
mapToInsightsParamsApi<
Copy link
Member Author

Choose a reason for hiding this comment

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

The way mapToInsightsParamsApi is typed is what's causing the typing issues I was encountering. When I inline the implementation of mapToInsightsParamsApi directly, the issue disappears.

I'm not sure what's going on exactly in how TypeScript interprets it all, but when unioning a strict record with an arbitrary record, the generic type TInsightsParamsType loses all the mandatory properties and the return type only contains objectIDs.

I'm doubtful we can fix this at the mapToInsightsParamsApi level, so the pragmatic option seems to cast.

Lmk if you have a working option that works better.

@sarahdayan sarahdayan marked this pull request as ready for review October 19, 2023 15:41
@sarahdayan sarahdayan merged commit 20d20a2 into next Oct 20, 2023
9 checks passed
@sarahdayan sarahdayan deleted the feat/sendevent-extra-args branch October 20, 2023 09:57
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.

None yet

2 participants