From 3c06a5dc256b19c7061c4c8a829bbf6cbb177870 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 17 Nov 2016 20:11:55 -0800 Subject: [PATCH] refactor(comiler): various cleanups --- .../compiler-cli/src/compiler_host.ts | 4 +- .../@angular/compiler-cli/src/extractor.ts | 4 +- .../src/path_mapped_compiler_host.ts | 9 +-- modules/@angular/compiler/src/aot/compiler.ts | 76 ++++++++----------- .../compiler/src/aot/static_reflector.ts | 55 ++++++++------ .../test/aot/static_reflector_spec.ts | 21 ++++- 6 files changed, 89 insertions(+), 80 deletions(-) diff --git a/modules/@angular/compiler-cli/src/compiler_host.ts b/modules/@angular/compiler-cli/src/compiler_host.ts index 97f31209d6..a332c657a9 100644 --- a/modules/@angular/compiler-cli/src/compiler_host.ts +++ b/modules/@angular/compiler-cli/src/compiler_host.ts @@ -54,13 +54,13 @@ export class CompilerHost implements AotCompilerHost { throw new Error('Resolution of relative paths requires a containing file.'); } // Any containing file gives the same result for absolute imports - containingFile = path.join(this.basePath, 'index.ts'); + containingFile = this.getCanonicalFileName(path.join(this.basePath, 'index.ts')); } m = m.replace(EXT, ''); const resolved = ts.resolveModuleName(m, containingFile.replace(/\\/g, '/'), this.options, this.context) .resolvedModule; - return resolved ? resolved.resolvedFileName : null; + return resolved ? this.getCanonicalFileName(resolved.resolvedFileName) : null; }; /** diff --git a/modules/@angular/compiler-cli/src/extractor.ts b/modules/@angular/compiler-cli/src/extractor.ts index 2225ce0afa..38913416be 100644 --- a/modules/@angular/compiler-cli/src/extractor.ts +++ b/modules/@angular/compiler-cli/src/extractor.ts @@ -32,8 +32,8 @@ export class Extractor { const programSymbols: compiler.StaticSymbol[] = extractProgramSymbols(this.program, this.staticReflector, this.compilerHost, this.options); - const {ngModules, files} = compiler.analyzeAndValidateNgModules( - programSymbols, {transitiveModules: true}, this.metadataResolver); + const {ngModules, files} = + compiler.analyzeAndValidateNgModules(programSymbols, {}, this.metadataResolver); return compiler.loadNgModuleDirectives(ngModules).then(() => { const errors: compiler.ParseError[] = []; diff --git a/modules/@angular/compiler-cli/src/path_mapped_compiler_host.ts b/modules/@angular/compiler-cli/src/path_mapped_compiler_host.ts index 42c62bb869..6f01d29800 100644 --- a/modules/@angular/compiler-cli/src/path_mapped_compiler_host.ts +++ b/modules/@angular/compiler-cli/src/path_mapped_compiler_host.ts @@ -48,7 +48,7 @@ export class PathMappedCompilerHost extends CompilerHost { throw new Error('Resolution of relative paths requires a containing file.'); } // Any containing file gives the same result for absolute imports - containingFile = path.join(this.basePath, 'index.ts'); + containingFile = this.getCanonicalFileName(path.join(this.basePath, 'index.ts')); } for (const root of this.options.rootDirs || ['']) { const rootedContainingFile = path.join(root, containingFile); @@ -58,7 +58,7 @@ export class PathMappedCompilerHost extends CompilerHost { if (this.options.traceResolution) { console.log('resolve', m, containingFile, '=>', resolved.resolvedFileName); } - return resolved.resolvedFileName; + return this.getCanonicalFileName(resolved.resolvedFileName); } } } @@ -86,8 +86,7 @@ export class PathMappedCompilerHost extends CompilerHost { } const resolvable = (candidate: string) => { - const resolved = - this.getCanonicalFileName(this.moduleNameToFileName(candidate, importedFile)); + const resolved = this.moduleNameToFileName(candidate, importedFile); return resolved && resolved.replace(EXT, '') === importedFile.replace(EXT, ''); }; @@ -133,7 +132,7 @@ export class PathMappedCompilerHost extends CompilerHost { } } else { const sf = this.getSourceFile(rootedPath); - sf.fileName = this.getCanonicalFileName(sf.fileName); + sf.fileName = sf.fileName; const metadata = this.metadataCollector.getMetadata(sf); return metadata ? [metadata] : []; } diff --git a/modules/@angular/compiler/src/aot/compiler.ts b/modules/@angular/compiler/src/aot/compiler.ts index d58488df9a..f2895f3996 100644 --- a/modules/@angular/compiler/src/aot/compiler.ts +++ b/modules/@angular/compiler/src/aot/compiler.ts @@ -46,14 +46,9 @@ export class AotCompiler { clearCache() { this._metadataResolver.clearCache(); } compileAll(rootFiles: string[]): Promise { - const options = { - transitiveModules: true, - excludeFilePattern: this._options.excludeFilePattern, - includeFilePattern: this._options.includeFilePattern - }; - const programSymbols = extractProgramSymbols(this._staticReflector, rootFiles, options); + const programSymbols = extractProgramSymbols(this._staticReflector, rootFiles, this._options); const {ngModuleByPipeOrDirective, files, ngModules} = - analyzeAndValidateNgModules(programSymbols, options, this._metadataResolver); + analyzeAndValidateNgModules(programSymbols, this._options, this._metadataResolver); return loadNgModuleDirectives(ngModules).then(() => { const sourceModules = files.map( file => this._compileSrcFile( @@ -288,7 +283,7 @@ export interface NgAnalyzedModules { // Returns all the source files and a mapping from modules to directives export function analyzeNgModules( programStaticSymbols: StaticSymbol[], - options: {transitiveModules: boolean, includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, + options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, metadataResolver: CompileMetadataResolver): NgAnalyzedModules { const {ngModules, symbolsMissingModule} = _createNgModules(programStaticSymbols, options, metadataResolver); @@ -296,7 +291,8 @@ export function analyzeNgModules( } export function analyzeAndValidateNgModules( - programStaticSymbols: StaticSymbol[], options: {transitiveModules: boolean}, + programStaticSymbols: StaticSymbol[], + options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, metadataResolver: CompileMetadataResolver): NgAnalyzedModules { const result = analyzeNgModules(programStaticSymbols, options, metadataResolver); if (result.symbolsMissingModule && result.symbolsMissingModule.length) { @@ -370,31 +366,27 @@ export function extractProgramSymbols( staticReflector: StaticReflector, files: string[], options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp} = {}): StaticSymbol[] { const staticSymbols: StaticSymbol[] = []; - files - .filter( - fileName => _filterFileByPatterns( - fileName, options.includeFilePattern, options.includeFilePattern)) - .forEach(sourceFile => { - const moduleMetadata = staticReflector.getModuleMetadata(sourceFile); - if (!moduleMetadata) { - console.log(`WARNING: no metadata found for ${sourceFile}`); - return; - } + files.filter(fileName => _filterFileByPatterns(fileName, options)).forEach(sourceFile => { + const moduleMetadata = staticReflector.getModuleMetadata(sourceFile); + if (!moduleMetadata) { + console.log(`WARNING: no metadata found for ${sourceFile}`); + return; + } - const metadata = moduleMetadata['metadata']; + const metadata = moduleMetadata['metadata']; - if (!metadata) { - return; - } + if (!metadata) { + return; + } - for (const symbol of Object.keys(metadata)) { - if (metadata[symbol] && metadata[symbol].__symbolic == 'error') { - // Ignore symbols that are only included to record error information. - continue; - } - staticSymbols.push(staticReflector.findDeclaration(sourceFile, symbol, sourceFile)); - } - }); + for (const symbol of Object.keys(metadata)) { + if (metadata[symbol] && metadata[symbol].__symbolic == 'error') { + // Ignore symbols that are only included to record error information. + continue; + } + staticSymbols.push(staticReflector.getStaticSymbol(sourceFile, symbol)); + } + }); return staticSymbols; } @@ -404,7 +396,7 @@ export function extractProgramSymbols( // are also declared by a module. function _createNgModules( programStaticSymbols: StaticSymbol[], - options: {transitiveModules: boolean, includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, + options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, metadataResolver: CompileMetadataResolver): {ngModules: CompileNgModuleMetadata[], symbolsMissingModule: StaticSymbol[]} { const ngModules = new Map(); @@ -412,9 +404,7 @@ function _createNgModules( const ngModulePipesAndDirective = new Set(); const addNgModule = (staticSymbol: any) => { - if (ngModules.has(staticSymbol) || - !_filterFileByPatterns( - staticSymbol.filePath, options.includeFilePattern, options.excludeFilePattern)) { + if (ngModules.has(staticSymbol) || !_filterFileByPatterns(staticSymbol.filePath, options)) { return false; } const ngModule = metadataResolver.getUnloadedNgModuleMetadata(staticSymbol, false, false); @@ -422,10 +412,8 @@ function _createNgModules( ngModules.set(ngModule.type.reference, ngModule); ngModule.declaredDirectives.forEach((dir) => ngModulePipesAndDirective.add(dir.reference)); ngModule.declaredPipes.forEach((pipe) => ngModulePipesAndDirective.add(pipe.reference)); - if (options.transitiveModules) { - // For every input modules add the list of transitively included modules - ngModule.transitiveModule.modules.forEach(modMeta => addNgModule(modMeta.type.reference)); - } + // For every input module add the list of transitively included modules + ngModule.transitiveModule.modules.forEach(modMeta => addNgModule(modMeta.type.reference)); } return !!ngModule; }; @@ -444,13 +432,13 @@ function _createNgModules( } function _filterFileByPatterns( - fileName: string, includeFilePattern: RegExp, excludeFilePattern: RegExp) { + fileName: string, options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp} = {}) { let match = true; - if (includeFilePattern) { - match = match && !!includeFilePattern.exec(fileName); + if (options.includeFilePattern) { + match = match && !!options.includeFilePattern.exec(fileName); } - if (excludeFilePattern) { - match = match && !excludeFilePattern.exec(fileName); + if (options.excludeFilePattern) { + match = match && !options.excludeFilePattern.exec(fileName); } return match; } \ No newline at end of file diff --git a/modules/@angular/compiler/src/aot/static_reflector.ts b/modules/@angular/compiler/src/aot/static_reflector.ts index 848e8225a7..460769dfb7 100644 --- a/modules/@angular/compiler/src/aot/static_reflector.ts +++ b/modules/@angular/compiler/src/aot/static_reflector.ts @@ -26,13 +26,13 @@ const ANGULAR_IMPORT_LOCATIONS = { * templates statically. */ export class StaticReflector implements ReflectorReader { - private typeCache = new Map(); + private staticSymbolCache = new Map(); + private declarationCache = new Map(); private annotationCache = new Map(); private propertyCache = new Map(); private parameterCache = new Map(); private metadataCache = new Map(); private conversionMap = new Map any>(); - private declarationMap = new Map(); private opaqueToken: StaticSymbol; constructor(private host: AotCompilerHost) { this.initializeConversionMap(); } @@ -204,10 +204,10 @@ export class StaticReflector implements ReflectorReader { getStaticSymbol(declarationFile: string, name: string, members?: string[]): StaticSymbol { const memberSuffix = members ? `.${ members.join('.')}` : ''; const key = `"${declarationFile}".${name}${memberSuffix}`; - let result = this.typeCache.get(key); + let result = this.staticSymbolCache.get(key); if (!result) { result = new StaticSymbol(declarationFile, name, members); - this.typeCache.set(key, result); + this.staticSymbolCache.set(key, result); } return result; } @@ -220,15 +220,20 @@ export class StaticReflector implements ReflectorReader { } return resolvedModulePath; }; + const cacheKey = `${filePath}|${symbolName}`; + let staticSymbol = this.declarationCache.get(cacheKey); + if (staticSymbol) { + return staticSymbol; + } const metadata = this.getModuleMetadata(filePath); if (metadata) { // If we have metadata for the symbol, this is the original exporting location. if (metadata['metadata'][symbolName]) { - return this.getStaticSymbol(filePath, symbolName); + staticSymbol = this.getStaticSymbol(filePath, symbolName); } // If no, try to find the symbol in one of the re-export location - if (metadata['exports']) { + if (!staticSymbol && metadata['exports']) { // Try and find the symbol in the list of explicitly re-exported symbols. for (const moduleExport of metadata['exports']) { if (moduleExport.export) { @@ -244,43 +249,43 @@ export class StaticReflector implements ReflectorReader { if (typeof exportSymbol !== 'string') { symName = exportSymbol.name; } - return this.resolveExportedSymbol(resolveModule(moduleExport.from), symName); + staticSymbol = this.resolveExportedSymbol(resolveModule(moduleExport.from), symName); } } } - // Try to find the symbol via export * directives. - for (const moduleExport of metadata['exports']) { - if (!moduleExport.export) { - const resolvedModule = resolveModule(moduleExport.from); - const candidateSymbol = this.resolveExportedSymbol(resolvedModule, symbolName); - if (candidateSymbol) return candidateSymbol; + if (!staticSymbol) { + // Try to find the symbol via export * directives. + for (const moduleExport of metadata['exports']) { + if (!moduleExport.export) { + const resolvedModule = resolveModule(moduleExport.from); + const candidateSymbol = this.resolveExportedSymbol(resolvedModule, symbolName); + if (candidateSymbol) { + staticSymbol = candidateSymbol; + break; + } + } } } } } - return null; + this.declarationCache.set(cacheKey, staticSymbol); + return staticSymbol; } findDeclaration(module: string, symbolName: string, containingFile?: string): StaticSymbol { - const cacheKey = `${module}|${symbolName}|${containingFile}`; - let symbol = this.declarationMap.get(cacheKey); - if (symbol) { - return symbol; - } try { const filePath = this.host.moduleNameToFileName(module, containingFile); - + let symbol: StaticSymbol; if (!filePath) { // If the file cannot be found the module is probably referencing a declared module // for which there is no disambiguating file and we also don't need to track // re-exports. Just use the module name. - return this.getStaticSymbol(module, symbolName); + symbol = this.getStaticSymbol(module, symbolName); + } else { + symbol = this.resolveExportedSymbol(filePath, symbolName) || + this.getStaticSymbol(filePath, symbolName); } - - let symbol = this.resolveExportedSymbol(filePath, symbolName) || - this.getStaticSymbol(filePath, symbolName); - this.declarationMap.set(cacheKey, symbol); return symbol; } catch (e) { console.error(`can't resolve module ${module} from ${containingFile}`); diff --git a/modules/@angular/compiler/test/aot/static_reflector_spec.ts b/modules/@angular/compiler/test/aot/static_reflector_spec.ts index 71b554eefd..d65275377a 100644 --- a/modules/@angular/compiler/test/aot/static_reflector_spec.ts +++ b/modules/@angular/compiler/test/aot/static_reflector_spec.ts @@ -502,6 +502,21 @@ describe('StaticReflector', () => { expect(symbol.name).toEqual('Thirty'); expect(symbol.filePath).toEqual('/tmp/src/reexport/src/origin30.d.ts'); }); + + it('should cache tracing a named export', () => { + const moduleNameToFileNameSpy = spyOn(host, 'moduleNameToFileName').and.callThrough(); + const getMetadataForSpy = spyOn(host, 'getMetadataFor').and.callThrough(); + reflector.findDeclaration('./reexport/reexport', 'One', '/tmp/src/main.ts'); + moduleNameToFileNameSpy.calls.reset(); + getMetadataForSpy.calls.reset(); + + const symbol = reflector.findDeclaration('./reexport/reexport', 'One', '/tmp/src/main.ts'); + expect(moduleNameToFileNameSpy.calls.count()).toBe(1); + expect(getMetadataForSpy.calls.count()).toBe(0); + expect(symbol.name).toEqual('One'); + expect(symbol.filePath).toEqual('/tmp/src/reexport/src/origin1.d.ts'); + }); + }); class MockAotCompilerHost implements AotCompilerHost { @@ -550,7 +565,7 @@ class MockAotCompilerHost implements AotCompilerHost { if (modulePath.indexOf('.') === 0) { const baseName = pathTo(containingFile, modulePath); const tsName = baseName + '.ts'; - if (this.getMetadataFor(tsName)) { + if (this._getMetadataFor(tsName)) { return tsName; } return baseName + '.d.ts'; @@ -558,7 +573,9 @@ class MockAotCompilerHost implements AotCompilerHost { return '/tmp/' + modulePath + '.d.ts'; } - getMetadataFor(moduleId: string): any { + getMetadataFor(moduleId: string): any { return this._getMetadataFor(moduleId); } + + private _getMetadataFor(moduleId: string): any { const data: {[key: string]: any} = { '/tmp/@angular/common/src/forms-deprecated/directives.d.ts': [{ '__symbolic': 'module',