@ember/test-helpers 2.8.0 breaks embroider-optimized scenario

[SergeAstapov]

2.8.0 causes following issue:

ERROR in ../node_modules/@ember/test-helpers/setup-rendering-context.js
Module build failed (from ../../../../home/runner/work/tracked-built-ins/tracked-built-ins/node_modules/thread-loader/dist/cjs.js):
Thread Loader (Worker 0)
$TMPDIR/embroider/f71d26/node_modules/@ember/test-helpers/setup-rendering-context.js: Unsafe dynamic component: this.ProvidedComponent in $TMPDIR/embroider/f71d26/node_modules/@ember/test-helpers/setup-rendering-context.js

Can be seen in https://github.com/tracked-tools/tracked-built-ins/runs/6480181579?check_suite_focus=true

I believe Embroider complaints about this line:

https://github.com/emberjs/ember-test-helpers/blob/master/addon-test-support/@ember/test-helpers/setup-rendering-context.ts#L27

Introduced in https://github.com/emberjs/ember-test-helpers/pull/1211/files#diff-be714ad7ae4501cdbd6b1b7ff97b6e0e5b7c73192119f2e0758b5354d72fcd37

Due to complexity of the setup, this does not seem straightforward fix to me.

Seem like one of the options is to configure packageRules, but seem not possible from the addon.

cc @chriskrycho

[ef4]

The problem is not this line:

const INVOKE_PROVIDED_COMPONENT = hbs`<this.ProvidedComponent />`;

It's this one:

const DYNAMIC_INVOKE_PROVIDED_COMPONENT = hbs`{{component this.ProvidedComponent}}`;

It's used in the Ember < 3.25 case here:

ember-test-helpers/addon-test-support/@ember/test-helpers/setup-rendering-context.ts

Lines 173 to 185 in 02360aa

	// Below 3.25, however, we *cannot* treat components as one big object and instead have to
	// register their class and template independently and then invoke them with the `component`
	// helper so they can actually be found by the resolver and rendered
	templateId += 1;
	let name = `-undertest-${templateId}`;
	let componentFullName = `component:${name}`;
	let templateFullName = `template:components/${name}`;
	context = {
	ProvidedComponent: name,
	};
	ownerToRenderFrom.register(componentFullName, templateOrComponent);
	templateOrComponent = DYNAMIC_INVOKE_PROVIDED_COMPONENT;
	ownerToRenderFrom.register(templateFullName, templateOrComponent);

But we can do that case differently. This is what ensureSafeComponent from @embroider/util was made for -- it makes direct invocation work on Ember < 3.25.

[rwjblue]

@ef4 As far as I can tell, the code you linked is roughly the same as what ensure-safe-component is doing. It seems the main difference is that Embroider explicitly trusts ensure-safe-component.

Making @ember/test-helpers depend on @embroider/util is a bit rough, it would have to be a breaking change since @embroider/util@^1 requires Node >= 12, but we still support Node 10. We have generally been planning to release a major when Ember 4.4 becomes an LTS (which is a few weeks from now, when 4.5.0 is released as stable) and we'll be dropping Node < 14, and Ember < 3.24 (but maybe we should just shoot for 3.25?).

I can imagine haxxxors to work around the assertion, basically something that breaks the detection here but that seems really bad/annoying. I guess the right thing is probably to revert the changes (essentially re-release 2.7.0), and wait for our major release in a few weeks.

[ef4]

I'm not suggesting using the template helper ensure-safe-component to avoid tripping the detection. I'm suggesting using the JS utility so that angle bracket invocation (which embroider always considers safe) works on ember < 3.25.

If you don't want to add the dependency, it's easy enough to copy the implementation of ensureSafeComponent into here.

[ef4]

The specific feature I'm talking about is converting a component class to a curried component instance.

[rwjblue]

FWIW, I noticed that 2.8.0 already added @embroider/macros@1 (which had the Node version issue that I mentioned above), so I just submitted #1221 to just use @embroider/util. It doesn't make things worse and so far no one has raised an issue RE: Node 10 support being dropped.

#1221 should unblock folks leveraging Embroider optimized builds, if we get folks raising issues RE: Node 10 we can fix by inlining the ensureSafeComponent code. I just don't want to proactively do all that work (just to throw it away) if no one actually cares. 😩

[rwjblue]

That fix landed and was released in 2.8.1