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

DefaultRouterStateSerializer: path property missing in routeConfig #1363

Closed
JanCejka opened this issue Oct 12, 2018 · 2 comments · Fixed by #1384
Closed

DefaultRouterStateSerializer: path property missing in routeConfig #1363

JanCejka opened this issue Oct 12, 2018 · 2 comments · Fixed by #1384

Comments

@JanCejka
Copy link

JanCejka commented Oct 12, 2018

Minimal reproduction of the bug/regression with instructions:

Example: https://stackblitz.com/edit/ngrx-seed-nl8b8q?file=src%2Fapp%2Fapp-routing.module.ts

Routing effect based on: https://github.com/vsavkin/state_management_ngrx4

Expected behavior:

routerConfig should contain path / pathMatch (if it exists in original routerConfig) to allow easy identification of Router rule that was used to match used route. This way of matching is used in above mentioned example.

In example above, if routes in app-routing.module.ts are defined as shown in example, it's not possible to distinguish in effects/routing.effects.ts, function handleNavigation what rule in router definition was used (data/:id or test/:id) since path / pathMatch keys are missing. This has worked before since routeConfig from ActivatedRouteSnapshot was used directly and it contains these keys.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

NgRx: 6.1.0
Angular: 6.1.1
Node: 8.9.4

Other information:

Current behaviour is caused by: TypescriptID@7917a27

I would be willing to submit a PR to fix this issue

[ ] Yes (Assistance is provided if you need help submitting a pull request)
[x] No

@timdeschryver
Copy link
Member

What exactly are we looking at here?

@JanCejka
Copy link
Author

Sorry, I put wrong link to stackblitz. Link to example on stackblitz and Expected behavior was updated.

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

Successfully merging a pull request may close this issue.

2 participants