Fix for addons using ember-cli-typescript 4x

[jamescdavis]

Fixes #528

We need to trigger a custom Babel run for any addons using ember-cli-typescript 4.

[ef4]

Thanks for working on this!

[ef4]

I didn't think this would be necessary, because ember-cli-babel adds the typescript plugin when it sees that ember-cli-typescript 4x is present?

Your fix above in needsCustomBabel should ensure that we allow ember-cli-babel to do its usual thing (other than the changes we make to its own settings like compileModules: false).

If this line is really needed to make it work, we should debug why because then I don't understand it.

[ef4]

Also, if we do keep this line, we should probably guard against double insertion of the plugin. If anybody else has already inserted it, babel will throw when it sees the duplicate.

For an example of where we already do this, see

embroider/packages/webpack/src/ember-webpack.ts

Lines 500 to 502 in 3d48872

	if (!options.plugins.some(pluginMatches(/@babel\/plugin-proposal-optional-chaining/))) {
	options.plugins.push(require.resolve('@babel/plugin-proposal-optional-chaining'));
	}

[ef4]

Also, this is resolving the plugin relative to @embroider/compat, which doesn't have a dependency on @babel/plugin-transform-typescript. It needs to resolve relative to ember-cli-babel's root, because AFAIK that is the package that depends on the plugin. But again, this is one more reason I think it's suspicious to add the plugin here -- I think ember-cli-babel should probably be doing it.

[jamescdavis]

You're totally right. All it needed was a6567f2!

I'm not sure how I ended up down the rabbit hole where I thought I needed to have Embroider add the plugin. ¯\_(ツ)_/¯

[ef4]

This is marked draft still but I think it's ready?

[jamescdavis]

@ef4 I've been trying to add an app that tests this test-packages, but having some issues getting it to properly fail without this fix. I was able to basically airlift https://github.com/scalvert/a11y-embroider-repro/tree/broken and it fails the build without the fix (with the same issue seen in that repro) and builds correctly with the fix, but the tests end up failing due to other issues (can't find @ember/destroyable). I think it's a weird workspaces issue. I'll go ahead and take this out of draft and work on getting a proper test-case in later.

[ef4]

I'm hesitant to keep adding separate test apps anyway. It's a lot to maintain and it's slow. People keep doing it because it's more obvious, but I'd encourage more tests that look like these:

https://github.com/embroider-build/embroider/blob/master/packages/compat/tests/app-import.test.ts
https://github.com/embroider-build/embroider/blob/master/packages/compat/tests/addon-styles.test.ts

[jamescdavis]

Yeah, it's very slow. Those tests are a much better strategy.