-
Notifications
You must be signed in to change notification settings - Fork 215
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good :)
There was a problem hiding this 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.
Very last @yannickcr can you build a version locally, and try it with react IS? Too see if there any problems on the types. 👍 |
b33696c
to
c84e0c6
Compare
c84e0c6
to
c8710ae
Compare
Good call @nunomaduro, the new export of Beside that it seems ok 🙂 |
I will double check this pull request after my current task. |
need to do that change in helper & InstantSearch.js too then |
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 |
Fixes #1080
This change marks all
@algolia/*
packages asbundledPackages
foralgoliasearch
, 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 throughalgoliasearch
are not properly re-exported.This PR is separated into 2 commits:
ae-forgotten-export
messages as errors and updates the build script to mark all@algolia/*
packages asbundledPackages
(foralgoliasearch
only). This is done dynamically so we do not have to update the configuration if we add/remove some packages.