resolver transform to emit imports for helper and modifiers that need…

[void-mAlex]

… them in strict mode templates

[mansona]

This looks good to me 👍 I think that this is in line with what we discussed in meetings and the fact that tests are passing gives me the confidence that this can be merged and released without much issue 👍

[ef4]

I pushed the above commits to clarify the code and explore how much was already correct here. By unifying the builtInKeywords list the types uncovered a case that wasn't handled (the use of built-in in ambiguous forms).

But more changes are still needed here. The built-ins don't always win. For example, if you make app/components/input.hbs in a classic app, you'll see that it defeats the ember-provided <Input />.

Helpers are a little better. If you try to create your own app/helpers/fn.js it will throw when you try to invoke it. But there is also the problem of version skew. uniq-id was introduced in ember 4.4, but embroider stable supports back to 3.28. On 3.28, uniq-id might actually come from a polyfill addon.

Whatever decisions resolver-transform makes get cached in the babel-cache, and will endlessly troll people if the answers can change based on other side-effects in the system. Like upgrading your ember version across the 4.4. boundary. That is a big reason why we split the component/helper/modifier resolution out of here and into module-resolver.

All of the above are reasons why I don't think we can hard-code these answers in resolver-transform. Rather, resolver-transform should delegate these cases to module-resolver, where we have much more information about what is available across the app and all addons, and where the answers won't live inside the babel cache.

For example, I think we would entirely remove input from the builtInKeywords list here and instead update module-resolver so it resolves #embroider_compat/components/input and #embroider/compat/ambiguous/input to the virtual content export { Input as default } from '@ember/component'.

I realize that might seem like it adds extra work for your codemod use case, but I don't think it does. The codemod is already going to need to know how to follow a reexport like that, because effectively 100% of addon-provided components will go through similar reexports, and the codemod will need to follow through them. That is, <PowerSelect /> becomes an import from #embroider_compat/components/power-select which resolves to your-app-name/components/power-select which resolves to a file containing export { default } from 'ember-power-select/components/power-select'. It's this last specifier that the codemod wants to output.

[void-mAlex]

Thank you for this @ef4 ,
I do have to point out that in my work I've found that #embroider_compat/components/power-select resolves directly to a file in the re-written packages and not something from the app scope, so I'm not convinced about the addon provided components needing this treatment as I have tested this scenario (though not very extensively) and it behaves differently - as far as I've seen I get straight to the path after the re-export but more testing is needed.

also uniqId is exported as uniqueId - I'll fix that up unless there is a specific reason the former one was used.

I'll work to fix up the tests as well

more to do ❤️

[mansona]

FYI I just rebased this against stable 👍