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

chore: types running with v4 locally, but also v4 and v3 #6271

Merged

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Jul 8, 2024

Summary

  • detect v5 correctly in types
  • use v5 client as base version to avoid conflicts (which customers won't have unless they install both versions like us)

Result

all tests green

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 fa45bd4:

Sandbox Source
example-instantsearch-getting-started Configuration
example-react-instantsearch-getting-started Configuration
example-react-instantsearch-next-app-dir-example Configuration
example-react-instantsearch-next-routing-example Configuration
example-vue-instantsearch-getting-started Configuration

@Haroenv Haroenv changed the title Fix/v5 types with v4 installed chore: types running with v4 locally, but also v4 and v3 Jul 15, 2024
@Haroenv Haroenv marked this pull request as ready for review July 15, 2024 11:30
@Haroenv Haroenv requested a review from dhayab July 15, 2024 11:30
@Haroenv Haroenv requested a review from aymeric-giraudet July 15, 2024 11:30
Comment on lines +132 to +134
// v5 expects only string values for facetName and facetValue
facetName: undefined as unknown as string,
facetValue: undefined as unknown as string,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

algolia/api-clients-automation#3355 after this is released we can amend to remove the cast. Still find for now as it's just a test though

@@ -31,7 +29,7 @@ export type FrequentlyBoughtTogetherRenderState<
/**
* The matched recommendations from Algolia API.
*/
items: Array<Hit<THit>>;
items: Array<AlgoliaHit<THit>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all recommend changes Hit -> AlgoliaHit is because __position and __queryId aren't actually present in these connectors

Copy link
Member

@dhayab dhayab left a comment

Choose a reason for hiding this comment

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

Impressive, thanks for handling this 🙇 ! I've left 2 non-blocking comments.

package.json Show resolved Hide resolved
tests/mocks/createAPIResponse.ts Outdated Show resolved Hide resolved
tests/mocks/createAlgoliaSearchClient.ts Outdated Show resolved Hide resolved
@Haroenv Haroenv merged commit de8ef70 into chore/examples-algoliasearch-v5 Jul 16, 2024
8 checks passed
@Haroenv Haroenv deleted the fix/v5-types-with-v4-installed branch July 16, 2024 07:47
Haroenv added a commit that referenced this pull request Jul 17, 2024
* chore: change type

* WIP: expand search parameters

doesn't work as there still are v4 v5 differences of course

* do it correctly

* use regular client

* not sure if this should stay

* WIP

* changes rule

* fix dependency

* change order to make script work

* almost all types fixed

* improve test

* fix(types): accepts client with wrong types

will be fixed once algolia/api-clients-automation#3357 is done

* tests

* chore(helper): compatibility

* chore(legacy): correct replacement

* don't remove v5 fully

* fix errors

* safer (or less safe lol)

* correct import

* fix fake type

* v5 only works

* tests

* script for examples too

* script for examples too

* chore(answers): bleh get rid of this!

* stuff

* simplify
Haroenv added a commit that referenced this pull request Aug 19, 2024
* chore: change type

* WIP: expand search parameters

doesn't work as there still are v4 v5 differences of course

* do it correctly

* use regular client

* not sure if this should stay

* WIP

* changes rule

* fix dependency

* change order to make script work

* almost all types fixed

* improve test

* fix(types): accepts client with wrong types

will be fixed once algolia/api-clients-automation#3357 is done

* tests

* chore(helper): compatibility

* chore(legacy): correct replacement

* don't remove v5 fully

* fix errors

* safer (or less safe lol)

* correct import

* fix fake type

* v5 only works

* tests

* script for examples too

* script for examples too

* chore(answers): bleh get rid of this!

* stuff

* simplify
Haroenv added a commit that referenced this pull request Aug 20, 2024
* chore(examples): update search client to v5

* chore: types running with v4 locally, but also v4 and v3 (#6271)

* chore: change type

* WIP: expand search parameters

doesn't work as there still are v4 v5 differences of course

* do it correctly

* use regular client

* not sure if this should stay

* WIP

* changes rule

* fix dependency

* change order to make script work

* almost all types fixed

* improve test

* fix(types): accepts client with wrong types

will be fixed once algolia/api-clients-automation#3357 is done

* tests

* chore(helper): compatibility

* chore(legacy): correct replacement

* don't remove v5 fully

* fix errors

* safer (or less safe lol)

* correct import

* fix fake type

* v5 only works

* tests

* script for examples too

* script for examples too

* chore(answers): bleh get rid of this!

* stuff

* simplify

* ci: add downgrade script for algoliasearch v4 and steps (#6286)

* ci: add downgrade script for algoliasearch v4 and steps

* test:ci instead

* store results

* fixes

---------

Co-authored-by: Haroen Viaene <hello@haroen.me>
Co-authored-by: Aymeric Giraudet <aymeric.giraudet@algolia.com>
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.

2 participants