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(analyzer): ignore type-only imports & exports #380

Merged
merged 3 commits into from Aug 3, 2021
Merged

Conversation

buschtoens
Copy link
Collaborator

Prevents type-only imports / exports from ending up in the final bundle.

import type { compute } from 'heavy-library';

export async function performHeavyComputation(
  ...args: Parameters<typeof compute>
): Promise<ReturnType<typeof compute>> {
  const { compute } = await import('heavy-library');
  return compute(...args);
}

@ef4
Copy link
Collaborator

ef4 commented Apr 9, 2021

Thanks for working on this, this will be nice.

@boris-petrov
Copy link
Contributor

Does this fix this issue (i.e. this comment in that issue)?

@ef4
Copy link
Collaborator

ef4 commented Apr 11, 2021

Yes, I think this would fix that.

@jamescdavis
Copy link
Collaborator

Bump. 🙂

If the TS syntax plugin is not loaded, the following scenarios fail, as the `import type` syntax is not understood:
- `lts-own`
- `release-own`
- `beta-own`
- `canary-own`
@buschtoens
Copy link
Collaborator Author

Alright, sorry that I kept this idling for so long, but I had no clue and time to dig into, why the 4 *-own scenarios were failing, while all other were running successfully. I finally got around to it now!

The issue was that all scenarios, except for *-own, seem to directly or indirectly enable the TypeScript syntax in the babel options. Without the TS syntax enabled, TS constructs like import type ... cannot be parsed, which is obvious in hindsight, but was quite annoying to debug, as there was no apparent syntax error getting thrown. 😅

I've added @babel/plugin-syntax-typescript to the babelOptions in the beforeEach hook via 1440052, which fixes the issue. I don't really know, whether this solution is acceptable / ideal though, or whether I should opt-in to TS somewhere else.
Any pointers would be greatly appreciated!

I've also rebased the PR and it's all working fine now. ✅

Please let me know, whether I should add more tests, or whether I should change the TS syntax opt-in.

@buschtoens buschtoens marked this pull request as ready for review July 23, 2021 19:48
@buschtoens buschtoens requested a review from ef4 July 23, 2021 19:48
@ef4 ef4 merged commit c748055 into master Aug 3, 2021
@ef4 ef4 deleted the erase-type-imports branch August 3, 2021 18:41
@ef4
Copy link
Collaborator

ef4 commented Aug 3, 2021

Looks great, thanks.

buschtoens added a commit that referenced this pull request Aug 13, 2021
Backports #380.

* fix(analyzer): ignore type-only imports & exports

* test(analyzer): type-only imports ignored in created file

* test(analyzer): add `@babel/plugin-syntax-typescript`

If the TS syntax plugin is not loaded, the following scenarios fail, as the `import type` syntax is not understood:
- `lts-own`
- `release-own`
- `beta-own`
- `canary-own`

* test(analyzer): conditional `babel@7` tests

The `import type` syntax is only available with `babel@7`, but
`ember-auto-import@1.x` only tests for `babel@6`.

This commit makes the `analyzer` test module run twice: once for
`babel@6` an once for `babel@7`, where the latter has a different babel
plugin configuration and runs the addition `import type` test.
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

4 participants