Skip to content

Added support for filtering unused named imports. #155

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

Closed
wants to merge 2 commits into from

Conversation

MaartenX
Copy link

@MaartenX MaartenX commented May 24, 2020

Uses the AST to check if named imports are used in the transpiled code. If not, filters out the named import. Another option could be to use the typechecker, but then it would break when transpileOnly was true.

Fixes #153.

This way non-emitting types will never be imported so we don't get the following error:

[!] Error: '<Interface>' is not exported by <TypeScriptFile>.ts, imported by <SvelteFile>.svelte

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR relates to an outstanding issue, so please reference it in your PR, or create an explanatory one for discussion. In many cases features are absent for a reason.
  • This message body should clearly illustrate what problems it solves. If there are related issues, remember to reference them.
  • Ideally, include a test that fails without this PR but passes with it. PRs will only be merged once they pass CI. (Remember to npm run lint!)

Tests

  • Run the tests tests with npm test or yarn test

@MaartenX MaartenX force-pushed the import-interfaces branch from 6cccb21 to 49c121c Compare May 24, 2020 14:48
@MaartenX MaartenX force-pushed the import-interfaces branch 14 times, most recently from 44680a4 to c193100 Compare May 26, 2020 15:07
const transformed = await transformer({
content,
filename,
markup: markupCache[svelteFile.filename],
Copy link

Choose a reason for hiding this comment

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

Hi, as a suggestion, I'd move the additional parameter to the end of the param list to be consistent with your additions to other param lists earlier in the file.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Maarten van Sambeek added 2 commits May 29, 2020 09:35
Uses the AST to check if named imports are used in the transpiled code.
If not, filters out the import.

This way non-emitting types will never be imported so we don't get the following error:

```[!] Error: '<Interface>' is not exported by <TypeScriptFile>.ts, imported by <SvelteFile>.svelte```
@MaartenX MaartenX force-pushed the import-interfaces branch from cf2177a to ae7c462 Compare May 29, 2020 07:36
@halfnelson
Copy link
Contributor

This will make the pre-processor much more useful, I run into this ALL THE TIME :)

@kaisermann kaisermann self-assigned this Jun 5, 2020
@orta
Copy link

orta commented Jun 5, 2020

I'm not 100% on all the problems this solves, but can recommending import type everywhere be an answer also? That can guarantee that it should be removed

@kaisermann
Copy link
Member

Hey @MaartenX 😁 ! First of all, thanks for your contribution! After checking this PR and #164, I've decided to go with #164 since it uses, as you suggested, the typechecker while keeping transpileOnly working.

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.

Importing external interfaces fails
4 participants