Skip to content

fix(router): routing services should be provided in forRoot only #1729

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

Merged
merged 3 commits into from
Feb 19, 2019

Conversation

vakrilov
Copy link
Contributor

@vakrilov vakrilov commented Feb 14, 2019

PR Checklist

What is the current behavior?

There are a couple of flaws in the NativeScriptRouterModule:

  1. Routing services (LocationStrategy, RouteReuseStrategy, RouterExtensions, etc.) are provided inside the NativeScriptRouterModule module. This might cause issues with lazy loaded modules which import NativeScriptRouterModule.
  2. The forRoot() and forChild() methods of NativeScriptRouterModule do not export the actual NativeScriptRouterModule (only the stock RouterModule form angular). This means that you will have to import both NativeScriptRouterModule.forRoot/forChild(routes) and NativeScriptRouterModule in every module.

What is the new behavior?

  1. Routing services are exported only in the module returned from NativeScriptRouterModule.forRoot(), similar to [Angular RouterModule`](https://github.com/angular/angular/blob/master/packages/router/src/router_module.ts#L133-L199)
  2. forRoot() and forChild() now export the actual NativeScriptRouterModule. Now it should required to import NativeScriptRouterModule if you have forChild() or forRoot()

Warning

⚠️⚠️⚠️
The forRoot and fotChild methods should contain a single return statement (source). Failing to comply with that leads to the following weird error when building projects with AOT: Function calls are not supported in decorators but 'NativeScriptRouterModule' was called.

@ghost ghost assigned vakrilov Feb 14, 2019
@ghost ghost added the in progress label Feb 14, 2019
@ADjenkov ADjenkov self-requested a review February 19, 2019 09:33
@vakrilov vakrilov merged commit 0f6a975 into master Feb 19, 2019
@ghost ghost removed the in progress label Feb 19, 2019
@vakrilov vakrilov deleted the router-for-child branch February 19, 2019 09:51
dottodot pushed a commit to dottodot/nativescript-angular that referenced this pull request Mar 16, 2019
* master:
  exclude files from api ref build
  NS Angular api ref build script
  fix(location-strategy): crash on going back with router-outlet after closing modal (NativeScript#1748)
  release: cut the 7.2.2 release (NativeScript#1742)
  fix(router): routing services should be provided in forRoot only (NativeScript#1729)
  fix(list-view): Add support for default item template
  fix(list-view): add support for "defailtTemplate"
  docs: cut the 7.2.1 release
  fix: Router tracing does not work with webpack
  chore: bump package version to 7.2.1
  chore: bump package versino tp 7.3.0
  test: Add tests for nested primary outlets
  fix(location-strategy): extend support for nested primary outlets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants