Skip to content

Commit

Permalink
fix(@schematics/angular): change findNodes to stop recursive lookup…
Browse files Browse the repository at this point in the history
… for child nodes of kind

Curtrently, when a node of kind is found, we recursivly continue to look up it's child nodes until the end of the AST. This ends up returing other nodes which we were not looking for as typically we are looking for the first level of children of the specified kind.

By default now, we stop recursivly looking for child nodes of kind when we encounter one.

Closes #15117
  • Loading branch information
alan-agius4 authored and vikerman committed Jul 26, 2019
1 parent ef10a0b commit aa052c5
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
6 changes: 4 additions & 2 deletions packages/schematics/angular/utility/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,11 @@ export function insertImport(source: ts.SourceFile, fileToEdit: string, symbolNa
* @param node
* @param kind
* @param max The maximum number of items to return.
* @param recursive Continue looking for nodes of kind recursive until end
* the last child even when node of kind has been found.
* @return all nodes of kind, or [] if none is found
*/
export function findNodes(node: ts.Node, kind: ts.SyntaxKind, max = Infinity): ts.Node[] {
export function findNodes(node: ts.Node, kind: ts.SyntaxKind, max = Infinity, recursive = false): ts.Node[] {
if (!node || max == 0) {
return [];
}
Expand All @@ -105,7 +107,7 @@ export function findNodes(node: ts.Node, kind: ts.SyntaxKind, max = Infinity): t
arr.push(node);
max--;
}
if (max > 0) {
if (max > 0 && (recursive || node.kind !== kind)) {
for (const child of node.getChildren()) {
findNodes(child, kind, max).forEach(node => {
if (max > 0) {
Expand Down
35 changes: 35 additions & 0 deletions packages/schematics/angular/utility/ast-utils_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,41 @@ describe('ast utils', () => {
);
});

it('should add a route to the routes to the correct array when having nested object literal', () => {
const moduleContent = `
import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { AppComponent } from './app.component';
const routes = [
{ path: 'foo', component: FooComponent, data: { path: 'test' }}
];
@NgModule({
declarations: [
AppComponent
],
imports: [
BrowserModule,
RouterModule.forRoot(routes)
],
bootstrap: [AppComponent]
})
export class AppModule { }
`;

const source = getTsSource(modulePath, moduleContent);
const changes = addRouteDeclarationToModule(
source,
'./src/app', `{ path: 'bar', component: BarComponent }`,
);
const output = applyChanges(modulePath, moduleContent, [changes]);
expect(output).toMatch(
// tslint:disable-next-line:max-line-length
/const routes = \[\r?\n?\s*{ path: 'foo', component: FooComponent, data: { path: 'test' }},\r?\n?\s*{ path: 'bar', component: BarComponent }\r?\n?\s*\]/,
);
});

it('should add a route to the routes argument of RouteModule', () => {
const moduleContent = `
import { BrowserModule } from '@angular/platform-browser';
Expand Down

0 comments on commit aa052c5

Please sign in to comment.