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): fix missing type exports #1086

Closed
wants to merge 2 commits into from
Closed

Conversation

yannickcr
Copy link
Contributor

Fixes #1080

This change marks all @algolia/* packages as bundledPackages for algoliasearch, which means that they are treated as part of this package and not external dependencies.

As a result API Extractor will throw an error if all types from @algolia/* packages that are accessible publicly through algoliasearch are not properly re-exported.

This PR is separated into 2 commits:

  • The 1st one updates API Extractor configuration to mark ae-forgotten-export messages as errors and updates the build script to mark all @algolia/* packages as bundledPackages (for algoliasearch only). This is done dynamically so we do not have to update the configuration if we add/remove some packages.
  • The 2nd one updates the actual TS files to fix the missing exports.

Haroenv
Haroenv previously approved these changes Mar 16, 2020
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.

looks good :)

Copy link
Contributor

@nunomaduro nunomaduro left a comment

Choose a reason for hiding this comment

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

Thank you so much for helping on this issue, left some questions.

@nunomaduro
Copy link
Contributor

nunomaduro commented Mar 16, 2020

Very last @yannickcr can you build a version locally, and try it with react IS? Too see if there any problems on the types. 👍

@yannickcr
Copy link
Contributor Author

yannickcr commented Mar 16, 2020

Good call @nunomaduro, the new export of Response was messing with the algoliasearch v3 detection ( https://github.com/algolia/react-instantsearch/blob/master/packages/react-instantsearch-core/src/types/algoliasearch.ts ). I renamed it to RequesterResponse (and Request to RequesterRequest to be coherent).

Beside that it seems ok 🙂

@nunomaduro
Copy link
Contributor

I will double check this pull request after my current task.

@Haroenv
Copy link
Contributor

Haroenv commented Mar 17, 2020

need to do that change in helper & InstantSearch.js too then

@yannickcr
Copy link
Contributor Author

yannickcr commented Mar 17, 2020

I renamed the exports in the client ( c8710ae#diff-e002f6d33e4974541b52b0359ef2f1f2R59-R64 ), so there is no need to do some changes in the Helper, React or IS.js

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

Successfully merging this pull request may close these issues.

proposal: re-exports all types in algoliasearch
3 participants