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

fix(types): use InstantSearch.js types #828

Merged
merged 5 commits into from
Aug 19, 2021
Merged

fix(types): use InstantSearch.js types #828

merged 5 commits into from
Aug 19, 2021

Conversation

tkrugg
Copy link
Contributor

@tkrugg tkrugg commented Aug 13, 2021

Changes of this PR.

1. Introduces TypedBaseWidget, without removing BaseWidget

We could have replaced BaseWidget but that would:

  • break many of the current implementations
  • require to update the docs before the release.
  • require update of all our doc-code-samples
    I'm choosing to keep BaseWidget and only deprecate it.

2. Migrate all widgets to TypeBaseWidget and InstantSearch.js types.

We no longer expose types from the Angular Instantsearch themselves (users will need to get them from instantsearch.js)

3. updates IS.js to 4.26 to use the latest types.

4. Migrate storybook

@netlify
Copy link

netlify bot commented Aug 13, 2021

✔️ Deploy Preview for angular-instantsearch ready!

🔨 Explore the source changes: 4e1180e

🔍 Inspect the deploy log: https://app.netlify.com/sites/angular-instantsearch/deploys/61168626d13b92000725db2c

😎 Browse the preview: https://deploy-preview-828--angular-instantsearch.netlify.app

@tkrugg tkrugg marked this pull request as ready for review August 13, 2021 15:18

private differ: KeyValueDiffer<string, any>; // SearchParameters (I don't know how to get the values of the type)
Copy link
Contributor

Choose a reason for hiding this comment

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

ok with the removal of the comment. Did you try ConfigureConnectorParams['searchParameters'][keyof ConfigureConnectorParams['searchParameters']] though? I think that's the same now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this but I started getting:

Overload 2 of 2, '(object: { [key: string]: string | number | boolean | string[] | readonly string[] | readonly (readonly string[])[] | readonly (readonly number[])[] | readonly ("exactPhrase" | "excludeWords")[] | readonly ("ignorePlurals" | "singleWordSynonym" | "multiWordsSynonym")[] | readonly { readonly from: number; readonly value: number; }[] | ... 6 more ... | { ...; }; }): KeyValueChanges<...>', gave the following error.
    Argument of type 'PlainSearchParameters' is not assignable to parameter of type '{ [key: string]: string | number | boolean | string[] | readonly string[] | readonly (readonly string[])[] | readonly (readonly number[])[] | readonly ("exactPhrase" | "excludeWords")[] | readonly ("ignorePlurals" | "singleWordSynonym" | "multiWordsSynonym")[] | readonly { readonly from: number; readonly value: numb...'.
      Index signature is missing in type 'PlainSearchParameters'.

Have you already encountered a similar use case?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's because the differ doesn't work with readonly I guess?

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

better approach! LGTM!

@tkrugg tkrugg merged commit 7c9a1ed into v4 Aug 19, 2021
@tkrugg tkrugg deleted the v4-fixes/types branch August 19, 2021 13: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