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): export all types as "type" to avoid exporting in .js #4837

Merged
merged 3 commits into from
Aug 18, 2021

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Aug 17, 2021

Summary

This issue is caused by export * from './types' added in #4834, which causes everything in /types to be exposed, but unfortunately because export from in the InstantSearch file mixed a type and a value, and thus exported it.

This is done by enabling consistent-type-imports in eslint to make sure it's correct in all cases

references:

Result

all type imports are actually import type or export type now

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 17, 2021

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 807986f:

Sandbox Source
InstantSearch.js Configuration

@Haroenv

This comment has been minimized.

@Haroenv Haroenv requested review from a team, eunjae-lee and tkrugg and removed request for a team August 17, 2021 16:35
Base automatically changed from chore/prettier to master August 18, 2021 08:18
@Haroenv Haroenv force-pushed the fix/type-exports branch 3 times, most recently from 950c019 to 59f0811 Compare August 18, 2021 08:35
This issue is caused by `export * from './types'` added in #4834, which causes everything in /types to be exposed, but unfortunately because `export from` in the InstantSearch file mixed a type and a value, and thus exported it.

This is done by enabling [consistent-type-imports](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/consistent-type-imports.md) in eslint to make sure it's correct in all cases

references:
- code that throws: https://unpkg.com/browse/instantsearch.js@4.27.1/es/types/instantsearch.js
- vite reproduction: https://github.com/eunjae-lee/vue-instantsearch-with-vue3-and-vite
Copy link
Contributor

@eunjae-lee eunjae-lee 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 and works fine on my mac

this catches when you export something that's a type as a regular export, further avoiding the issue
@Haroenv Haroenv merged commit dcbbd88 into master Aug 18, 2021
@Haroenv Haroenv deleted the fix/type-exports branch August 18, 2021 09:07
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