8000 refactor(devtools): fixing PR comments and adding unit tests · angular/angular@ea2938a · GitHub
[go: up one dir, main page]
Skip to content

Commit ea2938a

Browse files
sumitarorasumitarora
committed
refactor(devtools): fixing PR comments and adding unit tests
- Update code to fix PR comments and cleanup code - Add unit tests for the new code
1 parent 9288aa9 commit ea2938a

21 files changed

+591
-191
lines changed

‎devtools/projects/ng-devtools-backend/src/BUILD.bazel

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ ts_library(
66
name = "src",
77
srcs = ["public-api.ts"],
88
deps = [
9+
"//devtools/projects/ng-devtools-backend/src/lib:client_event_subscribers_rjs",
910
"//devtools/projects/ng-devtools-backend/src/lib:lib_rjs",
11+
"//devtools/projects/ng-devtools-backend/src/lib:router_tree_rjs",
1012
"//devtools/projects/ng-devtools-backend/src/lib/component-tree:component-tree_rjs",
11-
"//devtools/projects/ng-devtools-backend/src/lib:client_event_subscribers_rjs",
1213
],
1314
)

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

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ import {unHighlight} from './highlighter';
5050
import {disableTimingAPI, enableTimingAPI, initializeOrGetDirectiveForestHooks} from './hooks';
5151
import {start as startProfiling, stop as stopProfiling} from './hooks/capture';
5252
import {ComponentTreeNode} from './interfaces';
53-
import {getElementRefByName, getComponentRefByName, parseRoutes} from './router-tree';
53+
import {
54+
getElementRefByName,
55+
getComponentRefByName,
56+
parseRoutes,
57+
RoutePropertyType,
58+
} from './router-tree';
5459
import {ngDebugDependencyInjectionApiIsSupported} from './ng-debug-api/ng-debug-api';
5560
import {setConsoleReference} from './set-console-reference';
5661
import {serializeDirectiveState} from './state-serializer/state-serializer';
@@ -178,14 +183,20 @@ const navigateRouteCallback = (messageBus: MessageBus<Events>) => (path: string)
178183
router.navigateByUrl(path);
179184
};
180185

181-
export const viewSourceFromRouter = (path: string, type: string) => {
186+
/**
187+
* Opens the source code of a component or a directive in the editor.
188+
* @param name - The name of the component, provider, or directive to view source for.
189+
* @param type - The type of the element to view source for component, provider, or directive.
190+
* @returns - The element instance of the component, provider, or directive.
191+
*/
192+
export const viewSourceFromRouter = (name: string, type: RoutePropertyType) => {
182193
const router: any = getRouterInstance();
183194

184195
let element;
185196
if (type === 'component') {
186-
element = getComponentRefByName(router.config, path);
197+
element = getComponentRefByName(router.config, name);
187198
} else {
188-
element = getElementRefByName(type, router.config, path);
199+
element = getElementRefByName(type, router.config, name);
189200
}
190201
return element;
191202
};
@@ -250,7 +261,7 @@ const getRoutes = (messageBus: MessageBus<Events>) => {
250261
);
251262
if (forest.length === 0) return;
252263

253-
const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');
264+
const rootInjector = forest[0].resolutionPath?.find((i) => i.name === 'Root');
254265
if (!rootInjector) return;
255266

256267
const route = getRouterConfigFromRoot(rootInjector);
@@ -409,12 +420,21 @@ function getRouterInstance() {
409420
initializeOrGetDirectiveForestHooks().getIndexedDirectiveForest(),
410421
ngDebugDependencyInjectionApiIsSupported(),
411422
);
412-
const rootInjector = (forest[0].resolutionPath ?? []).find((i) => i.name === 'Root');
413-
const serializedProviderRecords = getSerializedProviderRecords(rootInjector!) ?? [];
414-
const routerInstance = serializedProviderRecords.filter(
423+
const rootInjector = forest[0].resolutionPath?.find((i) => i.name === 'Root');
424+
if (!rootInjector) {
425+
return;
426+
}
427+
428+
const serializedProviderRecords = getSerializedProviderRecords(rootInjector);
429+
const routerInstance = serializedProviderRecords?.find(
415430
(provider) => provider.token === 'Router', // get the instance of router using token
416431
);
417-
const router = getInjectorInstance(rootInjector!, routerInstance[0]);
432+
433+
if (!routerInstance) {
434+
return;
435+
}
436+
437+
const router = getInjectorInstance(rootInjector, routerInstance);
418438
return router;
419439
}
420440

@@ -594,14 +614,13 @@ const getInjectorInstance = (
594614

595615
const injector = idToInjector.get(serializedInjector.id)!;
596616
const providerRecords = getInjectorProviders(injector);
597-
let value;
598617

599618
if (typeof serializedProvider.index === 'number') {
600619
const provider = providerRecords[serializedProvider.index];
601-
value = injector.get(provider.token, null, {optional: true});
620+
return injector.get(provider.token, null, {optional: true});
602621
} else if (Array.isArray(serializedProvider.index)) {
603622
const providers = serializedProvider.index.map((index) => providerRecords[index]);
604-
value = injector.get(providers[0].token, null, {optional: true});
623+
return injector.get(providers[0].token, null, {optional: true});
605624
}
606-
return value;
625+
return null;
607626
};

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

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('parseRoutes', () => {
1616
component: 'no-name',
1717
path: '/',
1818
children: [],
19+
data: [],
1920
isAux: false,
2021
isLazy: false,
2122
isActive: false,
@@ -33,6 +34,7 @@ describe('parseRoutes', () => {
3334
expect(parsedRoutes).toEqual({
3435
'component': 'homeComponent',
3536
'path': '/',
37+
'data': [],
3638
'children': [],
3739
'isAux': false,
3840
'isLazy': false,
@@ -98,64 +100,85 @@ describe('parseRoutes', () => {
98100
{
99101
'component': 'component-one',
100102
'canActivateGuards': [],
103+
'canActivateChildGuards': [],
104+
'canMatchGuards': [],
105+
'canDeactivateGuards': [],
101106
'providers': [],
102107
'path': '/(outlet:component-one)',
108+
'title': undefined,
109+
'pathMatch': undefined,
110+
'data': [],
103111
'isAux': true,
104112
'isLazy': false,
105-
'isActive': false,
106-
pathMatch: undefined,
107-
title: undefined,
113+
'isActive': undefined,
108114
},
109115
{
110116
'component': 'component-two',
111117
'canActivateGuards': [],
118+
'canActivateChildGuards': [],
119+
'canMatchGuards': [],
120+
'canDeactivateGuards': [],
112121
'providers': [],
113122
'path': '/component-two',
123+
'title': undefined,
124+< E377 div class="diff-text-inner"> 'pathMatch': undefined,
125+
'data': [{'key': 'name', 'value': 'component-two'}],
114126
'isAux': false,
115127
'isLazy': false,
116-
'isActive': false,
117-
pathMatch: undefined,
118-
title: undefined,
128+
'isActive': undefined,
119129
'children': [
120130
{
121131
'component': 'component-two-two',
122132
'canActivateGuards': [],
133+
'canActivateChildGuards': [],
134+
'canMatchGuards': [],
135+
'canDeactivateGuards': [],
123136
'providers': [],
124137
'path': '/component-two/component-two-two',
138+
'title': undefined,
139+
'pathMatch': undefined,
140+
'data': [],
125141
'isAux': false,
126142
'isLazy': false,
127-
'isActive': false,
128-
pathMatch: undefined,
129-
title: undefined,
143+
'isActive': undefined,
130144
},
131145
],
132146
},
133147
{
134148
'component': 'lazy [Lazy]',
135149
'canActivateGuards': [],
150+
'canActivateChildGuards': [],
151+
'canMatchGuards': [],
152+
'canDeactivateGuards': [],
136153
'providers': [],
137154
'path': '/lazy',
155+
'title': undefined,
156+
'pathMatch': undefined,
157+
'data': [],
138158
'isAux': false,
139159
'isLazy': true,
140-
'isActive': false,
141-
pathMatch: undefined,
142-
title: undefined,
160+
'isActive': undefined,
143161
},
144162
{
145163
'component': 'redirect -> redirecting to -> "redirectTo"',
146164
'canActivateGuards': [],
165+
'canActivateChildGuards': [],
166+
'canMatchGuards': [],
167+
'canDeactivateGuards': [],
147168
'providers': [],
148169
'path': '/redirect',
170+
'title': undefined,
171+
'pathMatch': undefined,
172+
'data': [],
149173
'isAux': false,
150174
'isLazy': false,
151-
'isActive': false,
152-
pathMatch: undefined,
153-
title: undefined,
175+
'isActive': undefined,
154176
},
155177
],
156178
'isAux': false,
157179
'isLazy': false,
180+
'data': [],
158181
'isActive': false,
159-
});
182+
} as any);
160183
});
161184
});

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

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,33 @@
88

99
import {Route} from '../../../protocol';
1010

11-
// todo(aleksanderbodurri): type these properly
12-
type AngularRoute = any;
11+
export type RoutePropertyType =
12+
| 'canActivate'
13+
| 'canActivateChild'
14+
| 'canDeactivate'
15+
| 'canMatch'
16+
| 'providers'
17+
| 'component';
18+
19+
// todo(sumitarora): type these properly in another PR
20+
type AngularRoute = {
21+
title?: string;
22+
path?: string;
23+
pathMatch?: 'prefix' | 'full' | undefined;
24+
component?: any;
25+
redirectTo?: any;
26+
outlet?: string | undefined;
27+
canActivate?: any[];
28+
canMatch?: any[];
29+
canActivateChild?: any[];
30+
canDeactivate?: any[];
31+
providers?: any[];
32+
data?: any;
33+
children?: Routes;
34+
loadChildren?: any;
35+
loadComponent?: any;
36+
_loadedRoutes?: any;
37+
};
1338
type Routes = any;
1439
type Router = any;
1540

@@ -31,16 +56,16 @@ export function parseRoutes(router: Router): Route {
3156
return root;
3257
}
3358

34-
function getGuardNames(child: AngularRoute, type: string): string[] | null {
59+
function getGuardNames(child: AngularRoute, type: RoutePropertyType): string[] {
3560
const guards = child?.[type] || [];
3661
const names = guards.map((g: any) => g.name);
37-
return names || null;
62+
return names || [];
3863
}
3964

40-
function getProviderName(child: any): string[] | null {
65+
function getProviderName(child: any): string[] {
4166
const providers = child?.providers || [];
4267
const names = providers.map((p: any) => p.name);
43-
return names || null;
68+
return names || [];
4469
}
4570

4671
function assignChildrenToParent(
@@ -70,7 +95,7 @@ function assignChildrenToParent(
7095
canActivateGuards: getGuardNames(child, 'canActivate'),
7196
canActivateChildGuards: getGuardNames(child, 'canActivateChild'),
7297
canMatchGuards: getGuardNames(child, 'canMatch'),
73-
canDeActivateGuards: getGuardNames(child, 'canDeactivate'),
98+
canDeactivateGuards: getGuardNames(child, 'canDeactivate'),
7499
providers: getProviderName(child),
75100
path: routePath,
76101
data: [],
@@ -110,33 +135,50 @@ function childRouteName(child: AngularRoute): string {
110135
}
111136
}
112137

113-
export function getElementRefByName(ref: string, routes: any[], name: string): any | null {
138+
/**
139+
* Get the element reference by type & name from the routes array. Called recursively to search through all children.
140+
* @param type - type of element to search for (canActivate, canActivateChild, canDeactivate, canLoad, providers)
141+
* @param routes - array of routes to search through
142+
* @param name - name of the element to search for refers to the name of the guard or provider
143+
* @returns - the element reference if found, otherwise null
144+
*/
145+
export function getElementRefByName(
146+
type: RoutePropertyType,
147+
routes: AngularRoute[],
148+
name: string,
149+
): any | null {
114150
for (const element of routes) {
115-
if (element[ref]) {
116-
for (const guard of element[ref]) {
151+
if (element[type]) {
152+
for (const guard of element[type]) {
117153
if (guard.name === name) {
118154
return guard;
119155
}
120156
}
121157
}
122158

123159
if (element?._loadedRoutes) {
124-
const result = getElementRefByName(ref, element._loadedRoutes, name);
160+
const result = getElementRefByName(type, element._loadedRoutes, name);
125161
if (result !== null) {
126162
return result;
127163
}
128164
}
129165

130166
if (element?.children) {
131-
const result = getElementRefByName(ref, element.children, name);
167+
const result = getElementRefByName(type, element.children, name);
132168
if (result !== null) {
133169
return result;
134170
}
135171
}
136172
}
137173
}
138174

139-
export function getComponentRefByName(routes: any[], name: string): any | null {
175+
/**
176+
* Get the componet reference by name from the routes array. Called recursively to search through all children.
177+
* @param routes - array of routes to search through
178+
* @param name - name of the component to search for
179+
* @returns - the element reference if found, otherwise null
180+
*/
181+
export function getComponentRefByName(routes: AngularRoute[], name: string): any | null {
140182
for (const element of routes) {
141183
if (element?.component?.name === name) {
142184
return element.component;

‎devtools/projects/ng-devtools-backend/src/public-api.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@
1313
export * from './lib';
1414
export {findNodeFromSerializedPosition} from './lib/component-tree/component-tree';
1515
export {viewSourceFromRouter} from './lib/client-event-subscribers';
16+
export {RoutePropertyType} from './lib/router-tree';

‎devtools/projects/ng-devtools/src/lib/application-operations/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ export abstract class ApplicationOperations {
1313
abstract viewSource(position: ElementPosition, target: Frame, directiveIndex?: number): void;
1414
abstract selectDomElement(position: ElementPosition, target: Frame): void;
1515
abstract inspect(directivePosition: DirectivePosition, objectPath: string[], target: Frame): void;
16-
abstract viewSourceFromRouter(name: string, type: string): void;
16+
abstract viewSourceFromRouter(name: string, type: string, target: Frame): void;
1717
}

0 commit comments

Comments
 (0)
0