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

Update Router State in Store when using Resolve #358

Closed
pietmichal opened this issue Sep 7, 2017 · 8 comments
Closed

Update Router State in Store when using Resolve #358

pietmichal opened this issue Sep 7, 2017 · 8 comments

Comments

@pietmichal
Copy link

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[x] Feature request
[ ] Documentation issue or request
[ ] Support request

What is the current behavior?

When using resolve within Route, data property doesn't include the result of resolve in the store.

Expected behavior:

When using resolve, data property should be updated with the result from it.

Minimal reproduction of the problem with instructions:

Example route:

      {
        path: ':userid',
        component: UserDetailComponent,
        canActivate: [UserIdGuard],
        resolve: { userName: UserNameResolve }
      }

userName doesn't exist in data property of ActivatedRouteSnapshot within the store.

Version of affected browser(s),operating system(s), npm, node and ngrx:

@ngrx/router-store: 4.0.4

Other information:

I've noticed that even the serializer doesn't receive router state with updated data property.

@pietmichal pietmichal changed the title Updated Router State in Store when using Resolve Update Router State in Store when using Resolve Sep 7, 2017
@rupeshtiwari
Copy link
Contributor

rupeshtiwari commented Oct 20, 2017

Hi @brandonroberts @MikeRyanDev
I have fixed this issue by changing the hook to

afterPreactivation from beforePreactivation in StoreRouterConnectingModule class
I created PR for this fix #512.

However, I have question why we had hook on beforePreactivation if we change it to postPreactivation then effect will be fired after guards and resolves. Is it okay ? Will this impact the initial design in mind ?

@brandonroberts
Copy link
Member

Hey @roopkt. The idea for the for the resolve action is to add an additional listener for the ResolveEnd action, similar to the existing NavigationError and NavigationCancel events, then dispatch the appropriate NavigationResolve action.

@rupeshtiwari
Copy link
Contributor

Thanks! @brandonroberts got it I will work on this.

rupeshtiwari added a commit to rupeshtiwari/platform that referenced this issue Oct 23, 2017
@brandonroberts
Copy link
Member

@roopkt after some thought I leaning towards not bringing this into the library. Using resolvers with ngrx is considered an anti-pattern, as you should use guards/effects to push data into the store.

@rupeshtiwari
Copy link
Contributor

@brandonroberts Thanks

@fredgate
Copy link
Contributor

fredgate commented Mar 1, 2018

A question : I don't understand why use the guards instead resolvers to push data into store. The role of guards is to forbid access to a route, while role of resolver is to retrieve data needed for a route.
So the resolvers seem to be the best candidates for that.

@bbaia
Copy link
Contributor

bbaia commented Mar 10, 2018

Relates to #816

@klinki
Copy link

klinki commented Feb 15, 2019

I agree with @fredgate, using resolvers seems to be much better option to retrieve data. @brandonroberts why is it considered anti-pattern (and not supported by library)?

I just hit this limitation and it gives me a lot of trouble :(

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

No branches or pull requests

6 participants