docs: add draft dedicated router testing guide #62445
Conversation
|
Deployed adev-preview for 37e39c7 to: https://ng-dev-previews-fw--pr-angular-angular-62445-adev-prev-wbsw2qej.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
| import { Router } from '@angular/router'; | ||
| import { Location } from '@angular/common'; | ||
| import { Component } from '@angular/core'; | ||
| import { provideRouter } from '@angular/router'; |
There was a problem hiding this comment.
You have a duplicate import of @angular/router
| { path: 'about', component: MockAboutComponent } | ||
| ]) | ||
| ] | ||
| }).compileComponents(); |
There was a problem hiding this comment.
| }).compileComponents(); | |
| }) |
This isn't required unless you have a @defer block + AOT: true.
Make sure to also remove the other ones.
| imports: [RouterOutlet], | ||
| template: ` | ||
| <h1>Parent Component</h1> | ||
| <router-outlet></router-outlet> |
There was a problem hiding this comment.
| <router-outlet></router-outlet> | |
| <router-outlet /> |
| export class ParentComponent {} | ||
|
|
||
| @Component({ | ||
| selector: 'app-child', |
There was a problem hiding this comment.
Routed components don't require selectors. We can probably omit it here.
| private route = inject(ActivatedRoute); | ||
| searchTerm: string | null = null; | ||
|
|
||
| ngOnInit() { |
There was a problem hiding this comment.
I'd argue to move that to the constructor since we don't have any dependency in inputs.
There was a problem hiding this comment.
There is big difference between ctor v.s. ngOnInit.
I would put no logic into the ctor.
There was a problem hiding this comment.
The recommendations have changed and no longer recommands to put init logic into the ngOnInit.
| private route = inject(ActivatedRoute); | ||
| userId: string | null = null; | ||
|
|
||
| ngOnInit() { |
There was a problem hiding this comment.
We can probably move that to the constructor.
There was a problem hiding this comment.
If not applied for some reason, the test itself should not need to call component.ngOnInit();.
| <docs-code header="user-profile.component.spec.ts" language="typescript"> | ||
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | ||
| import { ActivatedRoute } from '@angular/router'; | ||
| import { UserProfileComponent } from './user-profile.component'; |
There was a problem hiding this comment.
I think the Component class suffix and .component suffix can be dropped from this and other imports/component names.
Also thank you for writing this guide. I am the worst with routing and testing respectively so this is great stuff to have
| import { TestBed } from '@angular/core/testing'; | ||
| import { ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; | ||
| import { authGuard } from './auth.guard'; | ||
| import { AuthService } from './auth.service'; |
There was a problem hiding this comment.
As a follow up of @michael-small style guide recommandation, we could rename the service to AuthStore.
|
|
||
| export const authGuard: CanActivateFn = () => { | ||
| const authService = inject(AuthService); | ||
| return authService.isAuthenticated(); |
There was a problem hiding this comment.
| return authService.isAuthenticated(); | |
| return inject(AuthService).isAuthenticated(); |
There was a problem hiding this comment.
refactor for a single line
| await TestBed.configureTestingModule({ | ||
| imports: [UserProfileComponent], | ||
| providers: [ | ||
| { provide: ActivatedRoute, useValue: activatedRouteStub } |
There was a problem hiding this comment.
Instead of this I'd recommend to only test using RouterTestingHarness which should take care of most of the things.
I guess @atscott will agree to this
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?
Issue Number: N/A
What is the new behavior?
Adds a dedicated testing guide for router.
Does this PR introduce a breaking change?
Other information