Support import paths with .gts extensions

[NullVoxPopuli]

I also opened this here: embroider-build/addon-blueprint#194

But I have a repro here: universal-ember/ember-primitives#114

error looks like this:

[js] created dist in 738ms
[js] npm run build:js exited with code 0
[types] src/components/portal.gts:4:49 - error TS2307: Cannot find module './portal-targets.gts' or its corresponding type declarations.
[types] 
[types] 4 import { findNearestTarget, type TARGETS } from './portal-targets.gts';
[types]                                                   ~~~~~~~~~~~~~~~~~~~~~~
[types] 

Note that gts extensions were used in the import path to avoid needing @rollup/plugin-node-resolve

I already have:

allowImportingTsExtensions: true

so I guess I'd expect it to also work for gts extensions?

[lukemelia]

@ef4 and I did some spelunking here.

As a spike, this issue is fixed by rewriting .gts imports to .ts imports immediately before this line:

glint/packages/core/src/transform/template/rewrite-module.ts

Line 54 in 5861a8b

return new TransformedModule(contents, errors, directives, correlatedSpans);

The approach to doing this given the current architecture of rewriteModule is to capture metadata about this change in parseScript called by calculateCorrelatedSpans and allow it to be applied in calculateTransformedSource. This might require some changes to the typing of emitMetadata.

While working on this, we were very confused that the transformed ast from parseScript was discarded and not actually used. What's the reason for this approach? We assume whitespace preservation, perhaps? If there is an appetite to change this approach to make it less surprising, we could do it as part of the import rewrite.

[ef4]

While working on this, we were very confused that the transformed ast from parseScript was discarded and not actually used.

Yeah, this is related to what I had proposed in point 3 of this old comment: #297 (comment)

Now that we've stabilized the pure-JS representation of templates in https://github.com/emberjs/rfcs/blob/master/text/0931-template-compiler-api.md, if I had the chance to redo the glint environment API I would want to change it so the environments emit the standard template representation plus source maps, and glint core would only need to understand that.

As it is, the environments aren't really allowed to do transformations and thus they leak a lot of knowledge into core.

[dfreeman]

While working on this, we were very confused that the transformed ast from parseScript was discarded and not actually used.

The TS compiler APIs don't (or at least didn't, to my knowledge) have a good consistent touchpoint for handing off a modified AST. They do have a transformer interface, but its availability varies across the different invocation versions (check vs watch vs project-build vs language service, etc).

We assume whitespace preservation, perhaps?

The TS AST (or closer to a CST) is actually quite good at maintaining details about trivia like whitespace, though that also makes it very finicky to work with. In part for that reason, the transform originally used Babel, which was much easier to maintain, but also much slower (not to mention heavier dependency-wise). We now do the bare minimum of node maintenance to ensure we can reserialize the tree back to source code, but it would likely require more detailed effort to make sure we're producing one that will fully pass muster in memory.

Yeah, this is related to what I had proposed in point 3 of this old comment: #297 (comment)

As I said in that thread, and in several other contexts since then, the goal was not that @glint/environment-ember-template-imports would be a long-lived package, but rather to provide the bare minimum to allow experimental support for potential strict-mode syntax like its namesake until an official syntax was finalized.

Directionally, the idea was that the core transformation logic would become aware of the final syntax and we'd no longer need to maintain an interface for environments to manage transformations.

if I had the chance to redo the glint environment API I would want to change it so the environments emit the standard template representation plus source maps, and glint core would only need to understand that

If you anticipate further experimentation in the syntactic space for how templates are embedded within scripts, then formalizing something like that could be valuable. We initially investigated and discarded the idea of using sourcemaps for handling location mapping, though I don't remember the specifics of that decision any more. It's certainly nice that you get composability 'for free' with that approach.

On the other hand, if you don't foresee new embedding mechanisms happening (even if the template syntax itself changes), then my personal take is that effort would be better spent on having @glint/core understand .gjs/.gts "natively" than on reworking the environment transformation API. Like I said in the linked thread, ultimately I'd love to see the preprocess and transform hooks be deprecated and removed.

The caveat I'll offer on all of the above, though, is that the primary reason I stepped down from the Ember TS team is that I don't have the capacity any more to actively shepherd this project. Given that, if you feel differently about what's worth investing in or see opportunities to simplify functionality that I didn't, then as long as the Ember TS team is on board I certainly won't have any objection.

[lukemelia]

Based on the above and reflecting that we may need wider analysis to decide how to adjust glint's packages, I'm going to create a narrow PR to address this specific issue keeping the current architecture.

[NullVoxPopuli]

This is not resolved -- followup issue here: #628