From 593e05dc97b677c5a99f26dae8186c2f485f42da Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 23 Nov 2016 12:08:44 -0800 Subject: [PATCH] Revert "refactor(comiler): various cleanups" This reverts commit ef386760911c248c47f42940acf22392273860e1. --- .../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, 80 insertions(+), 89 deletions(-) diff --git a/modules/@angular/compiler-cli/src/compiler_host.ts b/modules/@angular/compiler-cli/src/compiler_host.ts index a332c657a9..97f31209d6 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 = this.getCanonicalFileName(path.join(this.basePath, 'index.ts')); + containingFile = 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 ? this.getCanonicalFileName(resolved.resolvedFileName) : null; + return resolved ? resolved.resolvedFileName : null; }; /** diff --git a/modules/@angular/compiler-cli/src/extractor.ts b/modules/@angular/compiler-cli/src/extractor.ts index 38913416be..2225ce0afa 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, {}, this.metadataResolver); + const {ngModules, files} = compiler.analyzeAndValidateNgModules( + programSymbols, {transitiveModules: true}, 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 6f01d29800..42c62bb869 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 = this.getCanonicalFileName(path.join(this.basePath, 'index.ts')); + containingFile = 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 this.getCanonicalFileName(resolved.resolvedFileName); + return resolved.resolvedFileName; } } } @@ -86,7 +86,8 @@ export class PathMappedCompilerHost extends CompilerHost { } const resolvable = (candidate: string) => { - const resolved = this.moduleNameToFileName(candidate, importedFile); + const resolved = + this.getCanonicalFileName(this.moduleNameToFileName(candidate, importedFile)); return resolved && resolved.replace(EXT, '') === importedFile.replace(EXT, ''); }; @@ -132,7 +133,7 @@ export class PathMappedCompilerHost extends CompilerHost { } } else { const sf = this.getSourceFile(rootedPath); - sf.fileName = sf.fileName; + sf.fileName = this.getCanonicalFileName(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 f2895f3996..d58488df9a 100644 --- a/modules/@angular/compiler/src/aot/compiler.ts +++ b/modules/@angular/compiler/src/aot/compiler.ts @@ -46,9 +46,14 @@ export class AotCompiler { clearCache() { this._metadataResolver.clearCache(); } compileAll(rootFiles: string[]): Promise { - const programSymbols = extractProgramSymbols(this._staticReflector, rootFiles, this._options); + const options = { + transitiveModules: true, + excludeFilePattern: this._options.excludeFilePattern, + includeFilePattern: this._options.includeFilePattern + }; + const programSymbols = extractProgramSymbols(this._staticReflector, rootFiles, options); const {ngModuleByPipeOrDirective, files, ngModules} = - analyzeAndValidateNgModules(programSymbols, this._options, this._metadataResolver); + analyzeAndValidateNgModules(programSymbols, options, this._metadataResolver); return loadNgModuleDirectives(ngModules).then(() => { const sourceModules = files.map( file => this._compileSrcFile( @@ -283,7 +288,7 @@ export interface NgAnalyzedModules { // Returns all the source files and a mapping from modules to directives export function analyzeNgModules( programStaticSymbols: StaticSymbol[], - options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, + options: {transitiveModules: boolean, includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, metadataResolver: CompileMetadataResolver): NgAnalyzedModules { const {ngModules, symbolsMissingModule} = _createNgModules(programStaticSymbols, options, metadataResolver); @@ -291,8 +296,7 @@ export function analyzeNgModules( } export function analyzeAndValidateNgModules( - programStaticSymbols: StaticSymbol[], - options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, + programStaticSymbols: StaticSymbol[], options: {transitiveModules: boolean}, metadataResolver: CompileMetadataResolver): NgAnalyzedModules { const result = analyzeNgModules(programStaticSymbols, options, metadataResolver); if (result.symbolsMissingModule && result.symbolsMissingModule.length) { @@ -366,27 +370,31 @@ export function extractProgramSymbols( staticReflector: StaticReflector, files: string[], options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp} = {}): StaticSymbol[] { const staticSymbols: StaticSymbol[] = []; - files.filter(fileName => _filterFileByPatterns(fileName, options)).forEach(sourceFile => { - const moduleMetadata = staticReflector.getModuleMetadata(sourceFile); - if (!moduleMetadata) { - console.log(`WARNING: no metadata found for ${sourceFile}`); - return; - } + 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; + } - 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.getStaticSymbol(sourceFile, symbol)); - } - }); + 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)); + } + }); return staticSymbols; } @@ -396,7 +404,7 @@ export function extractProgramSymbols( // are also declared by a module. function _createNgModules( programStaticSymbols: StaticSymbol[], - options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, + options: {transitiveModules: boolean, includeFilePattern?: RegExp, excludeFilePattern?: RegExp}, metadataResolver: CompileMetadataResolver): {ngModules: CompileNgModuleMetadata[], symbolsMissingModule: StaticSymbol[]} { const ngModules = new Map(); @@ -404,7 +412,9 @@ function _createNgModules( const ngModulePipesAndDirective = new Set(); const addNgModule = (staticSymbol: any) => { - if (ngModules.has(staticSymbol) || !_filterFileByPatterns(staticSymbol.filePath, options)) { + if (ngModules.has(staticSymbol) || + !_filterFileByPatterns( + staticSymbol.filePath, options.includeFilePattern, options.excludeFilePattern)) { return false; } const ngModule = metadataResolver.getUnloadedNgModuleMetadata(staticSymbol, false, false); @@ -412,8 +422,10 @@ function _createNgModules( ngModules.set(ngModule.type.reference, ngModule); ngModule.declaredDirectives.forEach((dir) => ngModulePipesAndDirective.add(dir.reference)); ngModule.declaredPipes.forEach((pipe) => ngModulePipesAndDirective.add(pipe.reference)); - // For every input module add the list of transitively included modules - ngModule.transitiveModule.modules.forEach(modMeta => addNgModule(modMeta.type.reference)); + if (options.transitiveModules) { + // For every input modules add the list of transitively included modules + ngModule.transitiveModule.modules.forEach(modMeta => addNgModule(modMeta.type.reference)); + } } return !!ngModule; }; @@ -432,13 +444,13 @@ function _createNgModules( } function _filterFileByPatterns( - fileName: string, options: {includeFilePattern?: RegExp, excludeFilePattern?: RegExp} = {}) { + fileName: string, includeFilePattern: RegExp, excludeFilePattern: RegExp) { let match = true; - if (options.includeFilePattern) { - match = match && !!options.includeFilePattern.exec(fileName); + if (includeFilePattern) { + match = match && !!includeFilePattern.exec(fileName); } - if (options.excludeFilePattern) { - match = match && !options.excludeFilePattern.exec(fileName); + if (excludeFilePattern) { + match = match && !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 460769dfb7..848e8225a7 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 staticSymbolCache = new Map(); - private declarationCache = new Map(); + private typeCache = 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.staticSymbolCache.get(key); + let result = this.typeCache.get(key); if (!result) { result = new StaticSymbol(declarationFile, name, members); - this.staticSymbolCache.set(key, result); + this.typeCache.set(key, result); } return result; } @@ -220,20 +220,15 @@ 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]) { - staticSymbol = this.getStaticSymbol(filePath, symbolName); + return this.getStaticSymbol(filePath, symbolName); } // If no, try to find the symbol in one of the re-export location - if (!staticSymbol && metadata['exports']) { + if (metadata['exports']) { // Try and find the symbol in the list of explicitly re-exported symbols. for (const moduleExport of metadata['exports']) { if (moduleExport.export) { @@ -249,43 +244,43 @@ export class StaticReflector implements ReflectorReader { if (typeof exportSymbol !== 'string') { symName = exportSymbol.name; } - staticSymbol = this.resolveExportedSymbol(resolveModule(moduleExport.from), symName); + return this.resolveExportedSymbol(resolveModule(moduleExport.from), symName); } } } - 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; - } - } + // 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; } } } } - this.declarationCache.set(cacheKey, staticSymbol); - return staticSymbol; + return null; } 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. - symbol = this.getStaticSymbol(module, symbolName); - } else { - symbol = this.resolveExportedSymbol(filePath, symbolName) || - this.getStaticSymbol(filePath, symbolName); + return this.getStaticSymbol(module, 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 d65275377a..71b554eefd 100644 --- a/modules/@angular/compiler/test/aot/static_reflector_spec.ts +++ b/modules/@angular/compiler/test/aot/static_reflector_spec.ts @@ -502,21 +502,6 @@ 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 { @@ -565,7 +550,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'; @@ -573,9 +558,7 @@ class MockAotCompilerHost implements AotCompilerHost { return '/tmp/' + modulePath + '.d.ts'; } - getMetadataFor(moduleId: string): any { return this._getMetadataFor(moduleId); } - - private _getMetadataFor(moduleId: string): any { + getMetadataFor(moduleId: string): any { const data: {[key: string]: any} = { '/tmp/@angular/common/src/forms-deprecated/directives.d.ts': [{ '__symbolic': 'module',