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/remove type imports typescript #164

Merged

Conversation

halfnelson
Copy link
Contributor

Current problem

Imported interfaces and types cause rollup etc to choke as they are no longer stripped. see: #153

Fix

I noticed that the typescript forces them back in as part of a transform. I add support for the detection of import type { AInterface } ... style imports that can be skipped. There is a more powerful version for when the transformer isn't just doing transpile only. It uses the type checker to see if the symbol represents an interface or type alias and removes it.

This is the second attempt at a fix. The existing PR ( #155 ) removes the import if the value is unused in the script, but it may remove imports in error if the import is only used as prop values.

Tests

I added a bunch of tests for this feature that now pass. The less.js test was already failing before the changes.

@kaisermann kaisermann self-requested a review June 5, 2020 18:35
@kaisermann kaisermann self-assigned this Jun 5, 2020
@kaisermann kaisermann force-pushed the fix/remove-type-imports-typescript branch from fe6ef3d to b5eb427 Compare June 6, 2020 03:04
@kaisermann kaisermann added the bug Something isn't working label Jun 6, 2020
@kaisermann kaisermann force-pushed the fix/remove-type-imports-typescript branch from cd2abe7 to 8c9c7b2 Compare June 6, 2020 03:30
@kaisermann
Copy link
Member

Hey @halfnelson 👋 Thanks for the great pull request! The typescript API has always been kinda cryptic too me 😅. Released in v3.9.3 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants