Add deprecation warning to router service from host

[villander]

It creates a deprecation warning to #756 starting the addition of Engine Router Service.

Also, it provides suggestions for how to register the host's router as hostRouter and/or the root router as appRouter.

Deprecations page has been linked - ember-engines/ember-engines.com#111

[dgeb]

@villander This is a good start - thank you!

[dgeb]

While this PR covers explicit importing of the host's router with the name router, we also need to deprecate accessing the host's router service via lookup, since it is unfortunately automatically injected right now. But let's tackle that in a separate PR.

@rwjblue please review when you have a chance.

[villander]

we also need to deprecate accessing the host's router service via lookup

@dgeb I added it as well, feel free to review it again ; )

[villander]

@rwjblue could you please review this work? I'd like have your eyes here ; )

[dgeb]

If as this discussion indicates, the host's router is NOT being automatically injected, then there is indeed no need to add the deprecation to lookup (which just returns undefined).

I'm confused because this explicitly came up during our last team meeting in which we discussed adding the deprecation to lookup because of this injection. However, if it's not happening, and the only way for the host's router service to be accessed is by explicitly injecting it, then it would seem to me that the deprecation in buildChildEngineInstance is the only one that is needed.

@rwjblue I think it was you who raised the point about automatic injection, so please let me know if I'm misunderstanding something.

Investigating need for deprecation in lookup...

[rwjblue]

@dgeb - The issue isn't that we auto-inject service:router from the top level parent into Engines specifically (because as you mentioned, we don't do that). The issue is that the meaning of the APIs on service:router from within an Engine today is the same as it means from the top level parent (e.g. methods are not properly scoped to the engines mount point). We have to deprecate using service:router (even though it definitely gets its own engine local instance today) because if you call routerService.urlFor(...) you have to invoke it including the mount point, but we want to make it automatically scoped to the mount point. This will be a breaking change, which is why we need this deprecation (to guide folks away from the breakage).

[rwjblue]

Hmm, I tried checking where Ember registers service:router for lookup and it seems that it is only done for @ember/application's registry (here), which would infer that within an Engine @service router is not available unless you manually wire it through (in other words, my comment might be wrong).

Can someone confirm?

[bertdeblock]

@rwjblue Yup, you are right. The router service is not available automatically within an engine.

[dgeb]

@rwjblue @bertdeblock thanks for confirming.

@villander can you please update this PR to remove the lookup deprecation completely? Sorry for all the confusion.

[villander]

Yup @rwjblue @dgeb the service:router is not injected automatically, but many users use router:main to get the router from the host application. It's automatically injected into every engine. Also, we've been using it on #669

Do we want to deprecate the access to router:main into engines or skip it for while? since it's injected on EngineInstance we can deal with that in the next steps?

https://github.com/emberjs/ember.js/blob/21bd70c773dcc4bfe4883d7943e8a68d203b5bad/packages/%40ember/engine/instance.js#L183-L189

@dgeb changes applied, feel free to merge!

[dgeb]

Thanks @villander - I think this looks good now.

[villander]

Could you please merge it @dgeb? I have no privilege to do it

[bertdeblock]

I think we forgot to update enabled: '0.8.13'. Should be enabled: '0.8.16' now?

[dgeb]

@bertdeblock good call - this is corrected in #774