TS transform breaks imports in templates

[ef4]

@babel/plugin-transform-typescript removes unused imports because it assumes they're types. That's how tsc also behaves so it's at least understandable.

However, it doesn't respect the fact that our plugin is emitting code that does use the imports. Maddeningly, they insist on doing the removal in Program.enter, an unnecessarily painful place to do it, since it won't even wait to see if another plugin might introduce new bindings (which we could be careful to do, as in here.

This adds a failing test.

There are a few options to fix this. If we refactor our plugin to do its work in Program.enter and insist that it needs to com e before typescript we can get in early enough to avoid. But that is pretty gross.

Alternatively, we could require that people set onlyRemoveTypeImports: true in @babel/plugin-transform-typescript. That avoids the problem, but it requires them to be careful to always annotate their type-only imports explicitly.

[ef4]

This fixes the tests, what's not clear is if we really think it's OK to always require this:

diff --git a/__tests__/tests.ts b/__tests__/tests.ts
index 9d5c8fa..035e87d 100644
--- a/__tests__/tests.ts
+++ b/__tests__/tests.ts
@@ -1792,7 +1792,7 @@ describe('htmlbars-inline-precompile', function () {
             targetFormat: 'hbs',
           },
         ],
-        TransformTypescript,
+        [TransformTypescript, { onlyRemoveTypeImports: true } as any],
       ];
 
       let transformed = transform(
@@ -1820,7 +1820,7 @@ describe('htmlbars-inline-precompile', function () {
             targetFormat: 'wire',
           },
         ],
-        TransformTypescript,
+        [TransformTypescript, { onlyRemoveTypeImports: true } as any],
       ];
 
       let transformed = transform(

[NullVoxPopuli]

I think we should probably update our base tsconfig to require type be specified, like svelte has done: https://github.com/tsconfig/bases/blob/main/bases/svelte.json#L14

This should (:crossed_fingers: ) help mitigate an issues people would have with onlyRemoveTypeImports?

The option is new enough where only two configs have it: https://github.com/search?q=repo%3Atsconfig%2Fbases%20verbatimModuleSyntax&type=code

but the rest of the bases are combinable with others, and the ember one isn't so much due to ✨ history ✨ -- we could probably tell ember folks to extend from [bases/esm, bases/ember] eventually tho

[patricklx]

@ef4
I think it runs on enter, as it needs to remove the imports before they get transformed to something else like amd...

another option would also to re-add the imports on program:exit? It will rerun all visitors, but then it would also have the bindings and should not be removed.
But then this plugin would need to run before typescript transform...
Edit:
Also found something called jsx pragma. Not sure if that could be used

[patricklx]

I think with plugin pre function you can access state.file.ast, make a local copy of the imports and re-add the ones used by the templates later.

Or create exports for all imports, that way they would be kept and at the same time also handle type imports correctly