feat: expose Promise result of the Angular navigation in RoutingService #12795
Conversation
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||
docs/migration/4_0.md
Outdated
| @@ -1207,6 +1208,15 @@ related labels outside of checkout and the checkout lib is not used, you will ne | |||
| `RoutingService.go` - Removed 2nd argument `query`. Use `extras.queryParams` instead. | |||
| `RoutingService.navigate` - Removed 2nd argument `query`. Use `extras.queryParams` instead. | |||
|
|
|||
| ### RoutingActions ngrx | |||
| The following ngrx actions has been deprecated: | |||
There was a problem hiding this comment.
Shouldn't be have been deprecated, as actions is a plural form? But I'm not an expert ;)
There was a problem hiding this comment.
Thanks for noticing. Btw. They are not only deprecated. I decided to remove them (in later commit).
Thanks again and I'll update the docs and schematics comment.
| { | ||
| node: ROUTING_ACTIONS, | ||
| importPath: SPARTACUS_CORE, | ||
| comment: `The following '${ROUTING_ACTIONS}' has been removed: '${ROUTE_GO_ACTION}', '${ROUTE_GO_BY_URL_ACTION}', '${ROUTE_BACK_ACTION}' and '${ROUTE_FORWARD_ACTION}'. Please just use the methods of the ${ROUTING_SERVICE}, respectively: 'go()', 'goByUrl()', 'back()' and 'forward()'.`, |
| @@ -20,5 +22,8 @@ export const ROUTING_SERVICE_MIGRATION: ConstructorDeprecation = { | |||
| { className: SEMANTIC_PATH_SERVICE, importPath: SPARTACUS_CORE }, | |||
| { className: ROUTING_PARAMS_SERVICE, importPath: SPARTACUS_CORE }, | |||
| ], | |||
| addParams: [{ className: ROUTER, importPath: ANGULAR_ROUTER }], | |||
There was a problem hiding this comment.
Regarding to line 17, I noticed that we are missing a comment with a file location. Is it optional? Or can we add // projects/core/src/routing/facade/routing.service.ts to this file to be consistent with other migrations.
There was a problem hiding this comment.
Thanks. Indeed, I overlooked it. Will add.
| - `RoutingActions.RouteForwardAction` (and `RoutingActions.ROUTER_FORWARD`). | ||
|
|
||
| Please just use the methods of the `RoutingService`, respectively: `go()`, `goByUrl()`, `back()`, `forward()`. | ||
|
|
There was a problem hiding this comment.
In my opinion it's worth to mention here about changes in those methods. For example that they are returning promise, some of them have now additional params etc.
There was a problem hiding this comment.
In the migration doc, I'd focus on explaining breaking changes to customers and giving hints how to fix them.
- Changing the return type of methods from void to Promise, is not breaking. It's a new feature btw.
- Only optional parameters were added to one method - so it's also not breaking.
Hosting service deployment🚀 Spartacus deployed to https://d1e02mf4dp6upr.cloudfront.net |
Public API changes✔️ Nothing changed in analyzed entry points.
|
Exposed Promise result of the Angular navigation in methods of the
RoutingService:go()andgoByUrl().Now Angular Router is called directly from the
RoutingService, but not from ngrx effects anymore. Dropped the effects as well as the actions triggering them.Added migration docs and schematics for the breaking changes.
By the way, added the missing param
NavigationExtrasinRoutingService.goByUrl.BREAKING CHANGES
The following ngrx actions have been removed:
RoutingActions.RouteGo(andRoutingActions.ROUTER_GO)RoutingActions.RouteGoByUrlAction(andRoutingActions.ROUTER_GO_BY_URL)RoutingActions.RouteBackAction(andRoutingActions.ROUTER_BACK)RoutingActions.RouteForwardAction(andRoutingActions.ROUTER_FORWARD).Please use instead the methods of the
RoutingService, respectively:go(),goByUrl(),back(),forward().closes #12789