Skip to content

Commit a7db19f

Browse files
AleksanderBodurriAleksanderBodurri
committed
feat(devtools): clean up router tree for stable release
Addresses some cleanup items for the router tree: - No longer loads router ng global APIs as a side effect of importing the router. Rather this is now a runtime step that occurs when provideRouter is called. - No longer depends on router.navigateByUrl in Angular DevTools. There is now a dedicated global util for this - Router instance logic no longer depends on token name - Prevents navigating to lazy or redirect routes (these don't have an associated component)
1 parent 1a26fd3 commit a7db19f

19 files changed

+409
-210
lines changed

‎devtools/projects/ng-devtools-backend/src/lib/client-event-subscribers.ts

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ const getRoutesCallback = (messageBus: MessageBus<Events>) => () => getRoutes(me
191191
const navigateRouteCallback = (messageBus: MessageBus<Events>) => (path: string) => {
192192
const router: any = getRouterInstance();
193193
// If the router is not found or the navigateByUrl method is not available, we can't navigate
194-
if (router && router.navigateByUrl) {
195-
router.navigateByUrl(path);
194+
if (router) {
195+
ngDebugClient().ɵnavigateByUrl?.(router, path);
196196
} else {
197197
console.warn('Router not found or navigateByUrl method not available');
198198
}
@@ -207,6 +207,10 @@ const navigateRouteCallback = (messageBus: MessageBus<Events>) => (path: string)
207207
export const viewSourceFromRouter = (name: string, type: RoutePropertyType) => {
208208
const router: any = getRouterInstance();
209209

210+
if (router == null) {
211+
return;
212+
}
213+
210214
let element;
211215
if (type === 'component') {
212216
element = getComponentRefByName(router.config, name);
@@ -477,26 +481,20 @@ export interface SerializableComponentTreeNode
477481
}
478482

479483
function getRouterInstance() {
480-
const forest = prepareForestForSerialization(
481-
initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest(),
482-
ngDebugDependencyInjectionApiIsSupported(),
483-
);
484-
const rootInjector = forest[0].resolutionPath?.find((i) => i.name === 'Root');
485-
if (!rootInjector) {
486-
return;
487-
}
484+
const forest = initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest();
485+
const rootNode = forest[0];
488486

489-
const serializedProviderRecords = getSerializedProviderRecords(rootInjector);
490-
const routerInstance = serializedProviderRecords?.find(
491-
(provider) => provider.token === 'Router', // get the instance of router using token
492-
);
487+
if (!rootNode || !rootNode.nativeElement) {
488+
return null;
489+
}
493490

494-
if (!routerInstance) {
495-
return;
491+
const injector = getInjectorFromElementNode(rootNode.nativeElement);
492+
if (!injector) {
493+
return null;
496494
}
497495

498-
const router = getInjectorInstance(rootInjector, routerInstance);
499-
return router;
496+
const ng = ngDebugClient();
497+
return (ng as any).ɵgetRouterInstance?.(injector);
500498
}
501499

502500
// Here we drop properties to prepare the tree for serialization.

‎devtools/projects/ng-devtools-backend/src/lib/ng-debug-api/ng-debug-api.spec.ts

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {ɵDirectiveDebugMetadata, ɵGlobalDevModeUtils} from '@angular/core';
9+
import {ɵFrameworkAgnosticGlobalUtils} from '@angular/core';
1010
import {
1111
ngDebugDependencyInjectionApiIsSupported,
1212
ngDebugProfilerApiIsSupported,
@@ -17,7 +17,7 @@ import {
1717
} from './ng-debug-api';
1818
import {Framework} from '../component-tree/core-enums';
1919

20-
type Ng = ɵGlobalDevModeUtils['ng'];
20+
type Ng = ɵFrameworkAgnosticGlobalUtils;
2121

2222
/** Add a root element to the body. */
2323
const mockRoot = () => {
@@ -28,16 +28,37 @@ const mockRoot = () => {
2828
};
2929

3030
/** Creates an `ng` object with a `getDirectiveMetadata` mock. */
31-
const fakeNgGlobal = (framework: Framework): Partial<Record<keyof Ng, () => void>> => ({
32-
getComponent(): {} {
33-
return {};
34-
},
35-
getDirectiveMetadata(): Partial<ɵDirectiveDebugMetadata> {
31+
const fakeNgGlobal = (
32+
framework: Framework
33+
): Partial<Ng> => {
34+
const base: Partial<Ng> = {
35+
getComponent<T>(_element: any): any {
36+
return {};
37+
},
38+
getDirectiveMetadata(_directiveOrComponentInstance: any): any {
39+
return {
40+
framework,
41+
};
42+
},
43+
};
44+
45+
// Only Angular and ACX have route debug functions
46+
if (framework === Framework.Angular || framework === Framework.ACX) {
3647
return {
37-
framework,
48+
...base,
49+
ɵgetLoadedRoutes(route: any): any {
50+
return [];
51+
},
52+
ɵnavigateByUrl(router: any, url: string): any {},
53+
ɵgetRouterInstance(injector: any): any {
54+
return {};
55+
},
3856
};
39-
},
40-
});
57+
}
58+
59+
// Wiz does not have route debug functions
60+
return base;
61+
};
4162

4263
describe('ng-debug-api', () => {
4364
afterEach(() => {

‎devtools/projects/ng-devtools-backend/src/lib/ng-debug-api/ng-debug-api.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,10 @@ export function ngDebugProfilerApiIsSupported(): boolean {
7070
/** Checks whether Router API is supported within window.ng */
7171
export function ngDebugRoutesApiIsSupported(): boolean {
7272
const ng = ngDebugClient();
73-
74-
// Temporary solution. Convert to `ɵgetLoadedRoutes` when available.
75-
// If there is a Wiz application, make Routes API unavailable.
76-
const roots = getAppRoots();
7773
return (
78-
!!roots.length &&
79-
!roots.some((el) => {
80-
const comp = ng.getComponent!(el)!;
81-
return ng.getDirectiveMetadata?.(comp)?.framework === Framework.Wiz;
82-
})
74+
ngDebugApiIsSupported(ng, 'ɵgetLoadedRoutes') &&
75+
ngDebugApiIsSupported(ng, 'ɵgetRouterInstance') &&
76+
ngDebugApiIsSupported(ng, 'ɵnavigateByUrl')
8377
);
8478
}
8579

‎devtools/projects/ng-devtools-backend/src/lib/router-tree.spec.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ describe('parseRoutes', () => {
2020
isAux: false,
2121
isLazy: false,
2222
isActive: false,
23+
isRedirect: false,
2324
});
2425
});
2526

@@ -39,6 +40,7 @@ describe('parseRoutes', () => {
3940
'isAux': false,
4041
'isLazy': false,
4142
'isActive': false,
43+
isRedirect: false,
4244
});
4345
});
4446

@@ -111,6 +113,7 @@ describe('parseRoutes', () => {
111113
'isAux': true,
112114
'isLazy': false,
113115
'isActive': undefined,
116+
'isRedirect': false,
114117
},
115118
{
116119
'component': 'component-two',
@@ -126,6 +129,7 @@ describe('parseRoutes', () => {
126129
'isAux': false,
127130
'isLazy': false,
128131
'isActive': undefined,
132+
'isRedirect': false,
129133
'children': [
130134
{
131135
'component': 'component-two-two',
@@ -141,6 +145,7 @@ describe('parseRoutes', () => {
141145
'isAux': false,
142146
'isLazy': false,
143147
'isActive': undefined,
148+
'isRedirect': false,
144149
},
145150
],
146151
},
@@ -158,6 +163,7 @@ describe('parseRoutes', () => {
158163
'isAux': false,
159164
'isLazy': true,
160165
'isActive': undefined,
166+
'isRedirect': false,
161167
},
162168
{
163169
'component': 'redirect -> redirecting to -> "redirectTo"',
@@ -173,10 +179,12 @@ describe('parseRoutes', () => {
173179
'isAux': false,
174180
'isLazy': false,
175181
'isActive': undefined,
182+
'isRedirect': true,
176183
},
177184
],
178185
'isAux': false,
179186
'isLazy': false,
187+
'isRedirect': false,
180188
'data': [],
181189
'isActive': false,
182190
} as any);

‎devtools/projects/ng-devtools-backend/src/lib/router-tree.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export function parseRoutes(router: Router): Route {
2727
children: rootChildren ? assignChildrenToParent(null, rootChildren, currentUrl) : [],
2828
isAux: false,
2929
isLazy: false,
30+
isRedirect: false,
3031
data: [],
3132
isActive: currentUrl === '/',
3233
};
@@ -62,6 +63,7 @@ function assignChildrenToParent(
6263

6364
// only found in aux routes, otherwise property will be undefined
6465
const isAux = Boolean(child.outlet);
66+
const isRedirect = Boolean(child.redirectTo);
6567
const isLazy = Boolean(child.loadChildren || child.loadComponent);
6668

6769
const pathWithoutParams = routePath.split('/:')[0];
@@ -81,6 +83,7 @@ function assignChildrenToParent(
8183
isAux,
8284
isLazy,
8385
isActive,
86+
isRedirect,
8487
};
8588

8689
if (childDescendents) {

‎devtools/projects/ng-devtools/src/lib/devtools-tabs/devtools-tabs.component.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
[hidden]="!routerTreeVisible"
8181
[snapToRoot]="snapToRoot()"
8282
[routes]="routes()"
83+
[routerDebugApiSupport]="supportedApis().routes"
8384
/>
8485
}
8586

‎devtools/projects/ng-devtools/src/lib/devtools-tabs/devtools-tabs.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class DevToolsTabsComponent {
104104
if (supportedApis.dependencyInjection) {
105105
tabs.push('Injector Tree');
106106
}
107-
if (supportedApis.routes && this.routerGraphEnabled() && this.routes().length > 0) {
107+
if (this.routerGraphEnabled() && this.routes().length > 0) {
108108
tabs.push('Router Tree');
109109
}
110110
if (supportedApis.transferState && this.transferStateEnabled()) {

‎devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.html

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
ng-button
77
size="compact"
88
(click)="btnClick.emit('')"
9-
[disabled]="isRouteWithoutComponent()"
9+
[disabled]="rowValue()?.toString().startsWith('Lazy ')"
1010
>
11-
{{ data() }}
11+
{{ rowValue() }}
1212
</button>
1313
}
1414
@case ('flag') {
15-
<span [class]="data() ? 'tag-active' : 'tag-inactive'">
16-
{{ data() }}
15+
<span [class]="rowValue() ? 'tag-active' : 'tag-inactive'">
16+
{{ rowValue() }}
1717
</span>
1818
}
1919
@case ('list') {
@@ -26,7 +26,13 @@
2626
</div>
2727
}
2828
@default {
29-
<span class="row-data">{{ data() }}</span>
29+
<span class="row-data">
30+
@if (renderValueAsJson()) {
31+
{{ rowValue() | json }}
32+
} @else {
33+
{{ rowValue() }}
34+
}
35+
</span>
3036
}
3137
}
3238
</td>

‎devtools/projects/ng-devtools/src/lib/devtools-tabs/router-tree/route-details-row.component.spec.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ describe('RouteDetailsRowComponent', () => {
2424
component = fixture.componentInstance;
2525

2626
fixture.componentRef.setInput('label', 'Route Title');
27+
fixture.componentRef.setInput('data', {value: 'Route Data'});
28+
fixture.componentRef.setInput('dataKey', 'value');
2729
fixture.detectChanges();
2830
});
2931

@@ -32,8 +34,8 @@ describe('RouteDetailsRowComponent', () => {
3234
});
3335

3436
it('should render a label and text data', () => {
35-
fixture.componentRef.setInput('label', 'Route Title');
36-
fixture.componentRef.setInput('data', 'Route Data');
37+
fixture.componentRef.setInput('data', {value: 'Route Data'});
38+
fixture.componentRef.setInput('dataKey', 'value');
3739
fixture.detectChanges();
3840

3941
const labelElement = fixture.debugElement.query(By.css('th'));
@@ -45,9 +47,9 @@ describe('RouteDetailsRowComponent', () => {
4547
});
4648

4749
it('should render a label and flag data true', () => {
48-
fixture.componentRef.setInput('label', 'Route Title');
4950
fixture.componentRef.setInput('type', 'flag');
50-
fixture.componentRef.setInput('data', 'true');
51+
fixture.componentRef.setInput('data', {isActive: true});
52+
fixture.componentRef.setInput('dataKey', 'isActive');
5153
fixture.detectChanges();
5254

5355
const labelElement = fixture.debugElement.query(By.css('th'));
@@ -59,9 +61,9 @@ describe('RouteDetailsRowComponent', () => {
5961
});
6062

6163
it('should render a label and flag data false', () => {
62-
fixture.componentRef.setInput('label', 'Route Title');
6364
fixture.componentRef.setInput('type', 'flag');
64-
fixture.componentRef.setInput('data', false);
65+
fixture.componentRef.setInput('data', {isLazy: false, isRedirect: false});
66+
fixture.componentRef.setInput('dataKey', 'isLazy');
6567
fixture.detectChanges();
6668

6769
const labelElement = fixture.debugElement.query(By.css('th'));
@@ -73,9 +75,9 @@ describe('RouteDetailsRowComponent', () => {
7375
});
7476

7577
it('should render a label and chip data', () => {
76-
fixture.componentRef.setInput('label', 'Route Title');
7778
fixture.componentRef.setInput('type', 'chip');
78-
fixture.componentRef.setInput('data', 'Component Name');
79+
fixture.componentRef.setInput('data', {name: 'Component Name'});
80+
fixture.componentRef.setInput('dataKey', 'name');
7981
fixture.detectChanges();
8082

8183
const labelElement = fixture.debugElement.query(By.css('th'));
@@ -87,9 +89,9 @@ describe('RouteDetailsRowComponent', () => {
8789
});
8890

8991
it('should render a label and chip data disabled', () => {
90-
fixture.componentRef.setInput('label', 'Route Title');
9192
fixture.componentRef.setInput('type', 'chip');
92-
fixture.componentRef.setInput('data', 'Lazy Component Name');
93+
fixture.componentRef.setInput('data', {name: 'Lazy Component Name'});
94+
fixture.componentRef.setInput('dataKey', 'name');
9395
fixture.detectChanges();
9496

9597
const labelElement = fixture.debugElement.query(By.css('th'));
@@ -102,9 +104,9 @@ describe('RouteDetailsRowComponent', () => {
102104
});
103105

104106
it('should render a label and list data', () => {
105-
fixture.componentRef.setInput('label', 'Route Title');
106107
fixture.componentRef.setInput('type', 'list');
107-
fixture.componentRef.setInput('data', ['Guard 1', 'Guard 2']);
108+
fixture.componentRef.setInput('data', {providers: ['Guard 1', 'Guard 2']});
109+
fixture.componentRef.setInput('dataKey', 'providers');
108110
fixture.detectChanges();
109111

110112
const labelElement = fixture.debugElement.query(By.css('th'));

0 commit comments

Comments
 (0)