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

feat!: use local route for local router store #309

Merged
merged 11 commits into from
Aug 30, 2024

Conversation

LayZeeDK
Copy link
Member

@LayZeeDK LayZeeDK commented Jul 3, 2024

This change in implementation will make the local router store more closely match ActivatedRoute while the global router store matches NgRx Router Store selectors. Through complex route configurations, the router store implementations are exercised to identify edge case differences between them and any breaking changes introduced to the local router store.

Features

  • LocalRouterStore matches ActivatedRoute more closely
    • Use ActivatedRoute to serialize the router state for the local router store implementation (LocalRouterStore)
    • LocalRouterStore.currentRoute$ matches ActivatedRoute.snapshot

Testing

  • Use Spectactular (15.0) feature harnesses
  • Compare LocalRouterStore selectors to ActivatedRoute observables
  • Compare GlobalRouterStore selectors to NgRx Router Store selectors and Router#events
  • Cover nested and componentless routes with route data
  • Cover nested and componentless routes with route parameters
  • Cover route fragment
  • Cover current route for nested routes
  • Cover nested route URLs
  • Cover nested route titles

BREAKING CHANGES

BEFORE:

(Router Component Store <=0.3.2)

// URL: /parent/child/grandchild

@Component({
  /* (...) */
  providers: [provideLocalRouterStore()],
})
export class ChildComponent implements OnInit {
  #route = inject(ActivatedRoute);
  #routerStore = inject(RouterStore);

  ngOnInit() {
    const currentRouteSnapshot = this.#route.snapshot;
    console.log(currentRouteSnapshot.routeConfig.path);
    // -> "child"
    console.log(currentRouteSnapshot.url[0].path);
    // -> "child"

    firstValueFrom(this.#routerStore.currentRoute$)
      .then(currentRoute => {
        console.log(currentRoute.routeConfig.path);
        // -> "grandchild"
        console.log(currentRoute.url[0].path);
        // -> "grandchild"
      });
  }
}

AFTER:

(Router Component Store >0.3.2)

// URL: /parent/child/grandchild

@Component({
  /* (...) */
  providers: [provideLocalRouterStore()],
})
export class ChildComponent implements OnInit {
  #route = inject(ActivatedRoute);
  #routerStore = inject(RouterStore);

  ngOnInit() {
    const currentRouteSnapshot = this.#route.snapshot;
    console.log(currentRouteSnapshot.routeConfig.path);
    // -> "child"
    console.log(currentRouteSnapshot.url[0].path);
    // -> "child"

    firstValueFrom(this.#routerStore.currentRoute$)
      .then(currentRoute => {
        console.log(currentRoute.routeConfig.path);
        // -> "child"
        console.log(currentRoute.url[0].path);
        // -> "child"
      });
  }
}

@LayZeeDK LayZeeDK force-pushed the feat/use-local-route-for-localrouterstore branch 2 times, most recently from e8cc5cb to ec7c959 Compare July 8, 2024 23:30
@LayZeeDK LayZeeDK changed the title feat! use local route for local router store feat!: use local route for local router store Jul 10, 2024
@LayZeeDK LayZeeDK force-pushed the feat/use-local-route-for-localrouterstore branch 2 times, most recently from 9d49464 to 4b4f938 Compare August 30, 2024 20:00
@LayZeeDK LayZeeDK force-pushed the feat/use-local-route-for-localrouterstore branch from 4b4f938 to cc98f5f Compare August 30, 2024 22:04
@LayZeeDK LayZeeDK marked this pull request as ready for review August 30, 2024 22:07
@LayZeeDK LayZeeDK merged commit b11d897 into main Aug 30, 2024
6 checks passed
@LayZeeDK LayZeeDK deleted the feat/use-local-route-for-localrouterstore branch August 30, 2024 22:11
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.

1 participant