New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What is the `resolve` equivalent in the new router? #100

Open
PascalPrecht opened this Issue Feb 15, 2015 · 41 comments

Comments

Projects
None yet
@PascalPrecht
Copy link
Contributor

PascalPrecht commented Feb 15, 2015

I read the source code and also the existing documentation you wrote for the router. Since I try to also sort of compare the old with the new router in the article, I need to explain the resolve $routeProvider config equivalent of the new router.

As far as I understand, instead of having this resolve block, we use canActivate and activate for it. Am I getting this right?

Whereas canActivate decides with the routing happens at all, activate is in charge of doing all the work that is needed to make needed data available in the controller. As you show in the docs, this could be something like:

MyController.prototype.activate = function() {
  return this.bigFiles = this.$http.downloadBigFiles();
};

this.bigFiles is what we usually would request in a resolve block. Please correct me if I'm wrong.

However, I still wonder how this behaves, if we have asynchronous work todo (actually the example looks asynchronous too). In the old router, resolve properties were injected into the controller that is associated to the view.

I understand that we don't need that anymore, as long as we just expose data on this (in the example this.bigFiles), since controller methods can just access it. But what if we have a promise, that needs to resolve first? Is that functionality also (still) supported in the new router?

Hope my question is clear.

@PascalPrecht PascalPrecht changed the title `resolve` equivalent in new router Do we have a `resolve` equivalent in the new router? Feb 15, 2015

@PascalPrecht

This comment has been minimized.

Copy link
Contributor

PascalPrecht commented Feb 15, 2015

I've updated the issue title, so it can be properly used as FAQ when searching by labels.

@PascalPrecht PascalPrecht changed the title Do we have a `resolve` equivalent in the new router? What is the `resolve` equivalent in the new router? Feb 15, 2015

@btford btford added this to the v0.3.0 milestone Feb 16, 2015

@andrezero

This comment has been minimized.

Copy link

andrezero commented Feb 19, 2015

Hi

It's really cool to have these convention based can methods invoked in the controllers but it's not an alternative for some use cases where having a resolve mechanism defined within the route (and not the controller) is very useful.

For instance, I use resolve for asynchronous authorisation of the requested route+params. These are properties of the router and not the controller and I can assemble two different apps using the same controllers with different authorisation schemes.

Cheers

@niemyjski

This comment has been minimized.

Copy link

niemyjski commented Mar 6, 2015

Not to change the subject but I really never liked the use of resolve for anything more than a parameter value that would already be in the route/state params...

So in your case you are resolving big files, and now you are putting logic that knows how to populate that in your router logic... What do you do when big files changes (aka you know it changes from someplace / you get a notification via websocket). Now your only option is to have the get code for big files in two spots or reload the whole view and all children views... Unless, there is an easy way to accomplish reloading your resolved data without destroying the controller and view... I think we should avoid against using resolve when showing people how to load mutable controller data as it could promote a bad practice...

@btford, @PascalPrecht Thoughts??

@kpgarrod

This comment has been minimized.

Copy link

kpgarrod commented Mar 10, 2015

John Papa's style guide (https://github.com/johnpapa/angular-styleguide#style-y081) recommends the use of route resolves in certain circumstances and provides some good reasons.

@PascalPrecht

This comment has been minimized.

Copy link
Contributor

PascalPrecht commented Mar 18, 2015

@niemyjski Thanks for you input. So the bigFiles thing I've mentioned is just an example. It could be anything else. It totally makes sense to have a mechanism like resolve of ngRoute where work needs to be done before the view change takes place. E.g. if you have view/component that only makes sense to be loaded with some specific data you do want to make sure that it's available before the view change happens.

However, I agree with you that it's a rather not so nice way to populate that kind of logic into router logic (as it is currently the case for resolve in ngRoute) but that's why I asked the question here. I've asked the same question via mail and @btford didn't say "yes" to canActivate().

So for me it's still a bit unclear.

@niemyjski

This comment has been minimized.

Copy link

niemyjski commented Mar 18, 2015

Yeah, I'm in the same boat. Even with uirouter, they seem to promote this "bad" practice throughout. It seems like this should be in a style guide.. I'm in agreement with John Papa (a little bit). I like passing in a function / service to the controller to do my lifting. It seems like this should be in a style guide.. I'm in agreement with john papa (a little bit). I like passing in a function / service to the controller to do my lifting. I just find that i'm always using state params more than I probably should..

@btford btford modified the milestones: v0.5.0, v0.6.0 Mar 19, 2015

@cjolif

This comment has been minimized.

Copy link

cjolif commented Mar 24, 2015

People that were leveraging resolve to use AMD to load controllers, directives, service on the fly won't be able to leverage canActivate to achieve similar results. So there is definitely some kind of regression here?

@btford

This comment has been minimized.

Copy link
Contributor

btford commented Mar 24, 2015

tl;dr – all lifecycle hooks are injectable, but I expect devs will want to use the activate hook like resolve was used before.

@PascalPrecht

This comment has been minimized.

Copy link
Contributor

PascalPrecht commented Mar 24, 2015

@btford maybe you can show a simple snippet that would work as an equivalent to resolve?

"all lifecycle hooks are injectable" doesn't really help me here... what do you mean by that? Lifecycle hooks are canActivate, activate etc. I don't think you'd inject those somewhere else right?

@btford btford added the type: docs label Mar 28, 2015

@Narretz

This comment has been minimized.

Copy link

Narretz commented Mar 30, 2015

Hm, I also liked the resolve previous resolve system better, because it decoupled the routing from the actual controller.

@kpgarrod

This comment has been minimized.

Copy link

kpgarrod commented Mar 31, 2015

I agree. I think it is particularly useful for reusable components because
you can pass in different data depending on the context.

On Mon, 30 Mar 2015 22:37 Martin Staffa notifications@github.com wrote:

Hm, I also liked the resolve previous resolve system better, because it
decoupled the routing from the actual controller.


Reply to this email directly or view it on GitHub
#100 (comment).

@damiengarbarino

This comment has been minimized.

Copy link

damiengarbarino commented Mar 31, 2015

Any news on the sample to replace the resolve in the AMD use case? In my use case, I'm loading all directives/services/controllers I need and the view controller itself as well in this resolve function. Thanks!

@ghost

This comment has been minimized.

Copy link

ghost commented Apr 8, 2015

Resolve was perfect as it allowed me to read necessary configuration from the server via $http before my controllers were instantiated. My app is basically useless without the config being loaded first.
Using canActivate seemed to be a shoo-in for this due to the ability to cancel the route however I see that the controller is initialised before the promise is resolved.
So basically a resolve like function is still needed or is there a better way to do this pre-application initialisation? Preferably without doing the HTTP request outside of angular and manually bootstrapping!

@tommck

This comment has been minimized.

Copy link

tommck commented Apr 8, 2015

I'm not sure I understand what can't be done with the new life cycle events. The constructor for the controller will be run, yes, but the UI won't be rendered until activate() is finished, so anything that depended on the data being there would just be inside your activate() method.

Do you have a specific situation where this won't work? Maybe a [pseudo]code sample?

@ghost

This comment has been minimized.

Copy link

ghost commented Apr 8, 2015

I´ve uploaded a quick and dirty project. Maybe I am doing this wrong, but it is similar to what I am trying to achieve in the proper application.
https://github.com/heidi-de/new-router-test
Ideally I want the initialisation of the component controller to be delayed until the promise resolves, but as you can see when it runs, the component controller is run first, the promise will become resolved, but the jsontest variable is never updated with the ip address.
Apologies for the narcissistic component naming :-)
In my real app I need the configuration object in order to set the apps language and also lay out the views based on the which roles the user has.

@tommck

This comment has been minimized.

Copy link

tommck commented Apr 8, 2015

your code has a bug in it.. you're setting "this.jsontest" equal to a Promise object. I'm sending a pull request with a tweak or two

@ghost

This comment has been minimized.

Copy link

ghost commented Apr 8, 2015

Merged your changes and fixed the bug with regards to the promise. I will have another look at my codebase tomorrow and see if I can refactor it some other way with this in mind! Thanks for your help Tom, really appreciate it!

@tommck

This comment has been minimized.

Copy link

tommck commented Apr 8, 2015

No problem at all! Glad it helped

@ghost

This comment has been minimized.

Copy link

ghost commented Apr 12, 2015

Looks like I can refactor my app to get around this, but it is a ton of work (my app is very large!). The old way was a lot more convenient, but the advantages of the new way far outweigh the convenience of the old way :-)

@sebald

This comment has been minimized.

Copy link

sebald commented Apr 14, 2015

@PascalPrecht I can see uses for both. active seems to be correct place to load data from a server. But in some apps you may have to check first if you can load the data at all. E.g. a 3rd party Github app. If the user requests data for a repository Github may answer with a 404. In this case it would make much more sense to me to load the data in the canActivate and have the user redirected to a "You have no permission to do that" page if the loading fails.

@demisx

This comment has been minimized.

Copy link

demisx commented Apr 16, 2015

@sebald 👍 We use a very similar approach in our ng 1.x app. Only we called this function canAccess. :) Should probably refactor it to canActivate.

@paulwalker

This comment has been minimized.

Copy link

paulwalker commented May 4, 2015

Putting aside the merits of a resolve semantic defined on the route vs a canActivate defined on the controller, am I correct in my assessment of this conversation that a fundamental issue exists with the new router in that the canActivate does not allow for returning a promise?

If so, is there any reasonable technique in the new router to process an async callback before allowing navigation?

@btford

This comment has been minimized.

Copy link
Contributor

btford commented May 4, 2015

canActivate does not allow for returning a promise

canActivate should allow returning a promise. If it does not, then there is a bug.

@paulwalker

This comment has been minimized.

Copy link

paulwalker commented May 4, 2015

Thank you. To touch on the design merits quickly, it would be really nice to have the ability to provide a promise to a group of routes, in which the routes are only registered or able to be used if the promise resolves to true.

I hope that makes sense, perhaps there is a better practice for this sort of thing like only registering certain routes upon async callback confirmation?

@franleplant

This comment has been minimized.

Copy link

franleplant commented May 5, 2015

This is a really interesting issue, and here are some concerns that come from real world scenarios:

  • Former resolve object was a good way of getting data from the server before kicking off the controller. The canonical example is when the route is /user/:id load from the server that user with that id and only then start userController. I don't like how the controller, with the new lifecycle hooks needs to be aware of the routeParams.
  • Former resolve object was mediocre for handling redirections upon named params restriction, for example, when the user navigates to sections/:sectionID, check if the sectionID does match with an existing section and if not then redirect to a set default section.
    ui-router solves this with regexps to validate named params, they call them Typed Params. How will one handle this scenario with the new router?
  • Former resolve object was mediocre for handling hierarchical relationships between resolves and views, for example, when user navigates to /vehicle/:vehicleId/section/:sectionId you need to get from the server the vehicle with that id and after you get it, you need to get from the server the sections available for that vehicle (of course this is a trivial example, but it has it's uses).
    ui-router solves this with nested routes and resolves. How will one do this with the new router?
@paulwalker

This comment has been minimized.

Copy link

paulwalker commented May 8, 2015

I have not been successful registering routes after an async callback.

  function AppController($router, $session) {
    this.session = $session;

    $router.config(routes);
    $session.signedIn(function() {
      $router.config(authRoutes);
    });
  }

I attempted to return the promise in the canActivate of the parent AppController like so:

  AppController.prototype.canActivate = function() {
    return this.session.promise;
  };

But angular still attempts to unsuccessfully load the child route before callback. Any ideas?

@dpsthree

This comment has been minimized.

Copy link

dpsthree commented May 14, 2015

@PascalPrecht I think the functionality is still there, it's just hidden in a healthy layer of indirection. Consider the following:

angular.module('app', [])
  .controller('MyController', ['user', 'myService', MyController]);

function MyController(user, myService) {
  //Setup the controller under the assumption that everything is OK
  this.resultsPromise = myService.getSomeData()
  this.resultsPromise.then(function (results) {
    //Assume this will be reached, we will validate using canActivate
    //Manipulate Data as needed here or in activate()
  });
}

MyController.prototype.canActivate = function() {
  return this.resultsPromise.then(function (results) {
    //Do whatever is needed to verify the results are OK to use
    //This can include the verification of things such as the route params if 
    //they are placed on the controller first
    return true
  }); 
};

MyController.prototype.activate = function () {
  //optional, we can do data processing here to keep it out of the controller
  this.resultsPromise.then(function (results) {
    this.data = manipulateResults(results);
  });
}

So long as your controller logic that is dependent upon route verification is contained inside a then function there is no need to concern yourself with verification in the controller.

@paulwalker

This comment has been minimized.

Copy link

paulwalker commented May 14, 2015

Right, I just don't want to have to contain that logic in every controller. This is where something like ui-router's $stateChangeStart event is helpful, allowing one to annotate the route with a particular common security context is helpful.

$rootScope.$on('$stateChangeStart', 
function(event, toState, toParams, fromState, fromParams){ 
    event.preventDefault(); 
    // transitionTo() promise will be rejected with 
    // a 'transition prevented' error
})
@dpsthree

This comment has been minimized.

Copy link

dpsthree commented May 14, 2015

Another thought, it seems like everyone (including myself) has in mind using the routing controller as the logic controller for the remainder of that route. What if we decoupled them. Dedicate a controller as the place to put code concerning routing and a separate controller for the logic of the route. Use the latter with ng-controller or a directive. Move any data needed from the "resolve" mechanism through a shared service. With this in place we could "pollute" the routing controller as much as we want with logic concerning routing, redirecting, auth, etc... and not mix it with the view's controller code. Another benefit of this is that it frees up the logic controller for reuse.

This approach was also useful with UI router when having a reusable controller was desired, but the injectable route resolution parameters got in the way.

@paulwalker this is a nice piece of functionality from ui-router, but is a benefit of its events, not a lack of a resolve mechanism.

@alfonso-presa

This comment has been minimized.

Copy link

alfonso-presa commented May 28, 2015

Even though I really like the new router controller hooks and I think they are a must have feature, IMHO there should also be some way of replicating the resolve functionality in the new router for the following reasons:

  • With canActivate you can't have your dependencies resolved before your controller is instantiated. This complicates the development of the controllers and requires a deep knowledge of the pipeline in order to get things done correctly.
  • From a developer perspective and in terms of code readability the method canActivate should be used to determine if the controller can be activated... nothing more and nothing less :-).
  • You should be able to set the initial ViewModel contents in a single method and IMHO the best way to do this is the constructor, as it makes the developer confident about nothing being rendered without the dependencies resolved.

My proposal is adding a $resolve property to the controller containing an object a`la old router's/ui-router resolver property. Here you have an example:

function SampleController (items) {
    //Items will contain the result of the ItemsService promise once it is resolved.
    this.items = items;
}
SampleController.$resolve = {
    items: 'ItemsService' //ItemsService is a service returning a promise
};

angular.module('myapp').controller('SampleController', SampleController);

I've created a PR with this feature #333

@franleplant

This comment has been minimized.

Copy link

franleplant commented May 28, 2015

Overall I agree with final objective of #333

Another missing feature, which is paired with the former resolve mechanism is the $routeChange{Success,Error,Start} events, what about them?

@alfonso-presa

This comment has been minimized.

Copy link

alfonso-presa commented May 28, 2015

@paulwalker @franleplant I guess $routeChange* events functionality or a similar one can be achieved easily in the new router by even decorating the current pipeline methods or by providing a custom pipeline with more events.

You have an example of this in this test: https://github.com/angular/router/blob/master/test/pipeline.es5.spec.js#L19

@dpsthree The problem I see with that approach (this one #100 (comment)) is that you're polluting the view model (the controller itself) with elements that are not meant to be used by the view it self. I guess this kind of stuff is more easy to clean up with ES6/TS but it doesn't feel right in ES5.

I'm considering that in case the resolve functionality is not implemented in the new router, probably the best will be to initialize the view model in the activate method and leave the constructor almost empty. I guess activate can also return a promise to delay the view rendering in case something is required to be in the view model prior to it. What do you think?

@cjolif

This comment has been minimized.

Copy link

cjolif commented Jun 22, 2015

Re-reading all of this, I still think I have a regression compare to the old router. I think the canActivate solution works when the controller is here. But one of the resolve use-case was to load the controller on-demand using for example AMD (to do lazy loading) at the time the navigation occurs. I don't think the new router is covering this use-case because canActivate forces the controller to already be available.

@cjolif

This comment has been minimized.

Copy link

cjolif commented Aug 28, 2015

As the code seems to have moved to angular/angular should this issue (as well as the other I suppose) move as well?

@PascalPrecht

This comment has been minimized.

Copy link
Contributor

PascalPrecht commented Aug 30, 2015

I'd like to keep those issues here until the new router is in a state that we can test against it and verify.

@yellow1912

This comment has been minimized.

Copy link

yellow1912 commented Feb 26, 2016

Is there any update on this issue? I would really miss the ability to resolve content before showing the view. My use case is that we pre-render lots of things server side, then we send that chunk of template to browser for displaying.

Without "resolve", every time the user switches to a new route then the screen will show empty view for a second because the request to the server does not return anything yet.

@ArniDzhan

This comment has been minimized.

Copy link

ArniDzhan commented Feb 26, 2016

@yellow1912 canActivate can do exactly what resolve used to do, if you return promise it doesn't load the controller until data loaded.
here is an updated project you can use and see some examples from
hope that helps

https://github.com/petebacondarwin/ng1-component-router-demo

@pavankumarkatakam

This comment has been minimized.

Copy link

pavankumarkatakam commented May 9, 2016

Im actually use resolve for angular module loading asynchronous by ocLazyLoader. Is this possible in component router to load components asynchronously with new component router. overall project im using this.

@pavankumarkatakam

This comment has been minimized.

Copy link

pavankumarkatakam commented May 16, 2016

Is there any progress on this?

@vkniazeu

This comment has been minimized.

Copy link

vkniazeu commented Jul 5, 2016

@marcoskubis

This comment has been minimized.

Copy link

marcoskubis commented Sep 6, 2016

Any progress about this? I've made some changes in the plunkr example: plunkr.co (Click in heroes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment