Resolve "nested layout" components

[Windvis]

This adds a new (failing) "nested-layout-components" scenario and the fix for it in a second commit.

Supersedes #1587 since it's not only a v2 addon issue.

[Windvis]

When I added a similar line to the componentTemplateCandidates generator method I ran into an issue with the v2 addon component. Something about not being allowed to call setComponentTemplate twice. My guess is that the v2 component is resolved twice in that scenario, since the extensions logic checks for both .js and .hbs files.

I haven't dug deeper to find the origins of the current setup. Adding it only here made the tests pass and I noticed yield { prefix: '/components/', suffix: '' }; wasn't added to componentTemplateCandidates either, which might be because of similar reasons?

[Windvis]

Creating a new scenario adds a lot of extra CI jobs. Should I try to fit it in another scenario?

[ef4]

Yes please. If this fix is entirely in module-resolver.ts it can get tested in core-resolve-test.ts.

[ef4]

Thanks for working on this.

I was guessing the fix for this would actually be at a slightly different spot. But I haven't dug in myself yet, so your solution may also be right.

Is the problematic case the one where componentJSCandidates() is called by tryReverseComponent() or is it the one where it's called by resolveComponent()? I think in resolveComponent() this fix would be superfluous because there the output is sent to nodeResolve, and that would necessarily handle index.js automatically.

If the problematic case happens only inside tryReverseComponent then the fix may be better inside that method.

I was also suspecting the problem could be in getEntryFromMergeMap(), because there we match over multiple candidate extensions but potentially should also include the index variant of each extension as well. If this is an issue for addon-provided components but not app-provided components, it's the mergeMap I'd expect to be the problem.

[Windvis]

Is the problematic case the one where componentJSCandidates() is called by tryReverseComponent() or is it the one where it's called by resolveComponent()? I think in resolveComponent() this fix would be superfluous because there the output is sent to nodeResolve, and that would necessarily handle index.js automatically.

As far as I can see, it's the resolveComponent code path where it goes wrong. nodeResolve does get called, but it tries to look up the app reexports of those addon components and they don't seem to be there when I check the folder locally.

For example, in the tests I added here it looks for app-template/components/v1-nested-layout-component in a tmp folder, so it should in theory look for the /index.* version as well, but that folder isn't there. It would then go to the fallbackResolve code path where the /index suffixed one does get found.

I was also suspecting the problem could be in getEntryFromMergeMap(), because there we match over multiple candidate extensions but potentially should also include the index variant of each extension as well. If this is an issue for addon-provided components but not app-provided components, it's the mergeMap I'd expect to be the problem.

Yea, I originally planned to do it like that, but then I saw that the componentJSCandidates method also had special handling for pod layouts and this seemed more in line with that.

In any case, it's probably going to take me a while to grasp the logic we're dealing with here, so no hard feelings if you would rather look into it yourself to find the real underlying issue 😄.

[lolmaus]

@Windvis If you happen to need a strict mode test (as you mentioned in your previous PR), you can use this draft as a pattern, except enable the strict mode for an individual test case.

[ef4]

I don't fully understand why this would be the correct place to fix this bug. /index is already implicitly allowed by nodeResolve. Maybe the bug here is really in the app-js matching?

We can use the test from here to debug it.

[Windvis]

Closing since I won't have the time to look into this again and it wasn't the correct fix anyways.