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

Replacing 'routerReducer' as an injectable token rather than static text #410

Closed
phillipzada opened this issue Sep 20, 2017 · 5 comments
Closed

Comments

@phillipzada
Copy link
Contributor

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

What is the current behavior?

Currently in router_store_module.ts routerReducer is a static text key.

Expected behavior:

Allow for the key name to be injected via a provider and token but defaulting to 'routerReducer' of not provided

Minimal reproduction of the problem with instructions:

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

Other information:

Happy to put together a PR if approved.

@brandonroberts
Copy link
Member

Seems reasonable to me. I would suggest adding a forRoot method that takes a config object with a key for the router reducer name. This way you don't have to register a new provider to override it.

@phillipzada
Copy link
Contributor Author

Cool, so the end result will be something like this

StoreRouterConnectingModule.forRoot( { stateName: 'routerReducer' } );

@brandonroberts
Copy link
Member

Yep. What about stateKey for the name?

@edmundo096
Copy link

edmundo096 commented Oct 15, 2017

This catched me by surprise. I used another name for the router state key and time traveling didn't worked on my first try.
After checking the example-app, I noticed the persistence on using the key 'routerReducer', so I checked the src and found that key. I gave it a try and it worked.

At least the setup code on the docs makes mention it in the StoreModule.forRoot({ routerReducer: routerReducer }), line, but maybe a note would be useful either if this is changed or not.

@pauldubois777
Copy link

@edmundo096 - Thanks for your post! This was driving me crazy trying to debug why time-traveling wasn't working! I tried a lot of things, and did a lot of searching for fixes until I found your post. I renamed the property to routerReducer and now it works!

I hope this can be fixed so that specific naming of the property isn't required, or at least update the documentation. I can help if needed.

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

4 participants