From 7d49ae511f0dc8c5712d5a166ff146b37753bccb Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Tue, 7 May 2019 16:33:39 -0600 Subject: [PATCH 1/3] Allow TraverseCallback to bail out early It is used in getChildByName to search children, which led to *always* searching all children, which is especially problematic for any deeply nested structures. Improves convert time by 25% on the monaco.d.ts file in #975 --- src/lib/models/reflections/abstract.ts | 24 ++++++++----------- src/lib/models/reflections/container.ts | 9 +++---- src/lib/models/reflections/declaration.ts | 29 ++++++++++++++++------- src/lib/models/reflections/parameter.ts | 4 +++- src/lib/models/reflections/project.ts | 13 ++-------- src/lib/models/reflections/signature.ts | 17 +++++++++---- 6 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/lib/models/reflections/abstract.ts b/src/lib/models/reflections/abstract.ts index 145edfb88..f399d9d5f 100644 --- a/src/lib/models/reflections/abstract.ts +++ b/src/lib/models/reflections/abstract.ts @@ -258,7 +258,11 @@ export enum TraverseProperty { } export interface TraverseCallback { - (reflection: Reflection, property: TraverseProperty): void; + /** + * May return false to bail out of any further iteration. To preserve backwards compatibility, if + * a function returns undefined, iteration must continue. + */ + (reflection: Reflection, property: TraverseProperty): boolean | void; } /** @@ -468,20 +472,11 @@ export abstract class Reflection { return false; } - /** - * @param name The name of the child to look for. Might contain a hierarchy. - */ - getChildByName(name: string): Reflection; - - /** - * @param names The name hierarchy of the child to look for. - */ - getChildByName(names: string[]): Reflection; - /** * Return a child by its name. * - * @returns The found child or NULL. + * @param names The name hierarchy of the child to look for. + * @returns The found child or undefined. */ getChildByName(arg: string | string[]): Reflection | undefined { const names: string[] = Array.isArray(arg) ? arg : arg.split('.'); @@ -492,9 +487,10 @@ export abstract class Reflection { if (child.name === name) { if (names.length <= 1) { result = child; - } else if (child) { + } else { result = child.getChildByName(names.slice(1)); } + return false; } }); @@ -610,7 +606,7 @@ export abstract class Reflection { const lines = [indent + this.toString()]; indent += ' '; - this.traverse((child, property) => { + this.traverse((child) => { lines.push(child.toStringHierarchy(indent)); }); diff --git a/src/lib/models/reflections/container.ts b/src/lib/models/reflections/container.ts index 777291006..a39310983 100644 --- a/src/lib/models/reflections/container.ts +++ b/src/lib/models/reflections/container.ts @@ -2,6 +2,7 @@ import { Reflection, ReflectionKind, TraverseCallback, TraverseProperty } from ' import { ReflectionCategory } from '../ReflectionCategory'; import { ReflectionGroup } from '../ReflectionGroup'; import { DeclarationReflection } from './declaration'; +import { toArray } from 'lodash'; export class ContainerReflection extends Reflection { /** @@ -38,10 +39,10 @@ export class ContainerReflection extends Reflection { * @param callback The callback function that should be applied for each child reflection. */ traverse(callback: TraverseCallback) { - if (this.children) { - this.children.slice().forEach((child: DeclarationReflection) => { - callback(child, TraverseProperty.Children); - }); + for (const child of toArray(this.children).slice()) { + if (callback(child, TraverseProperty.Children) === false) { + return; + } } } diff --git a/src/lib/models/reflections/declaration.ts b/src/lib/models/reflections/declaration.ts index 45290eeb9..e6e171e19 100644 --- a/src/lib/models/reflections/declaration.ts +++ b/src/lib/models/reflections/declaration.ts @@ -3,6 +3,7 @@ import { Type, ReflectionType } from '../types/index'; import { ContainerReflection } from './container'; import { SignatureReflection } from './signature'; import { TypeParameterReflection } from './type-parameter'; +import { toArray } from 'lodash'; /** * Stores hierarchical type data. @@ -152,28 +153,40 @@ export class DeclarationReflection extends ContainerReflection implements Defaul * @param callback The callback function that should be applied for each child reflection. */ traverse(callback: TraverseCallback) { - if (this.typeParameters) { - this.typeParameters.slice().forEach((parameter) => callback(parameter, TraverseProperty.TypeParameter)); + for (const parameter of toArray(this.typeParameters).slice()) { + if (callback(parameter, TraverseProperty.TypeParameter) === false) { + return; + } } if (this.type instanceof ReflectionType) { - callback(this.type.declaration, TraverseProperty.TypeLiteral); + if (callback(this.type.declaration, TraverseProperty.TypeLiteral) === false) { + return; + } } - if (this.signatures) { - this.signatures.slice().forEach((signature) => callback(signature, TraverseProperty.Signatures)); + for (const signature of toArray(this.signatures).slice()) { + if (callback(signature, TraverseProperty.Signatures) === false) { + return; + } } if (this.indexSignature) { - callback(this.indexSignature, TraverseProperty.IndexSignature); + if (callback(this.indexSignature, TraverseProperty.IndexSignature) === false) { + return; + } } if (this.getSignature) { - callback(this.getSignature, TraverseProperty.GetSignature); + if (callback(this.getSignature, TraverseProperty.GetSignature) === false) { + return; + } } if (this.setSignature) { - callback(this.setSignature, TraverseProperty.SetSignature); + if (callback(this.setSignature, TraverseProperty.SetSignature) === false) { + return; + } } super.traverse(callback); diff --git a/src/lib/models/reflections/parameter.ts b/src/lib/models/reflections/parameter.ts index 5a493737c..bc92c5efd 100644 --- a/src/lib/models/reflections/parameter.ts +++ b/src/lib/models/reflections/parameter.ts @@ -19,7 +19,9 @@ export class ParameterReflection extends Reflection implements DefaultValueConta */ traverse(callback: TraverseCallback) { if (this.type instanceof ReflectionType) { - callback(this.type.declaration, TraverseProperty.TypeLiteral); + if (callback(this.type.declaration, TraverseProperty.TypeLiteral) === false) { + return; + } } super.traverse(callback); diff --git a/src/lib/models/reflections/project.ts b/src/lib/models/reflections/project.ts index 6e4fbd9c1..66becaa59 100644 --- a/src/lib/models/reflections/project.ts +++ b/src/lib/models/reflections/project.ts @@ -78,22 +78,13 @@ export class ProjectReflection extends ContainerReflection { return values; } - /** - * @param name The name to look for. Might contain a hierarchy. - */ - findReflectionByName(name: string): Reflection; - - /** - * @param names The name hierarchy to look for. - */ - findReflectionByName(names: string[]): Reflection; - /** * Try to find a reflection by its name. * + * @param names The name hierarchy to look for, if a string, the name will be split on "." * @return The found reflection or undefined. */ - findReflectionByName(arg: any): Reflection | undefined { + findReflectionByName(arg: string | string[]): Reflection | undefined { const names: string[] = Array.isArray(arg) ? arg : arg.split('.'); const name = names.pop(); diff --git a/src/lib/models/reflections/signature.ts b/src/lib/models/reflections/signature.ts index f1cb0420f..33c0d9447 100644 --- a/src/lib/models/reflections/signature.ts +++ b/src/lib/models/reflections/signature.ts @@ -3,6 +3,7 @@ import { Reflection, TypeContainer, TypeParameterContainer, TraverseProperty, Tr import { ContainerReflection } from './container'; import { ParameterReflection } from './parameter'; import { TypeParameterReflection } from './type-parameter'; +import { toArray } from 'lodash'; export class SignatureReflection extends Reflection implements TypeContainer, TypeParameterContainer { parent?: ContainerReflection; @@ -57,15 +58,21 @@ export class SignatureReflection extends Reflection implements TypeContainer, Ty */ traverse(callback: TraverseCallback) { if (this.type instanceof ReflectionType) { - callback(this.type.declaration, TraverseProperty.TypeLiteral); + if (callback(this.type.declaration, TraverseProperty.TypeLiteral) === false) { + return; + } } - if (this.typeParameters) { - this.typeParameters.slice().forEach((parameter) => callback(parameter, TraverseProperty.TypeParameter)); + for (const parameter of toArray(this.typeParameters).slice()) { + if (callback(parameter, TraverseProperty.TypeParameter) === false) { + return; + } } - if (this.parameters) { - this.parameters.slice().forEach((parameter) => callback(parameter, TraverseProperty.Parameters)); + for (const parameter of toArray(this.parameters).slice()) { + if (callback(parameter, TraverseProperty.Parameters) === false) { + return; + } } super.traverse(callback); From 7728ec99bf14dec4da1357e5efb991e1901ef0ad Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Tue, 7 May 2019 20:45:20 -0600 Subject: [PATCH 2/3] Fix coverage report --- package.json | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 23159f109..4753a5ad4 100644 --- a/package.json +++ b/package.json @@ -80,5 +80,14 @@ "documentation", "generator", "gruntplugin" - ] + ], + "nyc": { + "extension": [ + ".ts", + ".tsx" + ], + "exclude": [ + "**/*.d.ts" + ] + } } From 27f254ff47085c25fa63836b137d3d2393b84a87 Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Wed, 8 May 2019 10:58:15 -0600 Subject: [PATCH 3/3] Remove unnecessary slice call --- src/lib/models/reflections/container.ts | 2 +- src/lib/models/reflections/declaration.ts | 4 ++-- src/lib/models/reflections/signature.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/models/reflections/container.ts b/src/lib/models/reflections/container.ts index a39310983..866bfbf79 100644 --- a/src/lib/models/reflections/container.ts +++ b/src/lib/models/reflections/container.ts @@ -39,7 +39,7 @@ export class ContainerReflection extends Reflection { * @param callback The callback function that should be applied for each child reflection. */ traverse(callback: TraverseCallback) { - for (const child of toArray(this.children).slice()) { + for (const child of toArray(this.children)) { if (callback(child, TraverseProperty.Children) === false) { return; } diff --git a/src/lib/models/reflections/declaration.ts b/src/lib/models/reflections/declaration.ts index e6e171e19..0ced543a9 100644 --- a/src/lib/models/reflections/declaration.ts +++ b/src/lib/models/reflections/declaration.ts @@ -153,7 +153,7 @@ export class DeclarationReflection extends ContainerReflection implements Defaul * @param callback The callback function that should be applied for each child reflection. */ traverse(callback: TraverseCallback) { - for (const parameter of toArray(this.typeParameters).slice()) { + for (const parameter of toArray(this.typeParameters)) { if (callback(parameter, TraverseProperty.TypeParameter) === false) { return; } @@ -165,7 +165,7 @@ export class DeclarationReflection extends ContainerReflection implements Defaul } } - for (const signature of toArray(this.signatures).slice()) { + for (const signature of toArray(this.signatures)) { if (callback(signature, TraverseProperty.Signatures) === false) { return; } diff --git a/src/lib/models/reflections/signature.ts b/src/lib/models/reflections/signature.ts index 33c0d9447..c1d1db399 100644 --- a/src/lib/models/reflections/signature.ts +++ b/src/lib/models/reflections/signature.ts @@ -63,13 +63,13 @@ export class SignatureReflection extends Reflection implements TypeContainer, Ty } } - for (const parameter of toArray(this.typeParameters).slice()) { + for (const parameter of toArray(this.typeParameters)) { if (callback(parameter, TraverseProperty.TypeParameter) === false) { return; } } - for (const parameter of toArray(this.parameters).slice()) { + for (const parameter of toArray(this.parameters)) { if (callback(parameter, TraverseProperty.Parameters) === false) { return; }