emberjs / babel-plugin-ember-template-compilation

Babel implementation of Ember's low-level template-compilation API
9 stars 11 forks source link

fix ts import issue with fcct #31

Closed patricklx closed 1 year ago

NullVoxPopuli commented 1 year ago

Firstly, I really appreciate you trying to fix this problem!!!

however, I don't think we want to go down this route -- "two wrongs don't make a right" -- @ef4 is working on fixing the babel typescript plugin.

patricklx commented 1 year ago

@ef4 @NullVoxPopuli i know there is the attempt to fix it in typescript-transform, but think I have a good fix now. pre runs before any visitor, so it works independent of the order. I do not think there is a good fix to do in typescript-transform, as it needs to to the removals before other plugins, otherwise amd transform etc, will convert the type imports and then it will be difficult to remove them again by typescript-transform.

patricklx commented 1 year ago

The only downside is that it also does not remove unused type imports. If they are annotated with type it works correctly

ef4 commented 1 year ago

I agree that we might need to do our own pre hack if fixes in typescript-transform turn out to be hard to land. And yeah, I had forgotten how annoying amd-transform is for these situations (embroider doesn't use it, but we still need to care about classic builds that do).

I think we can probably extend the ideas here to be more targeted though. For example:

  1. pre could capture a list of imports.
  2. When we're processing each template, we already know what names it needs. For each name:
    • if the name is not in scope
      • and the name was in the original list of imports
      • then we re-insert an import for that name.

This would allow all the normal removal of imports to take its usual course, and limit our changes to precisely the names that we know we need that other plugins can't know that we need.

patricklx commented 1 year ago

I was also trying that first. But reinsert might be to late, since amd transform did already run. I thought dev mode doesn't do template analysis? On the amd transform. I meant it in general, not specific to ember. But we can remove unused specifiers ourselves. It's only about unused imports ( with specifiers). At the end we can identify which are actually used. (if templates are actually analysed)

ef4 commented 1 year ago

I continued this branch in #32.