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(typescript): export correct types #4476

Merged
merged 6 commits into from
Aug 14, 2020
Merged

fix(typescript): export correct types #4476

merged 6 commits into from
Aug 14, 2020

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Aug 14, 2020

Summary

This PR fixes some miscellaneous typescript issues discovered in angular instantsearch

Result

  • if JS client v3 is used, it's imperative that the ts-ignore comments stay present, since not the same exports are used. Solution is to double-annotate to keep the multi-line version in exposed to the d.ts (see e.g. Head comments are removed in some cases for next import line microsoft/TypeScript#39520)
  • similar in places.js
  • when updating TypeScript, I found that api-extractor can't deal with import() types, so those have to be transformed
  • fix stricter behaviour of tranformItems in hitsPerPage (we send the user the "render" values, not the one they entered)
  • SpeechRecognitionError is no longer exported in lib.d.ts, and is a generic Event now
  • updated Typescript to 3.8.3. I've tried 3.9.2 as well, but that has a bug with function() (transforms to (arg:any) => ; instead of (arg:any) => any;). I didn't open a TS issue for it, since it's fixed in v4 which we can migrate to later

This PR fixes some miscellaneous typescript issues discovered in angular instantsearch

- if JS client v3 is used, it's imperative that the ts-ignore comments stay present, since not the same exports are used. Solution is to double-annotate to keep the multi-line version in exposed to the d.ts (see e.g. microsoft/TypeScript#39520)
- similar in places.js
- when updating TypeScript, I found that api-extractor can't deal with `import()` types, so those have to be transformed
- fix stricter behaviour of tranformItems in hitsPerPage (we send the user the "render" values, not the one they entered)
- SpeechRecognitionError is no longer exported in lib.d.ts, and is a generic Event now
- updated Typescript to 3.8.3. I've tried 3.9.2 as well, but that has a bug with `function()` (transforms to `(arg:any) => ;` instead of `(arg:any) => any;`). I didn't open a TS issue for it, since it's fixed in v4 which we can migrate to later
@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 14, 2020

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 9c009e1:

Sandbox Source
InstantSearch.js Configuration

Copy link
Contributor

@yannickcr yannickcr left a comment

Choose a reason for hiding this comment

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

Small suggestion about the import() fix, beside that LGTM.

The @ts-ignore solution is very hacky but I don't think we can have a better solution right now 😐

Did you try with TypeScript 4.0 RC and if yes does it really fix this problem ?

scripts/build/types.js Outdated Show resolved Hide resolved
@Haroenv
Copy link
Contributor Author

Haroenv commented Aug 14, 2020

At least in the playground code like this was working in v4, but not 3.9:

/**
 * @param {function()} arg0
 */
function(arg0) {}

I couldn’t find an issue for it though, but since it’s fixed, there must have been one

@Haroenv Haroenv requested a review from yannickcr August 14, 2020 11:11
@Haroenv Haroenv merged commit 5fb4c5b into master Aug 14, 2020
@Haroenv Haroenv deleted the feat/ts-extractor branch August 14, 2020 11:42
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