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 #1284

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Jun 23, 2021

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.

Copy link
Contributor

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

I thought it was done but I'm seeing some errors of missing export

@@ -196,6 +196,148 @@ import { createUserAgent, RequestOptions } from '@algolia/transporter';

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we just do export * from '@algolia/client-*' ?
Types listing seems unnecessary and hard to maintain + we would benefits from exporting helpers too

Copy link
Contributor

Choose a reason for hiding this comment

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

(for node.ts only obviously)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems like a good option, would you want to make a PR to take this over @bodinsamuel ? The only thing to be careful of with export * is that in some cases types are exported from JS files and then will have runtime errors when accessed directly from JS. I don't think that's really an issue here, but just to keep in mind

@bobylito
Copy link
Contributor

bobylito commented Apr 20, 2022

Hi there, what's the status? Is there something I can help with, maybe?

I use the latest version of the client and I can't access the type for the Settings and TS is bothering me with the queryType specifically.

PS: it seems that the current workaround for this is to import from subpackages (#1080 (comment))

import { Settings } from '@algolia/client-search';

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.

proposal: re-exports all types in algoliasearch
4 participants