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

TypeError with TS@5.1.3: Property 'hitsPerPage' does not exist on type 'IntrinsicAttributes & PlainSearchParameters' #5658

Closed
1 task done
jcwillox opened this issue Jun 5, 2023 · 13 comments · Fixed by algolia/algoliasearch-helper-js#943 or #5676

Comments

@jcwillox
Copy link

jcwillox commented Jun 5, 2023

🐛 Current behavior

So this might be a bug with typescript, but it's the only library that broke when updating it so I'm going to create the bug report here.

When running with version 5.1.3 of typescript and react-instantsearch-hooks-web I get a type error on the Configure component, it appears it's no longer inferring the SearchOptions correctly. Downgrading to version 5.0.4 of typescript fixes the issue.

> tsc --noEmit

components/SearchApp/SearchApp.tsx:28:18 - error TS2322: Type '{ hitsPerPage: number; filters: string | undefined; }' is not assignable to type 'IntrinsicAttributes & PlainSearchParameters'.
  Property 'hitsPerPage' does not exist on type 'IntrinsicAttributes & PlainSearchParameters'.

28       <Configure hitsPerPage={hitsPerPage} filters={filters} />
                    ~~~~~~~~~~~

🔍 Steps to reproduce

  1. Install the following packages react-instantsearch-hooks-web@6.44.1 typescript@5.1.3
  2. Create a component with
<InstantSearch searchClient={searchClient} indexName={indexName}>
  <Configure hitsPerPage={hitsPerPage} filters={filters} />
  {children}
</InstantSearch>
  1. Run tsc --noEmit
> tsc

components/SearchApp/SearchApp.tsx:28:18 - error TS2322: Type '{ hitsPerPage: number; filters: string | undefined; }' is not assignable to type 'IntrinsicAttributes & PlainSearchParameters'.
  Property 'hitsPerPage' does not exist on type 'IntrinsicAttributes & PlainSearchParameters'.

28       <Configure hitsPerPage={hitsPerPage} filters={filters} />
                    ~~~~~~~~~~~

Or checkout this repro https://stackblitz.com/edit/algolia-ts-bug-2mfs3u?file=app%2FSearchApp.tsx and run npm run typecheck in the terminal.

Live reproduction

https://stackblitz.com/edit/algolia-ts-bug-2mfs3u?file=app%2FSearchApp.tsx

💭 Expected behavior

There should be no typescript error as with previous versions of typescript.

Package version

react-instantsearch-hooks-web@6.44.1, typescript@5.1.3

Operating system

Windows, Linux

Browser

Chrome

Code of Conduct

  • I agree to follow this project's Code of Conduct
@jcwillox jcwillox added the triage Issues to be categorized by the team label Jun 5, 2023
@Haroenv
Copy link
Contributor

Haroenv commented Jun 5, 2023

I wonder if this is caused by PlainSearchParameters being extended from SearchOptions, but not manually retyping those keys? https://github.com/algolia/algoliasearch-helper-js/blob/develop/index.d.ts#L394

@jcwillox
Copy link
Author

jcwillox commented Jun 8, 2023

It seems the editor/typescript has trouble picking up the type of SearchOptions even when directly imported from algoliasearch-helper/types/algoliasearch and including it as a dependency. When importing SearchOptions directly from @algolia/client-search it does have the correct types but that's not surprising.

image

So perhaps its something to do with that PickForClient magic?

@jcwillox
Copy link
Author

jcwillox commented Jun 8, 2023

Currently, I'm just re-exporting the Configure component and including the SearchOptions type directly, which side-steps the issue without having to use @ts-ignore or type assertions.

import { SearchOptions } from "@algolia/client-search";
import {
  Configure as ConfigureAlgolia,
  ConfigureProps,
} from "react-instantsearch-hooks-web";

export const Configure = (props: ConfigureProps & SearchOptions) => (
  <ConfigureAlgolia {...props} />
);

@Haroenv
Copy link
Contributor

Haroenv commented Jun 8, 2023

Thanks for listing this workaround, we didn't find that option yet!

Haroenv added a commit to algolia/algoliasearch-helper-js that referenced this issue Jun 13, 2023
in TypeScript 5.1.3, the check `any extends T` when T is any no longer evaluates to one branch of the conditional, but the union of both branches.

This is fixed by using a "never possible to be true, unless T is any" condition as in https://stackoverflow.com/a/49928360/3185307

fixes algolia/instantsearch#5658
FX-2396
Haroenv added a commit to algolia/algoliasearch-helper-js that referenced this issue Jun 13, 2023
* fix(types): improve conditional for client version

in TypeScript 5.1.3, the check `any extends T` when T is any no longer evaluates to one branch of the conditional, but the union of both branches.

This is fixed by using a "never possible to be true, unless T is any" condition as in https://stackoverflow.com/a/49928360/3185307

fixes algolia/instantsearch#5658
FX-2396

* chore(dev): update typescript

this showed the error before the previous commit as

```
test/types.ts:16:7 - error TS2345: Argument of type '{ query: string; }' is not assignable to parameter of type 'PlainSearchParameters'.
  Object literal may only specify known properties, and 'query' does not exist in type 'PlainSearchParameters'.

16       query: 'something fun',
         ~~~~~

Found 1 error in test/types.ts:16

error Command failed with exit code 2.
```
@RobinSCU
Copy link

Hi @Haroenv,

thanks for the fix! Do you have an ETA for a release where the fix is included?

Have a great day!

@Haroenv
Copy link
Contributor

Haroenv commented Jun 14, 2023

I've just released the fix in the helper, so if you re-update the dependencies, you will no longer have this issue.

@RobinSCU
Copy link

Thank you!

Haroenv added a commit that referenced this issue Jun 15, 2023
Although automatic updates can already solve this issue on its own, an explicit update will prevent many more people seeing it.

fixes #5658
Haroenv added a commit that referenced this issue Jun 19, 2023
Although automatic updates can already solve this issue on its own, an explicit update will prevent many more people seeing it.

fixes #5658
dhayab pushed a commit that referenced this issue Jul 10, 2023
…arch-helper-js#943)

* fix(types): improve conditional for client version

in TypeScript 5.1.3, the check `any extends T` when T is any no longer evaluates to one branch of the conditional, but the union of both branches.

This is fixed by using a "never possible to be true, unless T is any" condition as in https://stackoverflow.com/a/49928360/3185307

fixes #5658
FX-2396

* chore(dev): update typescript

this showed the error before the previous commit as

```
test/types.ts:16:7 - error TS2345: Argument of type '{ query: string; }' is not assignable to parameter of type 'PlainSearchParameters'.
  Object literal may only specify known properties, and 'query' does not exist in type 'PlainSearchParameters'.

16       query: 'something fun',
         ~~~~~

Found 1 error in test/types.ts:16

error Command failed with exit code 2.
```
@g-mero
Copy link

g-mero commented Sep 18, 2023

Same issue on instantsearch.js

@Haroenv
Copy link
Contributor

Haroenv commented Sep 19, 2023

@g-mero, are you on the latest version of everything? This issue was fixed a couple months ago downstream

@Jorundur
Copy link

@Haroenv

I've got a similar issue after migrating from v6 to v7.

Packages:

  "react-instantsearch": "^7.4.1",
  "react-instantsearch-core": "^7.4.1",

Removed @types packages as per migration guide.

In a component I've got:

import { Configure, InstantSearch, useInfiniteHits } from 'react-instantsearch'

(previously this was imported from react-instantsearch-hooks-web)

render:

return (
    <div>
      <InstantSearch searchClient={searchClient} indexName={algoliaIndex}>
        <Configure filters={`productType:"${parentExpedition.productType}"`} hitsPerPage={4} />
        {/* irrelevant code removed */}
      </InstantSearch>
    </div>
  )

Screenshot 2023-12-14 at 11 29 37

As mentioned above - the props type is PlainSearchParameters which doesn't have filters. Although PlainSearchParameters extends SearchOptions, SearchOptions doesn't have anything for filters either as far as I can see but I could be misunderstanding the PickForClient code.

Any ideas how to get this to work? I can see that the latest version (3.16.0) of algoliasearch-helper is being used.

@Haroenv
Copy link
Contributor

Haroenv commented Dec 14, 2023

I can't reproduce this in our examples, filters is correctly picked from algoliasearch (v4 and v3), we even have tests for it. If you can reproduce this consistently in a standalone environment, please create a sandbox, GitHub repo or similar and we can try to find the root cause @Jorundur

@Jorundur
Copy link

Jorundur commented Dec 14, 2023

@Haroenv I just fixed this.

Had this warning when yarn-ing which I hadn't noticed:

"react-instantsearch > instantsearch.js@4.62.0" has unmet peer dependency "algoliasearch@>= 3.1 < 6".

Adding algoliasearch fixed this.

I didn't know that was required - it wasn't before when using v6 at least. So maybe it should be added to the migration guide? (Perhaps it's already there and I missed it).

@Haroenv
Copy link
Contributor

Haroenv commented Dec 14, 2023

It always was required, but in v6 there was no typescript types builtin. I believe v6 on DefinitelyTyped had a dependency on algoliasearch possibly though

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