Fix Ember 3.6 deprecation

[boris-petrov]

Use the routeDidChange event instead of the didTransition hook

[webark]

Thanks for taking the time to work on this!

from
https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md

EmberRouter has the following public API today:

• map
• location
• rootURL
• willTransition
• didTransition

That API will be carried over verbatim to RouterService, and the publicly accessible Ember.Router class will become RouterService. In terms of implementation, I expect the existing EmberRouter class will continue to exist mostly unchanged. But public access to it will be moderated through RouterService.

I think what is needed here is to just extend the RouterService, not change out the hook.

[webark]

But the routeDidChange might be a better hook though for this. I’ll look into it and get back to you.

Why did you do an onInit hook rather then just defining init and calling super? It was my understanding that was the preferred way, rather then adding through the event hook. But i could be mistaken.

[boris-petrov]

I'm not sure about your first comment whether it is better to do it by extending the RouterService, if you think it is, go ahead, it's definitely fine by me. :)

About the second part - I prefer not overriding init when using reopen or something like that. But actually you are right that it doesn't matter. I don't think that one way is preferred over the other.

[webark]

ya the

But public access to it will be moderated through RouterService.

made me think that adding functionality to the Router directly is discouraged.

[webark]

need to reset the travis config.

[webark]

@boris-petrov had to close and reopen this to get the new travis config pulled. This is working now in the new version, but breaking in the old.

I think we should move the functionality into a standalone method that gets passed the routes, and the "owner" then either run a check on version that will do the appropriate hook, or see if there is some kind of polly fill for routeDidChange

[boris-petrov]

Ah, yes, I did not think of that. I see here that there is no such polyfill as of now. I've never written an addon so I'm not sure what's the normal way to handle these things. There is always the option to drop support for Ember < 3 and bump the major version of the plugin. Otherwise I'm not sure how to proceed.

P.S. Is it possible to check the version of Ember at runtime? If so, then yes, we could just have two implementations.

[webark]

I know you can check the ember version.. But if there was a way to check for the hook, that would be better.. I’ll do some digging and get back to you.

[mydea]

What about making this opt-in? We don't even use this in our app (e.g. we only use component styles), and you need some manual setup for this anyhow (adding class={{routeStyleNamespaceClassSet}}). Also, for all apps that don't need that, it adds a minor-but-unnecessary overhead to every page transition.

So I would propose to remove the re-export in app/initializers/route-styles, and let the user add this manually - there could also be a blueprint to do that more easily. This way, there could be two versions (e.g. with the router service and with the didTransition hook), and every app can decide what to import (or nothing).

[webark]

@mydea That’s not a bad idea..!! Hmm.. Ok. I’ll work on that next week unless someone else gets to it first.

[cibernox]

I came here to do the same fix, glad to see someone is working on it already!

Keep up the good work!

[webark]

fix with #308 . thanks so much @boris-petrov !! 🙏🙇‍♂️