Repository navigation
Conversation
Mirrors `Router.navigate` so tests can pass `queryParams`, `fragment` and other `NavigationExtras` without an extra navigation.
Updating RouterTestingHarness documentation to include `navigate` method
JeanMeche
requested review from
JeanMeche and
atscott
and removed request for
MarkTechson and
crisbeto
October 15, 2026 16:01
JeanMeche
reviewed
Oct 16, 2026
| extras: NavigationExtras | undefined, | ||
| requiredRoutedComponentType: Type<T>, | ||
| ): Promise<T>; | ||
| async navigate<T>( |
Member
There was a problem hiding this comment.
Toughts on going this route (pun intended) ?
async navigate(commands: readonly any[], extras?: NavigationExtras): Promise<null | {}>;
async navigate<T>(commands: readonly any[], requiredRoutedComponentType: Type<T>): Promise<T>;
async navigate<T>(
commands: readonly any[],
extras: NavigationExtras | undefined,
requiredRoutedComponentType: Type<T>,
): Promise<T>;
async navigate<T>(
commands: readonly any[],
extrasOrComponent?: NavigationExtras | Type<T>,
requiredRoutedComponentType?: Type<T>,
): Promise<T | null> {
let extras: NavigationExtras | undefined;
let componentType: Type<T> | undefined;
if (typeof extrasOrComponent === 'function') {
componentType = extrasOrComponent;
} else {
extras = extrasOrComponent;
componentType = requiredRoutedComponentType;
}
return this.runNavigation(
(router) => router.navigate(commands, extras),
componentType,
);
}
JeanMeche
reviewed
Oct 16, 2026
Comment on lines
+294
to
+303
| it('should read search term and fragment passed as navigation extras', async () => { | ||
| component = await harness.navigate( | ||
| ['/search'], | ||
| {queryParams: {q: 'angular'}, fragment: 'results'}, | ||
| Search, | ||
| ); | ||
|
|
||
| expect(component.searchTerm()).toBe('angular'); | ||
| expect(TestBed.inject(Router).url).toBe('/search?q=angular#results'); | ||
| }); |
Member
There was a problem hiding this comment.
The search component does not consume the fragment. This isn't a great illustration. Consider updating the component to actually consume this.route.fragment
JeanMeche
reviewed
Oct 16, 2026
| @@ -83,6 +83,7 @@ it('should get the activated component directly', async () => { | |||
| ### Best Practices | |||
|
|
|||
| - **Navigate with the Harness:** Always use `harness.navigateByUrl()` to simulate navigation. This method returns a promise that resolves with the instance of the activated component. | |||
Member
There was a problem hiding this comment.
We need to amend this.
Suggested change
| - **Navigate with the Harness:** Always use `harness.navigateByUrl()` to simulate navigation. This method returns a promise that resolves with the instance of the activated component. | |
| - **Navigate with the Harness:** Always use `harness.navigateByUrl()` or `harness.navigate()` to simulate navigation. This method returns a promise that resolves with the instance of the activated component. |
JeanMeche
reviewed
Oct 16, 2026
JeanMeche
left a comment
Member
There was a problem hiding this comment.
AGENT: Thanks for working on this! I've left a couple of inline notes on test coverage and runNavigation behavior for your consideration.
| expect(harness.routeNativeElement?.innerHTML).toContain('hello world'); | ||
| }); | ||
|
|
||
| it('navigates with commands, query params and fragment', async () => { |
Member
There was a problem hiding this comment.
AGENT: Consider expanding test coverage for navigate to cover additional cases:
- Navigating with commands only (no extras or component type):
await harness.navigate(['/a']) - Navigating with commands and component type (no extras):
await harness.navigate(['/a'], TestCmp) - Navigating with extras but no component type:
await harness.navigate(['/a'], { queryParams: { q: '1' } }) - Guard rejection / cancellation: Verifying behavior when a guard cancels the navigation (
return false) or redirects (return UrlTree) - Mismatched component type: Verifying that
navigatethrows the expected error when the activated component doesn't matchrequiredRoutedComponentType - Relative navigation: Verifying that
{ relativeTo: route }behaves as expected with the harness
| * @returns The activated component instance, or `null` if the outlet was not activated. | ||
| */ | ||
| private async runNavigation<T>( | ||
| startNavigation: (router: Router) => Promise<unknown>, |
Member
There was a problem hiding this comment.
AGENT: In runNavigation, if a test performs a successful initial navigation (activating ComponentA) followed by a subsequent navigation that is rejected by a guard (returning false):
outlet.isActivatedremainstruefrom the previous navigation.- If called without
requiredRoutedComponentType, this returns the stale previous component (ComponentA) rather thannull. - If called with
requiredRoutedComponentType, it throws an unexpected component error rather than indicating navigation did not activate any component.
While this behavior existed in navigateByUrl, consolidating into runNavigation reinforces it. Could we check whether startNavigation(router) succeeded, or is keeping parity with navigateByUrl intentional here?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrors
Router.navigateso tests can passqueryParams,fragmentand otherNavigationExtraswithout an extra navigation.PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
RouterTestingHarnessonly exposesnavigateByUrl, which accepts a string. To set query params or a fragment, tests must build the URL string by hand or injectRouterand perform an extra navigationIssue Number: #71210
What is the new behavior?
Adds
RouterTestingHarness.navigate(commands, extras?, requiredRoutedComponentType?), mirroringRouter.navigate. It acceptsNavigationExtras(e.g.queryParams,fragment), waits for the navigation and any redirects to complete, and returns the activated component. Passing a component type enforces it, as withnavigateByUrlDoes this PR introduce a breaking change?
Other information
The new private helper
runNavigationconsolidates the logic shared bynavigateByUrlandnavigate: waiting for the navigation and redirects, running change detection, and resolving or type-checking the activated component. The two public methods only supply the router callExisting
navigateByUrlusages in the repo's tests are intentionally left unchanged