@ember/test-helpers is trying to import from ember-testing but that is not one of its explicit dependencies

[simonihmig]

Also reported here, but the root cause seems to be in Embroider: emberjs/ember-test-helpers#1505

Context:

• using @ember/test-helpers@4.0.1, which is importing from the virtual ember-testing module as of lately (Remove old code that existed for now unsupported ember-sources (2.4, etc) emberjs/ember-test-helpers#1486)
• using @embroider/core@3.4.14 (or probably any other version pre 3.4.15, that hasn't Update ember standard modules to include @ember/renderer and @ember/-internals and ember-testing #2075)
• running the build with Embroider and safe mode (optimized is working!)

I was debugging what is going in Embroider's module resolver, my findings:

As said, under Embroider optimized this is working fine. The request for ember-testing is rewritten in handleRenaming to ember-source/ember-testing/index.js, which seems to do the trick:

The same is not happening under safe mode, it doesn't have that full list of renameModules that optimized has:

But I assume that is expected?

An import request for ember-testing is handled here, as the specifier matches what @ember/test-helpers has declared as externals in its package.json. However, there are follow-up requests for deeper module paths like ember-testing/lib/test/pending_request, where isExplicitlyExternal() is returning false, as it checks for equality (by using .includes()):

In that case, that request is running then into throwing the exception here.

When upgrading to latest @embroider/core (that has emberjs/ember-test-helpers#1486), things are working as the resolver is having ember-testing in emberVirtualPackages, so exits early here.

So upgrading is a fix for this situation. But it seems to me isExplicitlyExternal is not handling this case correctly. I am naively thinking, that a fix could be as simple as replacing .includes() with .startsWith() here.?

Before going further and filing a PR, I wanted to make sure this makes sense and my observations and assumptions are correct here!

Would still require users to upgrade to latest core though! 🙃