algolia / algoliasearch-client-javascript

⚡️ A fully-featured and blazing-fast JavaScript API client to interact with Algolia.
https://www.algolia.com/doc/api-client/javascript/getting-started/
MIT License
1.33k stars 222 forks source link

[bug]: Latest release breaks `algoliasearch` types #1562

Closed mrmckeb closed 1 month ago

mrmckeb commented 1 month ago

Description

In 5.9.0 (migrating from 5.8.1), types are broken for algoliasearch.

From my quick investigation, the package references types from @algolia/client-search, however this is not a dependency of algoliasearch and causes TypeScript to return any. Other types that were previously accessible, like SearchResponses are no longer available.

algoliasearch(appId: string, apiKey: string, options?: any): any

Client

All

Version

5.9.0

Relevant log output

No response

Haroenv commented 1 month ago

Do you have a specific setup in which this reproduces? There are .d.ts files published and they're registered in the package.json too

thegnuu commented 1 month ago

Something changed in the type definitions and somehow this keeps happening in this library even on minor changes which frequently breaks all of our implementations...

Previously the return of algoliasearch(...) was of the type SearchClient. import { SearchClient, algoliasearch } from 'algoliasearch';

With 5.9.0 this changed to Algoliasearch for some reason, I had to fix all of my imports and usages to this:

import { Algoliasearch, algoliasearch } from 'algoliasearch';

The type SearchClient is still available, but not exported in the packagealgoliasearch just in @algolia/client-search

To me the usage of Algoliasearch and algoliasearch seems a bad idea, separating two different things by a capital letter does not seem to be the best idea IMHO :)

thegnuu commented 1 month ago

I just found out that there are other issues appearing after updating:

ModuleDependencyError: export 'Status' (reexported as 'Status') was not found in '@algolia/client-abtesting'

We will revert everything to the previous version for now. This should not happen on minor releases, this breaks the whole semver idea, since those changes seems to be intended and not by accident. We are all happy to migrate stuff if changes are necessary but please let us know about it.

If you need any help reproducing the issue or more information feel free to reach out to me, I am happy to help!

shortcuts commented 1 month ago

Hey, sorry for the issue, when adding every other existing APIs to the algoliasearch package, the search one been left out from the dependencies in the package.json. Definitely not intentional, an I'm not even sure why it hasn't been picked up by our tooling.

https://github.com/algolia/api-clients-automation/pull/3988 should add the dependency back, and the SearchClient type should remain available.

I'll release 5.9.1 right away and work on a solution to better assert our algoliasearch bundle

thegnuu commented 1 month ago

@shortcuts no worries, things like this can happen. If you have a possibility to update your tooling to handle "wrong" typings it would be awesome since we had numerous small issues in the past.

Thanks a lot for the quick fix!

shortcuts commented 1 month ago

If you have a possibility to update your tooling to handle "wrong" typings it would be awesome since we had numerous small issues in the past.

Ahaha I thought it was the case actually but seems like not really

Thanks a lot for the quick fix!

Hopefully it's good! 5.9.1 has been released

thegnuu commented 1 month ago

@shortcuts well, at least we know now that it does not work ;)

Unfortunately I still have a lot of weird issues at the moment:

export 'EventType' (imported as 'EventType') was not found in '@algolia/client-personalization' (possible exports: apiClientVersion, personalizationClient)
export 'Status' (imported as 'Status') was not found in '@algolia/client-abtesting' (possible exports: abtestingClient, apiClientVersion)
export 'EventType' (imported as 'EventType') was not found in '@algolia/client-personalization' (possible exports: apiClientVersion, personalizationClient)
export 'Status' (imported as 'Status') was not found in '@algolia/client-abtesting' (possible exports: abtestingClient, apiClientVersion)
export 'EventType' (reexported as 'EventType') was not found in '@algolia/client-personalization' (possible exports: apiClientVersion, personalizationClient)
export 'Status' (reexported as 'Status') was not found in '@algolia/client-abtesting' (possible exports: abtestingClient, apiClientVersion)

I am currently not sure why this is happening, I have algoliasearch as a dependency in our SDK package, this one is buildable without any issues. As soon as I want to build it in another package which uses the SDK those errors occur. But this might as well be related to our setup, I am not sure at the moment. The weird thing is that everything works with 5.7.0

mrmckeb commented 1 month ago

Thanks for fixing this.

For future, you could use some testing libraries like Vitest for type testing @shortcuts. It might help catch these sorts of issues ahead of time. https://vitest.dev/guide/testing-types

Thanks @thegnuu for jumping in and responding with additional detail too :)

shortcuts commented 1 month ago

@mrmckeb thanks, I'll definitely look into that! Was the issue resolved for you?

I am currently not sure why this is happening, I have algoliasearch as a dependency in our SDK package, this one is buildable without any issues. As soon as I want to build it in another package which uses the SDK those errors occur. But this might as well be related to our setup, I am not sure at the moment. The weird thing is that everything works with 5.7.0

Could it be some treeshaking on your bundler side that removes the need of the unused dependency, but since we re-export every types it's now missing?

In any case, that's an error on our side, I'll look into that to maybe export only the search types by default, and let people import other client types from the standalone package directly, wdyt @Haroenv ?

mrmckeb commented 1 month ago

Hi @shortcuts, we're actually now hitting the same new issue as @thegnuu - so this is not fixed sorry.

For now we're still stuck on 5.8.1.

shortcuts commented 1 month ago

https://github.com/algolia/algoliasearch-client-javascript/issues/1565 is related now too, explained the bug here https://github.com/algolia/algoliasearch-client-javascript/issues/1565#issuecomment-2430277733

I think I finally have a good fix

shortcuts commented 1 month ago

Sorry to everyone for all the false hope and back and forth debugging

shortcuts commented 3 weeks ago

5.10.2 should've fixed all of the issues reported here