algoliasearch-client-javascript icon indicating copy to clipboard operation
algoliasearch-client-javascript copied to clipboard

No clear migration guide for v5 and no explicit typing

Open WavyWalk opened this issue 1 year ago • 22 comments

We want to migrate to v4 from v5, but migrating even this snippet is problematic:

const object = await algoliasearch(algoliaApplicationId, algoliaApiKey)
      .initIndex(productsAlgoliaIndex)
      .getObjects<AlgoliaRecord>(productIds)

I see in source code that getObjects exist in v5 but one can't pass index and it's not in docs, and there's no index in types.

Also types are not straightforward (I get that it's generated but still), so rules like @typescript-eslint/no-unsafe-assignment fail because of any's in unions.

As well not sure how to proceed with now failing type checks for react instantsearch because things like: hitsPerPage, analyticsTags on <Configure /> reference the types of v5 which do not have it.

Do you recommend to migrate now or wait till there will be a proper migration guide and types will be improved?

WavyWalk avatar Aug 16 '24 15:08 WavyWalk

You need to update your code using this documentation. It doesn't feel complete so it sucks. You might have to view code documentation/intellisense from your code editor/IDE.

Before:

import algoliasearch from "algoliasearch";

const object = await algoliasearch(algoliaApplicationId, algoliaApiKey)
  .initIndex(productsAlgoliaIndex)
  .getObjects<AlgoliaRecord>(productIds);

After:

import { algoliasearch } from "algoliasearch";

const object = await algoliasearch(algoliaApplicationId, algoliaApiKey)
  .getObjects<AlgoliaRecord>({
    requests: [
      {
        indexName: productsAlgoliaIndex,
        objectID: productId1,
      },
      {
        indexName: productsAlgoliaIndex,
        objectID: productId2,
      },
      // ...
    ],
  });

OutdatedGuy avatar Aug 16 '24 18:08 OutdatedGuy

Hey @WavyWalk thanks for using the v5 client already, and sorry for the lack of documentation, we are currently working on providing snippets and guides! (and thanks @OutdatedGuy for the help :D)

I see in source code that getObjects exist in v5 but one can't pass index and it's not in docs, and there's no index in types.

The initIndex of the previous version was kind of magical syntactic sugar (and not suited for generated clients too), as mentioned here, every methods are now available at the root of the client, if you were using one via initIndex before, it will now expect an indexName parameter if it's a single-index method, or an indexName in the requests if it's a multi-index method

In the meantime of the guides being available, you can browse our API reference which includes a code snippet for every methods of the client, for every languages.

As well not sure how to proceed with now failing type checks for react instantsearch because things like: hitsPerPage, analyticsTags on <Configure /> reference the types of v5 which do not have it.

I'll investigate this but since I'm not super familiar with RIS cc @Haroenv

Do you recommend to migrate now or wait till there will be a proper migration guide and types will be improved?

Did you find any wrong typings or something that would prevent you from migrating? If so, I can fix those and help you move forward :) Otherwise I think it would be mostly migrating method signatures which (I hope) shouldn't be too different

shortcuts avatar Aug 16 '24 19:08 shortcuts

for React InstantSearch, ensure you're on the very latest version, if you're on an older version this may not be inferred right. In my testing it also worked completely as expected, but maybe we missed a case? please make a full reproduction if anything isn't inferred as expected

Haroenv avatar Aug 16 '24 20:08 Haroenv

Here's repo with reproduction of the issues I mentioned: https://github.com/WavyWalk/reproduce-algolia-v5

  • hitsPerPage, analyticsTags are missing on types for properties of react instant search Configure
  • type definitions break @typescript-eslint/no-redundant-type-constituents rule
  • because of that as well this rule will break @typescript-eslint/no-unsafe-assignmen

all you can see in https://github.com/WavyWalk/reproduce-algolia-v5/blob/master/src/App.tsx

WavyWalk avatar Aug 19 '24 13:08 WavyWalk

Oh no, you're right! I'm just realising that a last fix for the types was only done in https://github.com/algolia/instantsearch/pull/6270 which isn't merged yet. I'll get right on making sure CI passes there (unrelated failure) and we can release a fix soon

Haroenv avatar Aug 19 '24 13:08 Haroenv

regarding getObjects:

I see that objects can be fetched from multiple indexes and hence it accept array of index objectID pairs,

but if I pass hundreds of pairs with same index will it still make one call?

.getObjects<AlgoliaRecord>({
    requests: [
      {
        indexName: productsAlgoliaIndex,
        objectID: productId1,
      },
      {
        indexName: productsAlgoliaIndex,
        objectID: productId2,
      },
      // ...
    ],

more natural would be, does it support it?:

requests: [
      {
        indexName: someIndex,
        objectIds: Array<string>,
      },
      {
        indexName: otherIndex,
        objectID: Array<string>,
      }

```]

WavyWalk avatar Aug 19 '24 14:08 WavyWalk

Hey

regarding getObjects:

I see that objects can be fetched from multiple indexes and hence it accept array of index objectID pairs,

but if I pass hundreds of pairs with same index will it still make one call?

Yes

more natural would be, does it support it?:

Those new clients are meant to be closer to the API, so this signatures is exactly what is expected from the REST endpoint

shortcuts avatar Aug 19 '24 14:08 shortcuts

👋 Not to pile on here, but please could an actual migration guide be published here, that documents the actual signature changes for methods?

For example, it turns out browseObjects now takes an aggregator method, not batch, and the data passed to that method is of a different shape to before (an object containing a hits array, rather than just an array). Or take setSettings which now takes the settings as a nested indexSettings method and forwardToReplicas as part of the first object, not a second object. I had to inspect the source to find these things, and I should not have to do that when a migration guide supposedly exists...

Even the bits that are documented currently aren't great... wait was removed on methods and the replacements in the guide link out to documentation for the Python library. Even if they did link to the JS docs, it doesn't appear to be a 1:1 mapping of the methods -- if I call saveObjects I don't just get back taskID or an object containing it, instead I get back an array of objects containing task IDs... and so I need to manually call waitForTask for each one? This seems like a huge step back for DX...

I would complain far less if the documentation actually existed for the methods, but that doesn't exist either (sans a select few methods classed as "helpers" or unless you rely on code samples in the REST API docs which seem to be missing a bunch of options [like forwardToReplicas mentioned earlier])... so between no migration guide and no actual documentation, trying to upgrade to this new major is incredibly painful at present.

MattIPv4 avatar Aug 20 '24 00:08 MattIPv4

The React InstantSearch issue is solved now, thanks for holding on. The teams responsible are talking about how to add more migration guides etc., but in the mean time don't feel pressured to update if it's too complicated or unclear for you, v4 will continue to work for a long time (v3 even still works now).

Haroenv avatar Aug 20 '24 11:08 Haroenv

Hello 👋

Been trying to move to the v5 client like that

const { results } = await this.client.search<AlgoliaSearchResult>({ requests: [ { indexName: 'name', query: searchString, }, ], })

The provided results is an array of one object

{
  results: [
    {
      hits: [
        [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...], [Object ...]
      ],
      nbHits: 148,
      page: 0,
      nbPages: 3,
      hitsPerPage: 60,
      exhaustiveNbHits: true,
      exhaustive: [Object ...],
      query: "...",
      params: "query=...",
      index: "....",
      processingTimeMS: 1,
      processingTimingsMS: [Object ...],
      serverTimeMS: 3,
    }
  ],
}

But the type of the returned object is not correctly resolved, it is lacking nbHits and other params ...

image

Tirke avatar Aug 21 '24 15:08 Tirke

@Tirke you may be interested in https://github.com/algolia/algoliasearch-client-javascript/issues/1536

Haroenv avatar Aug 21 '24 15:08 Haroenv

@Haroenv The issue I'm showcasing is still present on 5.1.0 :(

Tirke avatar Aug 22 '24 07:08 Tirke

@Tirke yes, in 5.1.0 the alias searchForHits was added, which is like search, but only for hits

Haroenv avatar Aug 22 '24 08:08 Haroenv

Ah yes I skimmed a bit fst over the issue, those helpers are not showcased in the docs I guess? Thanks for the pointer :)

Tirke avatar Aug 22 '24 10:08 Tirke

I'm also not sure how to upgrade to v5. Here's the my old code:

import { type Hit } from '@algolia/client-search';
import { default as algoliasearch } from 'algoliasearch/lite';

const ALGOLIA_APP_ID = '';
const ALGOLIA_SEARCH_API_KEY = '';

type SearchOptions = {
	indexName: string;
	query: string;
	pageParam?: number;
	hitsPerPage: number;
};

export async function search<TData>({ indexName, query, pageParam, hitsPerPage }: SearchOptions): Promise<{
	hits: Hit<TData>[];
	nextPage: number | undefined;
}> {
	const client = algoliasearch(ALGOLIA_APP_ID, ALGOLIA_SEARCH_API_KEY);
	const index = client.initIndex(indexName);

	const { hits, page, nbPages } = await index.search<TData>(query, {
		page: pageParam ?? 0,
		hitsPerPage,
	});

	const nextPage = page + 1 < nbPages ? page + 1 : undefined;

	return { hits, nextPage };
}

lukebennett88 avatar Aug 25 '24 11:08 lukebennett88

Ah yes I skimmed a bit fst over the issue, those helpers are not showcased in the docs I guess? Thanks for the pointer :)

Not yet, we are working on generating them!

I'm also not sure how to upgrade to v5. Here's the my old code:

We will soon provide a documentation page with a code snippet for every methods, along with a mapping with method renames

For your specific code snippet, it would be

import { Hit } from '@algolia/client-search';
import { liteClient as algoliasearch } from 'algoliasearch/lite';

const ALGOLIA_APP_ID = '';
const ALGOLIA_SEARCH_API_KEY = '';

type SearchOptions = {
	indexName: string;
	query: string;
	pageParam?: number;
	hitsPerPage: number;
};

export async function search<TData>({ indexName, query, pageParam, hitsPerPage }: SearchOptions): Promise<{
	hits: Hit<TData>[];
	nextPage: number | undefined;
}> {
	const client = algoliasearch(ALGOLIA_APP_ID, ALGOLIA_SEARCH_API_KEY);

	const { hits, page, nbPages } = await client.searchSingleIndex<TData>({
                indexName,
                searchParams: {
                  query,
		  page: pageParam ?? 0,
		  hitsPerPage,
                },
	});

	const nextPage = page + 1 < nbPages ? page + 1 : undefined;

	return { hits, nextPage };
}

You can find some example here https://www.algolia.com/doc/rest-api/search/#tag/Search

shortcuts avatar Aug 25 '24 14:08 shortcuts

Thanks @shortcuts but that doesn't seem to work for me. There doesn't seem to be a searchSingleIndex method on client. There are no types for it, but I tried it anyway and it's not there.

I did manage to get this to work, but as you can see I've had to cast to any for the result.

import { type Hit } from '@algolia/client-search';
import { liteClient as algoliasearch } from 'algoliasearch/lite';

const ALGOLIA_APP_ID = '';
const ALGOLIA_SEARCH_API_KEY = '';

type SearchOptions = {
	indexName: string;
	query: string;
	pageParam?: number;
	hitsPerPage: number;
};

export async function search<TData>({ indexName, query, pageParam, hitsPerPage }: SearchOptions): Promise<{
	hits: Hit<TData>[];
	nextPage: number | undefined;
}> {
	const client = algoliasearch(ALGOLIA_APP_ID, ALGOLIA_SEARCH_API_KEY);

	const { results } = await client.search<TData>({
		requests: [
			{
				indexName,
				query,
				page: pageParam,
				hitsPerPage,
			},
		],
	});

	// TODO: fix types
	const result = results[0] as any;
	const { hits, nbPages, page } = result;
	const nextPage = page + 1 < nbPages ? page + 1 : undefined;

	return { hits, nextPage };
}

Not sure if there's a better way to do this.

lukebennett88 avatar Aug 26 '24 00:08 lukebennett88

Thanks @shortcuts but that doesn't seem to work for me.

Ah yes sorry I forgot there's only the multi-index search on the lite client

I did manage to get this to work, but as you can see I've had to cast to any for the result.

then you'd also be interested by https://github.com/algolia/algoliasearch-client-javascript/issues/1537#issuecomment-2302413180

shortcuts avatar Aug 26 '24 05:08 shortcuts

I also just found out after some digging that the lite client doesnt have the "searchSingleIndex" method. Is there a reason for it?

simonmaass avatar Aug 26 '24 12:08 simonmaass

I also just found out after some digging that the lite client doesnt have the "searchSingleIndex" method. Is there a reason for it?

Bundle size/keeping the lite client as minimal as possible, since you can do single index search with the multi-index method (search)

shortcuts avatar Aug 26 '24 12:08 shortcuts

I just finished migrating our own codebase from Algolia v4 to v5 and found the process very confusing. There's one page that I found with "upgrade" examples and it felt pretty anemic.

The main problem IMHO is that the (new?) TypeScript types are extremely convoluted and hard to follow. For example, when I Command+Click to see what the definition of searchSingleIndex is (because I've never heard of that method before), I get this type definition:

searchSingleIndex<T>({ indexName, searchParams }: import("../model").
SearchSingleIndexProps, requestOptions?: import("@algolia/client-common").RequestOptions):
Promise<import("../model").SearchResponse<T>>;

It looks auto-generated, no line breaks for clarity, no comments. I've never seen imports used as type parameters - probably because a human would never write this! I see this (buried in a soup of other typedefs/imports) and I want to give up.

Or when I try to figure out what options are available in SearchQuery which is the main type for interacting with Algolia. Command+Click, and I get:

export type SearchQuery = SearchForFacets | SearchForHits;

What's SearchForHits? It's:

export type SearchForHits = SearchForHitsOptions & SearchParams;

What's SearchForHitsOptions? It just goes on and on and on, all in different files, this endless stack of self-referencing turtles all the way down.

I would have stayed on v4 but our build broke for unknown reasons so I finally spent a few hours yesterday migrating.

Please improve these types so that humans can read and understand them!

nfarina avatar Aug 28 '24 15:08 nfarina

I just finished migrating our own codebase from Algolia v4 to v5 and found the process very confusing. There's one page that I found with "upgrade" examples and it felt pretty anemic.

Thanks for trying out the new major version still :D We are still working on the docs and improving the clients, sorry it was a complicated migration, and thanks a lot for the feedbacks

The main problem IMHO is that the (new?) TypeScript types are extremely convoluted and hard to follow. For example, when I Command+Click to see what the definition of searchSingleIndex is (because I've never heard of that method before), I get this type definition:

Seems like the issue comes from the spread here, without it the LSP properly finds the definition, that's definitely something we need to fix

It looks auto-generated, no line breaks for clarity, no comments. I've never seen imports used as type parameters - probably because a human would never write this! I see this (buried in a soup of other typedefs/imports) and I want to give up.

You are right, it is generated, but that's not a generation issue, see the definition without the spread:

Screenshot 2024-08-28 at 17 28 46

Or when I try to figure out what options are available in SearchQuery which is the main type for interacting with Algolia. Command+Click, and I get:

export type SearchQuery = SearchForFacets | SearchForHits;

What's SearchForHits? It's:

export type SearchForHits = SearchForHitsOptions & SearchParams;

What's SearchForHitsOptions? It just goes on and on and on, all in different files, this endless stack of self-referencing turtles all the way down.

We reuse types and have polymorphic APIs so it's expected to see unions and intersections. We can try to see if the output is better without reusing types, it would reduce the composability (like having to manually Pick some fields), but if it provides a better DX, why not!

shortcuts avatar Aug 28 '24 15:08 shortcuts

The main problem IMHO is that the (new?) TypeScript types are extremely convoluted and hard to follow. For example, when I Command+Click to see what the definition of searchSingleIndex is (because I've never heard of that method before), I get this type definition:

searchSingleIndex<T>({ indexName, searchParams }: import("../model").
SearchSingleIndexProps, requestOptions?: import("@algolia/client-common").RequestOptions):
Promise<import("../model").SearchResponse<T>>;

Hey, the 5.3.0 version (will be published later today) should fix this

shortcuts avatar Sep 06 '24 12:09 shortcuts

👋 @shortcuts I see that 5.3.0 has been released, but https://www.algolia.com/doc/libraries/javascript/v5/upgrade/ appears to still be incredibly lacklustre with very little in the way of documentation on how to actually migrate usage from v4 (and still references Python methods!). Please could I ask you put some more effort into actually writing a migration guide that covers how folks should update their usages, 'cause methods are definitely not 1:1...? I do not feel anything I mentioned in https://github.com/algolia/algoliasearch-client-javascript/issues/1537#issuecomment-2297759303 has really been addressed.

MattIPv4 avatar Sep 06 '24 20:09 MattIPv4

Hey @MattIPv4 thanks for trying out the new major :)

I resolved the issue because the typing problem should be solved, but the but the documentation problem still remains indeed.

Our documentation team is working on writing the new (migration) guides as well as providing a mapping table of the method changes.

In the meantime, maybe the following resources could help:

(sorry again for the documentation issues)

shortcuts avatar Sep 06 '24 20:09 shortcuts

Thanks, glad to hear some better migration docs are on there way! Is there an issue tracking that that I could follow for updates, or will updates be shared here?

MattIPv4 avatar Sep 06 '24 21:09 MattIPv4

Hey team, the docs are mostly looking good, though some additional information for comparisons between v4 and v5 methods would be extremely useful. For example, in v4, the saveObjects method takes a autoGenerateObjectIDIfNotExist param. This is not mentioned anywhere in the v5 docs for this method. Do you still need to pass this? Does it still exist? If so, what is its default value? I'm currently migrating an older codebase from v4 and it's difficult to figure out what is and isn't no longer necessary.

m-shum avatar Jan 09 '25 16:01 m-shum

extremely confused here, the v5 doesnt still have proper documentation. what is initIndex replacement please?

patrickaigbogun avatar May 21 '25 22:05 patrickaigbogun

I agree that there's probably documentation missing. There's no replacement for initIndex, all methods take indexName as a parameter instead.

Haroenv avatar May 22 '25 06:05 Haroenv