Skip to content
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

Equivalent to resolve in Component Router? #4015

Closed
robwormald opened this issue Sep 5, 2015 · 37 comments
Closed

Equivalent to resolve in Component Router? #4015

robwormald opened this issue Sep 5, 2015 · 37 comments
Labels
effort2: days feature Issue that requests a new feature

Comments

@robwormald
Copy link
Contributor

With the removal of the @InjectPromise and @InjectLazy decorators, is there an equivalent functionality to resolve in ngRoute/uiRouter?

The angular1 version would look something like:

//controller
function MyController(foo){}

//route config
{
  controller: 'MyController',
  url: 'foos/:fooId',
  resolve: {  
    foo: function($routeParams,FooService){ return FooService.getFooById($routeParams.fooId); }
  }
}

This is IMO, one of the killer features of the current router and UI-router, as it remove async entirely from the controller and makes it super easy to test. The canActivate etc hooks cover one of the use cases for resolve (preventing access to routes/state), but I can't see a replacement for the above case. Perhaps I'm missing something?

I've been tinkering with an idea like this for NG2, implementing resolve as a decorator:

@Component({ selector: 'foo-detail' })
@View({ template: '<div>{{foo.name}}</div>'})
//token : resolver : injectables
@Resolve(Foo, (params, fooService) => fooService.getFooById(params.fooId), [RouteParams, FooService])
class FooDetailComponent {
  constructor(public foo: Foo){}
}

An alternative syntax might look like:

@Resolve({
  foo(params:RouteParams, fooService:FooService){ return fooService.getFooById(params.fooId) }
})

Though that limits you to string keys and I can't figure a way to pass bindings, and I'm not sure reflect will allow us to grab the metadata annotations this way.

Under the hood, it keeps async out of the injector, as its "simply" a matter of executing the resolver fn before passing them to the component's overridden bindings. Thoughts?

@brandonroberts
Copy link
Contributor

You could implement the OnActivate interface which can be used to resolve data before the component navigation completes. Its called after CanActivate and the component will not be activated until its resolved when using a promise.

@robwormald
Copy link
Contributor Author

@brandonroberts not a bad idea, but my concern there is that it necessarily couples a component to the router I think, vs using the constructor and DI, which would allow you to test/reuse a component independently of the router.

@btford
Copy link
Contributor

btford commented Sep 19, 2015

This is a planned feature. There's another issue where I've expended on my plan for implementing it.

@btford btford added comp: router feature Issue that requests a new feature effort2: days labels Sep 19, 2015
@endash
Copy link

endash commented Oct 23, 2015

+1 I've been using DI (with string tokens) in some cases for "giving a component its model" to avoid coupling to a central service class, but something higher level in the router would be awesome.

@goleafs
Copy link

goleafs commented Jan 20, 2016

Actually, I think it may even be desirable to have a resolve (or at least some way to pass additional arguments) into the canActivate itself.

This would be primarily for code reuse.
The other scenario is passing resolves down the line to sub-routes.. how will this work?

Is there a reference to the issue with the expanded plan for implementation?

@eqbalsajadi
Copy link

@awerlang
Copy link
Contributor

I'd like that. Most cases Foo would be just an object or array, we would achieve that with OpaqueToken I guess.

@goleafs not sure if we need to pass resolves to sub-routes, as the DI provides for shared services from where we could access the data.

@brandonroberts
Copy link
Contributor

I have a proposal for the resolvers.

@Resolve([
  {
    name: Foo,
    resolver: (params: RouteParams, someService: SomeService) => {
      return new Promise((resolve) => {
        someService.doSomething(params.get('id')).subscribe((result) => {
          resolve(result);
        });
      });
    }
  }
])
class MyRoute {
  constructor(foo: Foo) {}
}

@nigeman
Copy link

nigeman commented Feb 29, 2016

I like @brandonroberts suggestion above. Currently it seems we have to mimic this "async resolve" via the CanActivate DI workaround that @brandonroberts came up with a few months ago. Any movement on this? Just curious :)

@PatrickJS
Copy link
Member

the meta data emitted by ts for the constructor doesn't work on function so you have to change it a bit

@Resolve([
  {
    name: Foo,
    deps: [RouteParams, SomeService], 
    resolver: (params: RouteParams, someService: SomeService) => {
      return new Promise(resolve => {
        someService.doSomething(params.get('id')).subscribe((result) => {
          resolve(result);
        });
      });
    }
  }
])
class MyRoute {
  constructor(foo: Foo) {}
}

@brandonroberts
Copy link
Contributor

@gdi2290 good catch

@fxck
Copy link

fxck commented Mar 8, 2016

Is wrapping things in promises necessary? Same goes for CanActivate(#4112 (comment)).. wouldn't

someService.doSomething(params.get('id')).take(1);

do the same job?

@brandonroberts
Copy link
Contributor

@fxck currently, all the router lifecycle hooks need to return a promise that so that the route isn't activated until the promise(s) settle. Using an observable wouldn't immediately resolve the value, so it wouldn't wait. You could return a converted observable:

return someService.doSomething(params.get('id')).take(1).toPromise();

@naomiblack naomiblack modified the milestones: Angular 2 Blocking, Angular 2 Release Candidate Mar 8, 2016
@naomiblack naomiblack modified the milestones: Angular 2 Blocking, Angular 2 Release Candidate Mar 8, 2016
@cquillen2003
Copy link

+1 for @resolve decorator.

Data-fetching belongs in a decorator instead of in ngOnInit() or routerOnActivate() IMHO.

@syndicatedshannon
Copy link

I agree that given the current design I would intuitively look for a "Resolve" decorator.

The suggested format is very bulky when compared to what was necessary for UI Router, especially with the likely scenario that we'll be bridging observables. My current implementation of Brandon's workaround is actually a little slimmer, although it is quite specific to my needs (e.g. it assumes the referenced service will produce an observable), and is subject to the workaround's weaknesses, such as using magic strings:

@CanActivate(RootResolver('Foo', (fooSvc: FooService) => fooSvc.getFoo())) ... routerOnActivate(to) { this.foo = to.routeData.data['Foo']; }

And the receiver could probably be trimmed down further by stuffing the injector. I appreciate the software, just wanted to mention I was surprised it wasn't more terse.

@syndicatedshannon
Copy link

I'm glad this is on the hotlist.

Can someone please comment on the intention (or lack of intention) to support this on AuxRoute as well?

We're in a bind where, since AuxRoute does not process the CanActivate hook, the workaround is not working; and we cannot build aux components that are responsible for their own data, or move/reuse components between Aux and Primary routes.

Also, does anyone listening have a suggestion on a similar workaround for AuxRoutes?

@syndicatedshannon
Copy link

An older discussion by contributors on this topic:
angular/router#100

@syndicatedshannon
Copy link

Did anyone attend Brandon's workshop (https://www.ng-conf.org/#/sessions/bdogg64FD) yesterday? It was scheduled to discuss "resolving data before loading routes", and appears to have been using the new-new router (with CanActivate moved/relocated).

@brandonroberts
Copy link
Contributor

brandonroberts commented May 8, 2016

I attended 😄 I didn't cover resolving data before loading routes because that mechanism doesn't exist in the new router yet.

@syndicatedshannon
Copy link

Oh noes! And CanActivate is toast :). Keep us apprised please!

So, how did you like Brandon's workshop, Brandon?

@brandonroberts
Copy link
Contributor

Will do. I thought Brandon's workshop went pretty well. The other attendees spoke positively about it also 👍

@alexmady
Copy link

Hi - any ideas when the CanActivate mechanism will be available to use?
What is the suggested approach for an authentication mechanism in the mean time for those of us who are developing apps agains rc1?

Cheers

@danielpquinn
Copy link

In my opinion this is the most critical piece of functionality missing from Angular 2.0 today. I was able to get by using @brandonroberts OnActivate workaround, but now that it's gone in the release candidate I'm feeling a little high and dry here! I hesitate to use a third party solution like ngrx/router or ui-router so early in the life of this project.

That being said, I'm excited to see that progress is being made on the router after so many months of waiting, please continue the good work :) I feel like this is a situation where releasing a working implementation soon will be more valuable than debating implementation details for another six months in an attempt to come up with a perfect api or pattern. I have a fairly large app built on Angular 2 right now and overall it's been a dream to work with, but the component loading experience is negatively affected by the limitations of the router. I apologize if the tone of this comment is a little negative, but I desperately need this feature!

Please keep us updated, thanks!

@fxck
Copy link

fxck commented May 18, 2016

@danielpquinn one of the two people behind @ngrx/router is @brandonroberts. Not to mention that @ngrx/router team is actually in talks with angular team, to help them improve the new router(you can kinda read that in the weekly meeting notes). So believe me, it's a safe bet, I've been using for almost two months, and it's been a breeze, the community and the rate it gets improved without breaking changes is amazing, it allowed me to do all the things I wanted from the component router. And since I'm going fully reactive, it's even better alternative than the ui-router.

@syndicatedshannon
Copy link

syndicatedshannon commented May 18, 2016

I hear you Daniel and I agree that, for my needs, that this is the most critical piece missing from the RC.

However, precisely because that I don't want to end up resorting to a plug-in AFTER integrating the stock router, I would like to see the Angular designers get it nearly-right this time. We've already seen the design in 1.x was insufficient pretty early-on, as was the 2beta. I personally don't need another highly-incomplete refactor that results in rework without long-term benefit.

Like you, we also decided to start building our new app on Angular 2 beta a year early, knowing full well that it was not recommended. However, that doesn't mean I would urge the Angular team to sacrifice the quality of this integral piece of the platform for everyone's future, including my own.

Instead my team is building our own components to meet our specific needs for now; and we look forward to a standard routing solution that does most of what most teams need.

@danielpquinn
Copy link

Thanks for the feedback @fxck and @syndicatedshannon. I think you're right about taking time to get it right, and maybe @ngrx/router is the safest bet for now. I'll see how far that gets me today.

@syndicatedshannon
Copy link

syndicatedshannon commented May 18, 2016

Also keep in mind that you can still used router-deprecated. I didn't mean CanActivate was removed entirely, it just wasn't brought forward into the new router (which has other issues as well). It seems to still work the same way on router-deprecated. So, if you started a solution on 2beta router, just change your code to point at router-deprecated. There are a few minor inconsistencies, but... 'dems da breaks when using beta, I suppose!

@fxck
Copy link

fxck commented May 18, 2016

@danielpquinn feel free to join the gitter channel at gitter.im/ngrx/router there's always someone to help you out when you don't know how to do something.

@michaeljohnbennett
Copy link

Been a while since this was last updated, are we any closer to a solution in the new RC router?

@IgorMinar
Copy link
Contributor

this is already handled by router v3

@nigeman
Copy link

nigeman commented Jun 23, 2016

Link to some documentation about this ? Is it purely the life cycle hooks ? And DI inject into them ? Was that the final solution ? Cheers!

@michaeljohnbennett
Copy link

@nigeman +1 I coudn't see any documentation about these features in the API docs or cookbooks.

@syndicatedshannon
Copy link

syndicatedshannon commented Jul 2, 2016

RC4 includes the actual resolve() router interface implementation.

@ammar91
Copy link

ammar91 commented Jul 14, 2016

@syndicatedshannon As of router version 3 beta 2, Resolve is a generic interface. What type should I suppose to pass in it?

@awerlang
Copy link
Contributor

According to this definition, it can be anything. The generic parameter type ain't used to constraint anything. I'll put the return type of the method resolve though, like string if I return an Observable<string>.

export interface Resolve<T> {
  resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot):
      Observable<any>|Promise<any>|any;
}

@ammar91
Copy link

ammar91 commented Jul 14, 2016

@awerlang Thanks for the quick help.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort2: days feature Issue that requests a new feature
Projects
None yet
Development

No branches or pull requests