Fix MU addon component invocation

[patricklx]

MU addons are currently not detected as such, and therefore are always merged into the APP namespace, even when its also an MU project.

This also causes MU addons to be namespaced in tests with MU dummy app, which required the change in the acceptance test

That means the this invocation cannot be used anymore (where basic-thing is an addon component)

{{basic-thing}}WOOT!!{{\basic-thing}}

and instead the following must be used

<SomeCoolAddon::BasicThing>WOOT!!</SomeCoolAddon::BasicThing>

[ppcano]

@patricklx

needs emberjs canary version for acceptance test

Should be fixed by #8431. Please, test it if you have time.

• fix ember-module-unification flag detection

I will investigate it. The issue here was commented at #8424 (comment)

Please, could you rename the PR title as WIP - xxxxx?

I think it worths to mention that the goal of this PR is to detect an MU addon. MU addon components will not be automatically merged in the app namespace and they should be called with the MU syntax (Needs more testing and agreement).

Related ember-cli/ember-resolver#298

<SomeCoolAddon::BasicThing>WOOT!!</SomeCoolAddon::BasicThing>

[ppcano]

Now, this PR is working: an MU app must consume an MU addon component with the new MU syntax.

<SomeCoolAddon::BasicThing>WOOT!!</SomeCoolAddon::BasicThing>

I think we should also clarify and include a test for the following issue:

How should a classic app consume an MU addon component?

I tried something similar on the MU addon works with classic dummy app test of #8322 but I had to enable the EMBER_CLI_MODULE_UNIFICATION in the dummy classic app but this does not look to be the correct way.

@patricklx Let me know your thoughts and if you want to check this case.

cc/ @rwjblue

[patricklx]

mmm, I tested it now, extending this branch, by changing the MU flags checks in addon.js here to this.isModuleUnification().
I found that there is a re-export here, which exports the MU addon components, but only if MU flag is ON
https://github.com/ember-cli/ember-cli/blob/master/lib/models/addon.js#L817
treeForApp

• should return the tree if both are classic layout.
• return null if both are MU
• return the re-exported components when addon is MU, but project is classic

2d problem:

• src not in external tree
  so also had to change treeForSrc to check this.isModuleUnification

and then it passes! :) Is that what we want?

I created a similar test like yours and with this changes: https://gist.github.com/patricklx/35ae7c1fc3ccc035e4e528915e9fa8ed

[ppcano]

@patricklx

Yes, I think so. The addon should build its tree based on:

• this.isModuleUnification(): am I an MU or Classic addon?
• this.project.isModuleUnification: is the app MU or Classic?

Your changes looks correct, we have now more MU tests; they should fail if something is wrong.

I created a similar test like yours and with this changes:

I would include the mentioned change and the test in this PR.

This test should validate that a Classic app without EMBER_CLI_MODULE_UNIFICATION can consume an MU addon component and determines the syntax <BasicThing> vs <ns:BasicThing> or both (I don't know which one must be used).

It may be very good if this PR also includes the "inverse" test case:

An MU app with EMBER_CLI_MODULE_UNIFICATION can consume an Classic addon.

Including both tests will supercede #8322.

[ppcano]

@bartocc This issue may be of your interest. You asked in Discord.

is it possible to use an MU addon in a classic app today ?

[bartocc]

thx for the ping @ppcano

[ppcano]

@patricklx

Is there anything pending in this PR? You already added the two tests we talked.

I have reviewed it and suggested some changes.

[ppcano]

This PR supersedes #8322

[patricklx]

@ppcano its ready now,
edit: just saw that I had a change from the style fixes in this branch too

[ppcano]

@rwjblue This is the same issue we discussed at #8322 (comment)

The error is triggered by broccoli/lib/builder before BroccoliFunnel.build is executed; the BroccoliFunnel.allowEmpty routine is never executed.

I can investigate this but I need guidance on where to start.

[ppcano]

@patricklx This should be fixed by https://gist.github.com/ppcano/78e14886c7a660d17eadab709802adb0

Would you mind to include it and check if all the tests pass.

[ppcano]

@patricklx Great!

a6a9419 will be included at #8424 as a separate PR, right? LGTM

I think the title and description could be updated to describe better what the PR does:

The title could be something like: Fix MU addon component invocation or Fix invocation for MU addons ?

The description could show an example how it was working before and how it works now, in order anyone could see easily what the PR changes instead of having to look the PR code.

---

@rwjblue

• WIP - fix MU style merging #8424 depends on this. We have to move forward this PR first.

• Please, read my comment at Fix MU addon component invocation #8431 (comment). I can investigate it now or in a follow-up PR.

[patricklx]

One unrelated test failed... Just needs rerun

[ppcano]

@patricklx could you rebase commits into one?

@rwjblue this PR is a required change for #8424 and it is totally ready now (it does not include the mentioned dirNotFound hack).

[patricklx]

@ppcano @rwjblue rebased, should be ready now!

[ppcano]

This issue should fix #7666

[patricklx]

hi @ppcano @rwjblue anything left to do on this?

[rwjblue]

sorry @patricklx, will try to review soon (a bit swampped just before EmberConf)

[stefanpenner]

As module unification is no more, I believe this can be closed. If I am closing this in error please let me know why, and we can reopen.