WIP BREAKING CHANGE: Enforce top-level addon

[xg-wang]

Needs #348, closes #292 #290.
This commit supersedes #292 to enforce top-level addon in all cases.

ember-fetch brings in additional assets and supports FastBoot, disallowing used as nested dependency can avoid issues around these.

[xg-wang]

cc @dnalagatla @stefanpenner @rwjblue

[rwjblue]

Do we know how many packages currently depend on ember-fetch that need to be updated for this change?

Sound we emit a warning in the current version (telling folks which thing has the dependency, and that future versions will not allow usage in this way)? Then once we have updated all dependent packages to peer dep + validate, we can release the major bump. What do you think?

[jrjohnson]

Is this only for fastboot support? Our apps don't use fastboot and never will. We do use a common addon that houses dependencies like ember-fetch so we don't have to manage them in the 3 top level apps. Since it is the app itself (and not the addon) that knows if it requires fastboot can this be changed to a warning that is only output for fastboot apps?

A hard requirement here is burdensom for all the apps that don't use fastboot and I don't see fastboot as such a community wide feature that all apps should have to be modified to put up with its quirks.

[rwjblue]

FWIW, I think the primary goal is to only have "one" ember-fetch. The easiest way to do that is to ensure that it is only provided by the project itself.

[xg-wang]

Do we know how many packages currently depend on ember-fetch that need to be updated for this change?
Sound we emit a warning in the current version (telling folks which thing has the dependency, and that future versions will not allow usage in this way)? Then once we have updated all dependent packages to peer dep + validate, we can release the major bump. What do you think?

Did a code search on EmberObserver, there're quite a few addons using as dep. It does need a warning before actually break.

@jrjohnson For FastBoot project it’s a hard requirement. For regular ember apps the problem is more around addons having conflict versions of ember-fetch. This is not specific to ember-fetch. It usually doesn’t cause any problems but when things go wrong it’s often difficult to debug.

FWIW, I think the primary goal is to only have “one” ember-fetch. The easiest way to do that is to ensure that it is only provided by the project itself.

To be more clear, to only have “one” ember-fetch means if the project has ember-fetch, all nested ember-fetch should not import any vendor code. It seems #292 + only import if project has no ember-fetch is a more balanced approach, do you think it's a better path forward?

[jrjohnson]

It seems like a lot of this is immediately resolved by embroiderer when it is released. Is this a temporary solution until that becomes the default build tool?

I know my reluctance here probably seems a bit pedantic, but I feel like if this addon, which is so critical, can't solve this problem in an ergonomic way then that should be seen as a bug in ember-cli.

In my experience peerDependencies are not ergonomic and needing to install and extra NPM package to meet the dependency needs for an addon always strikes me as weird when NPM has a perfectly good way for packages to define their own dependencies so they make it into my app. It gets even more difficult when ember-fetch is nested 2-3 levels away from my app, so I'd be installing a package for an addon I have no idea that I'm using.

FWIW I've struggled in the past with what gets included in a final build and used a variety of hacks to ensure that addons only include themselves at the app layer. I do understand the desire to take this path, but it's a subtle annoyance that everyone will have to deal with which adds up over time.

[xg-wang]

@jrjohnson thanks, this makes a lot of sense. #292 is a more precise enforcement. Let me sync with @dnalagatla on that PR.