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

Add support for ignore named imports and exports #57

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

waivital
Copy link
Contributor

@waivital waivital commented Oct 13, 2023

When the skipTypeImports option is enabled, only ignoring type imports is not sufficient. With TypeScript 4.5, it also supports named type imports and named type exports.

@XhmikosR
Copy link
Member

XhmikosR commented Apr 4, 2024

While you have added tests, I'm not super familiar with the changes to be able to review your PR. Maybe someone else can review it.

@waivital
Copy link
Contributor Author

waivital commented Apr 4, 2024

Okay, and let me also add more context to this changes.

This PR extends the option skipTypeImports, which was originally introduced by THIS ONE. I think the idea behind this changes is the type-only imports and export will not actually contribute to the dependancy graph after typescript is compiled to javascript, so sometimes when analyzing dependencies, being able to skip these type-only dependencies will make the output simpler.

When TypeScript 4.5 is released, it allows a type modifier on individual named imports. If this new syntax is used in the code, the old logic for skipTypeImports will not take effect.

@XhmikosR
Copy link
Member

XhmikosR commented Apr 9, 2024

Let's try this one, the logic LGTM!

@XhmikosR XhmikosR merged commit 15f4b01 into dependents:main Apr 9, 2024
11 checks passed
@XhmikosR XhmikosR changed the title Add supports for ignore named imports and exports Add support for ignore named imports and exports Apr 9, 2024
@XhmikosR
Copy link
Member

XhmikosR commented Apr 9, 2024

Released as https://github.com/dependents/detective-typescript/releases/tag/v11.2.0, I hope we didn't break anything 🙂

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