From a91ab15525b14853fce97ca2cb9c584ce04a9649 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Mon, 5 Aug 2019 17:13:20 -0700 Subject: [PATCH] fix(language-service): Remove 'context' used for module resolution (#32015) The language service relies on a "context" file that is used as the canonical "containing file" when performing module resolution. This file is unnecessary since the language service host's current directory always default to the location of tsconfig.json for the project, which would give the correct result. This refactoring allows us to simplify the "typescript host" and also removes the need for custom logic to find tsconfig.json. PR Close #32015 --- .../expression_diagnostics_spec.ts | 9 +--- .../diagnostics/typescript_symbols_spec.ts | 9 +--- .../language-service/src/reflector_host.ts | 23 +++++--- .../language-service/src/typescript_host.ts | 54 ++----------------- .../test/reflector_host_spec.ts | 6 +-- 5 files changed, 26 insertions(+), 75 deletions(-) diff --git a/packages/compiler-cli/test/diagnostics/expression_diagnostics_spec.ts b/packages/compiler-cli/test/diagnostics/expression_diagnostics_spec.ts index 7a070fd728..371c913135 100644 --- a/packages/compiler-cli/test/diagnostics/expression_diagnostics_spec.ts +++ b/packages/compiler-cli/test/diagnostics/expression_diagnostics_spec.ts @@ -7,12 +7,10 @@ */ import {StaticSymbol} from '@angular/compiler'; -import {CompilerHost} from '@angular/compiler-cli'; import {ReflectorHost} from '@angular/language-service/src/reflector_host'; import * as ts from 'typescript'; -import {getExpressionDiagnostics, getTemplateExpressionDiagnostics} from '../../src/diagnostics/expression_diagnostics'; -import {CompilerOptions} from '../../src/transformers/api'; +import {getTemplateExpressionDiagnostics} from '../../src/diagnostics/expression_diagnostics'; import {Directory} from '../mocks'; import {DiagnosticContext, MockLanguageServiceHost, getDiagnosticTemplateInfo} from './mocks'; @@ -31,10 +29,7 @@ describe('expression diagnostics', () => { service = ts.createLanguageService(host, registry); const program = service.getProgram() !; const checker = program.getTypeChecker(); - const options: CompilerOptions = Object.create(host.getCompilationSettings()); - options.genDir = '/dist'; - options.basePath = '/src'; - const symbolResolverHost = new ReflectorHost(() => program !, host, options); + const symbolResolverHost = new ReflectorHost(() => program !, host); context = new DiagnosticContext(service, program !, checker, symbolResolverHost); type = context.getStaticSymbol('app/app.component.ts', 'AppComponent'); }); diff --git a/packages/compiler-cli/test/diagnostics/typescript_symbols_spec.ts b/packages/compiler-cli/test/diagnostics/typescript_symbols_spec.ts index 58123d97c7..5157470dfd 100644 --- a/packages/compiler-cli/test/diagnostics/typescript_symbols_spec.ts +++ b/packages/compiler-cli/test/diagnostics/typescript_symbols_spec.ts @@ -6,15 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ -import {StaticSymbol} from '@angular/compiler'; -import {CompilerHost} from '@angular/compiler-cli'; -import {EmittingCompilerHost, MockAotCompilerHost, MockCompilerHost, MockData, MockDirectory, MockMetadataBundlerHost, arrayToMockDir, arrayToMockMap, isSource, settings, setup, toMockFileArray} from '@angular/compiler/test/aot/test_util'; import {ReflectorHost} from '@angular/language-service/src/reflector_host'; import * as ts from 'typescript'; import {BuiltinType, Symbol, SymbolQuery, SymbolTable} from '../../src/diagnostics/symbols'; import {getSymbolQuery, toSymbolTableFactory} from '../../src/diagnostics/typescript_symbols'; -import {CompilerOptions} from '../../src/transformers/api'; import {Directory} from '../mocks'; import {DiagnosticContext, MockLanguageServiceHost} from './mocks'; @@ -42,10 +38,7 @@ describe('symbol query', () => { program = service.getProgram() !; checker = program.getTypeChecker(); sourceFile = program.getSourceFile('/quickstart/app/app.component.ts') !; - const options: CompilerOptions = Object.create(host.getCompilationSettings()); - options.genDir = '/dist'; - options.basePath = '/quickstart'; - const symbolResolverHost = new ReflectorHost(() => program, host, options); + const symbolResolverHost = new ReflectorHost(() => program, host); context = new DiagnosticContext(service, program, checker, symbolResolverHost); query = getSymbolQuery(program, checker, sourceFile, emptyPipes); }); diff --git a/packages/language-service/src/reflector_host.ts b/packages/language-service/src/reflector_host.ts index 5fcbb57286..a94fd3e0c5 100644 --- a/packages/language-service/src/reflector_host.ts +++ b/packages/language-service/src/reflector_host.ts @@ -7,7 +7,7 @@ */ import {StaticSymbolResolverHost} from '@angular/compiler'; -import {CompilerOptions, MetadataCollector, MetadataReaderHost, createMetadataReaderCache, readMetadata} from '@angular/compiler-cli/src/language_services'; +import {MetadataCollector, MetadataReaderHost, createMetadataReaderCache, readMetadata} from '@angular/compiler-cli/src/language_services'; import * as path from 'path'; import * as ts from 'typescript'; @@ -51,9 +51,7 @@ export class ReflectorHost implements StaticSymbolResolverHost { private hostAdapter: ReflectorModuleModuleResolutionHost; private metadataReaderCache = createMetadataReaderCache(); - constructor( - getProgram: () => ts.Program, serviceHost: ts.LanguageServiceHost, - private options: CompilerOptions) { + constructor(getProgram: () => ts.Program, private readonly serviceHost: ts.LanguageServiceHost) { this.hostAdapter = new ReflectorModuleModuleResolutionHost(serviceHost, getProgram); } @@ -63,14 +61,25 @@ export class ReflectorHost implements StaticSymbolResolverHost { moduleNameToFileName(moduleName: string, containingFile?: string): string|null { if (!containingFile) { - if (moduleName.indexOf('.') === 0) { + if (moduleName.startsWith('.')) { throw new Error('Resolution of relative paths requires a containing file.'); } + // serviceHost.getCurrentDirectory() returns the directory where tsconfig.json + // is located. This is not the same as process.cwd() because the language + // service host sets the "project root path" as its current directory. + const currentDirectory = this.serviceHost.getCurrentDirectory(); + if (!currentDirectory) { + // If current directory is empty then the file must belong to an inferred + // project (no tsconfig.json), in which case it's not possible to resolve + // the module without the caller explicitly providing a containing file. + throw new Error(`Could not resolve '${moduleName}' without a containing file.`); + } // Any containing file gives the same result for absolute imports - containingFile = path.join(this.options.basePath !, 'index.ts').replace(/\\/g, '/'); + containingFile = path.join(currentDirectory, 'index.ts'); } + const compilerOptions = this.serviceHost.getCompilationSettings(); const resolved = - ts.resolveModuleName(moduleName, containingFile !, this.options, this.hostAdapter) + ts.resolveModuleName(moduleName, containingFile, compilerOptions, this.hostAdapter) .resolvedModule; return resolved ? resolved.resolvedFileName : null; } diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 1844fd99c2..656aa46a37 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -7,10 +7,8 @@ */ import {AotSummaryResolver, CompileMetadataResolver, CompileNgModuleMetadata, CompilePipeSummary, CompilerConfig, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, FormattedError, FormattedMessageChain, HtmlParser, I18NHtmlParser, JitSummaryResolver, Lexer, NgAnalyzedModules, NgModuleResolver, ParseTreeResult, Parser, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, TemplateParser, analyzeNgModules, createOfflineCompileUrlResolver, isFormattedError} from '@angular/compiler'; -import {CompilerOptions, getClassMembersFromDeclaration, getPipesTable, getSymbolQuery} from '@angular/compiler-cli/src/language_services'; +import {getClassMembersFromDeclaration, getPipesTable, getSymbolQuery} from '@angular/compiler-cli/src/language_services'; import {ViewEncapsulation, ɵConsole as Console} from '@angular/core'; -import * as fs from 'fs'; -import * as path from 'path'; import * as ts from 'typescript'; import {AstResult, TemplateInfo} from './common'; @@ -69,7 +67,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost { private _reflectorHost !: ReflectorHost; // TODO(issue/24571): remove '!'. private _checker !: ts.TypeChecker | null; - private context: string|undefined; private lastProgram: ts.Program|undefined; private modulesOutOfDate: boolean = true; // TODO(issue/24571): remove '!'. @@ -127,7 +124,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost { if (fileName.endsWith('.ts')) { const sourceFile = this.getSourceFile(fileName); if (sourceFile) { - this.context = sourceFile.fileName; const node = this.findNode(sourceFile, position); if (node) { return this.getSourceFromNode( @@ -187,7 +183,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost { let sourceFile = this.getSourceFile(fileName); if (sourceFile) { - this.context = (sourceFile as any).path || sourceFile.fileName; ts.forEachChild(sourceFile, visit); } return result.length ? result : undefined; @@ -383,40 +378,10 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } private get reflectorHost(): ReflectorHost { - let result = this._reflectorHost; - if (!result) { - if (!this.context) { - // Make up a context by finding the first script and using that as the base dir. - const scriptFileNames = this.host.getScriptFileNames(); - if (0 === scriptFileNames.length) { - throw new Error('Internal error: no script file names found'); - } - this.context = scriptFileNames[0]; - } - - // Use the file context's directory as the base directory. - // The host's getCurrentDirectory() is not reliable as it is always "" in - // tsserver. We don't need the exact base directory, just one that contains - // a source file. - const source = this.getSourceFile(this.context); - if (!source) { - throw new Error('Internal error: no context could be determined'); - } - - const tsConfigPath = findTsConfig(source.fileName); - const basePath = path.dirname(tsConfigPath || this.context); - const options: CompilerOptions = {basePath, genDir: basePath}; - const compilerOptions = this.host.getCompilationSettings(); - if (compilerOptions && compilerOptions.baseUrl) { - options.baseUrl = compilerOptions.baseUrl; - } - if (compilerOptions && compilerOptions.paths) { - options.paths = compilerOptions.paths; - } - result = this._reflectorHost = - new ReflectorHost(() => this.tsService.getProgram() !, this.host, options); + if (!this._reflectorHost) { + this._reflectorHost = new ReflectorHost(() => this.tsService.getProgram() !, this.host); } - return result; + return this._reflectorHost; } private collectError(error: any, filePath: string|null) { @@ -682,17 +647,6 @@ function findSuitableDefaultModule(modules: NgAnalyzedModules): CompileNgModuleM return result; } -function findTsConfig(fileName: string): string|undefined { - let dir = path.dirname(fileName); - while (fs.existsSync(dir)) { - const candidate = path.join(dir, 'tsconfig.json'); - if (fs.existsSync(candidate)) return candidate; - const parentDir = path.dirname(dir); - if (parentDir === dir) break; - dir = parentDir; - } -} - function spanOf(node: ts.Node): Span { return {start: node.getStart(), end: node.getEnd()}; } diff --git a/packages/language-service/test/reflector_host_spec.ts b/packages/language-service/test/reflector_host_spec.ts index 59eb3918ba..3eb158954e 100644 --- a/packages/language-service/test/reflector_host_spec.ts +++ b/packages/language-service/test/reflector_host_spec.ts @@ -20,13 +20,13 @@ describe('reflector_host_spec', () => { const originalJoin = path.join; const originalPosixJoin = path.posix.join; let mockHost = - new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'app/node_modules', { + new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'node_modules', { ...path, join: (...args: string[]) => originalJoin.apply(path, args), posix: {...path.posix, join: (...args: string[]) => originalPosixJoin.apply(path, args)} }); - const reflectorHost = new ReflectorHost(() => undefined as any, mockHost, {basePath: '\\app'}); + const reflectorHost = new ReflectorHost(() => undefined as any, mockHost); if (process.platform !== 'win32') { // If we call this in Windows it will cause a 'Maximum call stack size exceeded error' @@ -37,4 +37,4 @@ describe('reflector_host_spec', () => { const result = reflectorHost.moduleNameToFileName('@angular/core'); expect(result).not.toBeNull('could not find @angular/core using path.win32'); }); -}); \ No newline at end of file +});