issue with gts/gjs if ts transform is enabled

[patricklx]

Since ts babel plugin removes the unused (but used in template) imports at program enter, this plugin doesn't have a chance to analyse them.

This plugin probably needs to do the same as ember babel template compilation.
Or just analyse the imports at program exit

Maybe the approach needs some rethink as every plugin that wants to analyse imports of gts files needs to adop the same behaviour.

[josemarluedke]

I believe I'm getting into this problem.

I have a pkg that exports components in the root of the pkg (import {Button} from '@frontile/buttons'),
When using the Button component, it ends up with an error in the browser saying that the component is undefined.

This is happening in gts files, have not tested in gjs. If I do a console.log referencing the Button component, that then works, probably because it understands that is being used, but not when only using in the template.

Works

import {Button} from '@frontile/buttons';
console.log(Button);

<template>
  <Button>Test</Button>
</template>

Does not work

import {Button} from '@frontile/buttons';

<template>
  <Button>Test</Button>
</template>

Versions:

"ember-auto-import": "^2.7.2
"ember-template-imports": "^4.1.0",
"ember-cli-babel": "^8.2.0",
"@frontile/buttons": "0.17.0-alpha.12",

Note 1: @frontile/buttons pkg is a v2 addon.
Note 2: When using Embroider in another project, this is not a problem. This is a problem only when not using Embroider.

[chancancode]

For apps that are already disambiguating by using import type, we can avoid the problem by instructing compiler to only strip type imports. This is typically done with using the verbatimModuleSyntax compiler option for tsc, but since we are using babel not tsc here, it would be the onlyRemoveTypeImports option for the babel plugin. Problem is, it would have to be added here and right now there is no way for apps to enable that. I used patch-package to add it and confirmed that it helped.

I think we should definitely have a way to enable that option, and that would be good enough for my app (but we may also just switch to Embroider soon anyway). But I am not sure we can rely on only that solution (essentially saying .gts requires verbatimModuleSyntax), perhaps there are reasons why it is not feasible for some apps to do that?

[NullVoxPopuli]

@chancancode I think we could even go as far as to lint against "incorrect" tsconfig.jsons and babel configs.

I think we have a problem of too much config right now, (as a whole JS ecosystem), and there are too many ways to do things incorrectly.

Like, absolutely no one should not be using onlyRemoveTypeImports (it's not TS' (the babel plugin) job to mess with your imports, how dare it, tbh, lol). and verbatimModuleSyntax should be a default, no config (anyone want to PR TypeScript? 😅 ). These two options play nicely together, and it's frustrating that it's so easy to get wrong :(

[patricklx]

I'm looking how we could fix this in babel. But there is no straightforward way...
We cannot use pre to analyse imports since we cannot know ourselves if the imports will be removed later or not.
And program:exit might also not work, since amd module transfor will transform the imports.
We would need something right before program:exit. But I couldn't find anything.
The best thing we can do i think is to add some placeholder expression at the end of the body. Like emptyExpression with a custom property.
When we enter it we should be at the end and can analyse the imports.
I will make a pr for it

[chancancode]

Probably the most reliable thing is to restage our pipeline so we resolve the eval earlier, as you said it in the OP it could be a problem for other things too

[patricklx]

well, it could also be any other babel plugin that adds imports dynamically. So I think it needs to be fixed anyway in EAI

[patricklx]

I actually found nowthat there is also path.requeue. That could also fix it.

[patricklx]

hmm,
https://github.com/ef4/babel-import-util/blob/main/src/index.ts#L132

[ef4]

Probably the most reliable thing is to restage our pipeline so we resolve the eval earlier, as you said it in the OP it could be a problem for other things too

I think the best "earlier" would mean making content-tag itself pluggable so it can accept a callback that would invoke glimmer's precompile. Then the whole transformation could happen in the content-tag stage and all the downstream tooling would not see the eval form. I think it's doable, but it's not trivial given that this does require implementing bindImport, bindExpression, etc behavior within content-tag.

Alternatively, we could introduce an extra babel pass. At a minimum this would double the traverse cost, at a maximum it would double the parse cost too. I think a double-parse would be unacceptably expensive, but an additional traverse might be ok.

In either case, it's best if the same stage that produces the scope bag is the one that precompiles the template to wire format, because both require parsing the template and it's better not to repeat that work.

[ef4]

hmm,
https://github.com/ef4/babel-import-util/blob/main/src/index.ts#L132

Yeah, the babel plugin ecosystem is kinda a mess when it comes to the mutation APIs. Lots of plugins don't use them at all, some plugins will break if you try to use the methods in your plugin.

In babel-import-util we don't use them, but we also manually ensure that babel's scope-aware APIs are updated to see the changes we're introducing. (That being one of the major benefits in theory of using the mutation apis.)

[patricklx]

Alternatively, we could introduce an extra babel pass. At a minimum this would double the traverse cost, at a maximum it would double the parse cost too. I think a double-parse would be unacceptably expensive, but an additional traverse might be ok

For example, doing a traverse in pre handler in template compilation plugin?

[ef4]

Yes, that might work.