From 613dc1122af9db877cc66f3e51ac0f18cb2ec348 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Thu, 5 Dec 2019 17:54:36 -0800 Subject: [PATCH] fix(language-service): Simplify resolution logic in banner (#34262) Due to a bug in the existing banner, `typescript` module was require-d instead of reusing the module passed in from tsserver. This bug is caused by some source files in language-service that imports `typescript` instead of `typescript/lib/tsserverlibrary`. This is not an unsupported use case, it's just that when typescript is resolved in the banner we have to be very careful about which modules to "require". The convoluted logic in the banner makes it very hard to detect anomalies. This commit cleans it up and removes a lot of unneeded code. This commit also removes `ts` import in typescript_host.ts and use `tss` instead to make it less confusing. PR Close #34262 --- packages/language-service/bundles/BUILD.bazel | 9 +++- packages/language-service/bundles/banner.js | 28 ++++++++++++ .../language-service/bundles/banner.js.txt | 23 ---------- .../language-service/src/typescript_host.ts | 43 +++++++++---------- 4 files changed, 57 insertions(+), 46 deletions(-) create mode 100644 packages/language-service/bundles/banner.js delete mode 100644 packages/language-service/bundles/banner.js.txt diff --git a/packages/language-service/bundles/BUILD.bazel b/packages/language-service/bundles/BUILD.bazel index cfb20da7b5..027480e3c1 100644 --- a/packages/language-service/bundles/BUILD.bazel +++ b/packages/language-service/bundles/BUILD.bazel @@ -9,7 +9,7 @@ ls_rollup_bundle( "typescript": "ts", "typescript/lib/tsserverlibrary": "tss", }, - license_banner = "banner.js.txt", + license_banner = ":banner", visibility = ["//packages/language-service:__pkg__"], deps = [ "//packages/language-service", @@ -17,3 +17,10 @@ ls_rollup_bundle( "@npm//tslib", ], ) + +genrule( + name = "banner", + srcs = ["banner.js"], + outs = ["banner.txt"], + cmd = "cp $< $@", +) diff --git a/packages/language-service/bundles/banner.js b/packages/language-service/bundles/banner.js new file mode 100644 index 0000000000..28ed36515f --- /dev/null +++ b/packages/language-service/bundles/banner.js @@ -0,0 +1,28 @@ +/** + * @license Angular v0.0.0-PLACEHOLDER + * Copyright Google Inc. All Rights Reserved. + * License: MIT + */ + +let $deferred; +function define(modules, callback) { + $deferred = {modules, callback}; +} +module.exports = function(provided) { + const ts = provided['typescript']; + if (!ts) { + throw new Error('Caller does not provide typescript module'); + } + const results = {}; + const resolvedModules = $deferred.modules.map(m => { + if (m === 'exports') { + return results; + } + if (m === 'typescript' || m === 'typescript/lib/tsserverlibrary') { + return ts; + } + return require(m); + }); + $deferred.callback(...resolvedModules); + return results; +}; diff --git a/packages/language-service/bundles/banner.js.txt b/packages/language-service/bundles/banner.js.txt deleted file mode 100644 index f4d2e9579d..0000000000 --- a/packages/language-service/bundles/banner.js.txt +++ /dev/null @@ -1,23 +0,0 @@ -/** - * @license Angular v0.0.0-PLACEHOLDER - * (c) 2010-2019 Google LLC. https://angular.io/ - * License: MIT - */ - -var $reflect = {defineMetadata: function() {}, getOwnMetadata: function() {}}; -var Reflect = (typeof global !== 'undefined' ? global : {})['Reflect'] || {}; -Object.keys($reflect).forEach(function(key) { Reflect[key] = Reflect[key] || $reflect[key]; }); -var $deferred, $resolved, $provided; -function $getModule(name) { - if (name === 'typescript/lib/tsserverlibrary') return $provided['typescript'] || require(name); - return $provided[name] || require(name); -} -function define(modules, cb) { $deferred = { modules: modules, cb: cb }; } -module.exports = function(provided) { - if ($resolved) return $resolved; - var result = {}; - $provided = Object.assign({'reflect-metadata': $reflect}, provided || {}, { exports: result }); - $deferred.cb.apply(this, $deferred.modules.map($getModule)); - $resolved = result; - return result; -} diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 00698bb098..34cd7e94d7 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -8,7 +8,6 @@ import {AotSummaryResolver, CompileDirectiveSummary, 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 {SchemaMetadata, ViewEncapsulation, ɵConsole as Console} from '@angular/core'; -import * as ts from 'typescript'; import * as tss from 'typescript/lib/tsserverlibrary'; import {AstResult} from './common'; @@ -23,7 +22,7 @@ import {findTightestNode, getDirectiveClassLike} from './utils'; * Create a `LanguageServiceHost` */ export function createLanguageServiceFromTypescript( - host: ts.LanguageServiceHost, service: ts.LanguageService): LanguageService { + host: tss.LanguageServiceHost, service: tss.LanguageService): LanguageService { const ngHost = new TypeScriptServiceHost(host, service); const ngServer = createLanguageService(ngHost); return ngServer; @@ -64,7 +63,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { private readonly collectedErrors = new Map(); private readonly fileVersions = new Map(); - private lastProgram: ts.Program|undefined = undefined; + private lastProgram: tss.Program|undefined = undefined; private analyzedModules: NgAnalyzedModules = { files: [], ngModuleByPipeOrDirective: new Map(), @@ -72,7 +71,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { }; constructor( - readonly tsLsHost: ts.LanguageServiceHost, private readonly tsLS: ts.LanguageService) { + readonly tsLsHost: tss.LanguageServiceHost, private readonly tsLS: tss.LanguageService) { this.summaryResolver = new AotSummaryResolver( { loadSummary(filePath: string) { return null; }, @@ -249,17 +248,17 @@ export class TypeScriptServiceHost implements LanguageServiceHost { const results: TemplateSource[] = []; if (fileName.endsWith('.ts')) { // Find every template string in the file - const visit = (child: ts.Node) => { + const visit = (child: tss.Node) => { const template = this.getInternalTemplate(child); if (template) { results.push(template); } else { - ts.forEachChild(child, visit); + tss.forEachChild(child, visit); } }; const sourceFile = this.getSourceFile(fileName); if (sourceFile) { - ts.forEachChild(sourceFile, visit); + tss.forEachChild(sourceFile, visit); } } else { const template = this.getExternalTemplate(fileName); @@ -286,7 +285,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { return []; } const results: Declaration[] = []; - const visit = (child: ts.Node) => { + const visit = (child: tss.Node) => { const candidate = getDirectiveClassLike(child); if (candidate) { const {decoratorId, classDecl} = candidate; @@ -311,19 +310,19 @@ export class TypeScriptServiceHost implements LanguageServiceHost { child.forEachChild(visit); } }; - ts.forEachChild(sourceFile, visit); + tss.forEachChild(sourceFile, visit); return results; } - getSourceFile(fileName: string): ts.SourceFile|undefined { + getSourceFile(fileName: string): tss.SourceFile|undefined { if (!fileName.endsWith('.ts')) { throw new Error(`Non-TS source file requested: ${fileName}`); } return this.program.getSourceFile(fileName); } - get program(): ts.Program { + get program(): tss.Program { const program = this.tsLS.getProgram(); if (!program) { // Program is very very unlikely to be undefined. @@ -345,8 +344,8 @@ export class TypeScriptServiceHost implements LanguageServiceHost { * * @param node Potential template node */ - private getInternalTemplate(node: ts.Node): TemplateSource|undefined { - if (!ts.isStringLiteralLike(node)) { + private getInternalTemplate(node: tss.Node): TemplateSource|undefined { + if (!tss.isStringLiteralLike(node)) { return; } const tmplAsgn = getPropertyAssignmentFromValue(node); @@ -386,7 +385,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { // TODO: This only considers top-level class declarations in a source file. // This would not find a class declaration in a namespace, for example. const classDecl = sourceFile.forEachChild((child) => { - if (ts.isClassDeclaration(child) && child.name && child.name.text === classSymbol.name) { + if (tss.isClassDeclaration(child) && child.name && child.name.text === classSymbol.name) { return child; } }); @@ -407,7 +406,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } } - private getCollectedErrors(defaultSpan: Span, sourceFile: ts.SourceFile): DeclarationError[] { + private getCollectedErrors(defaultSpan: Span, sourceFile: tss.SourceFile): DeclarationError[] { const errors = this.collectedErrors.get(sourceFile.fileName); if (!errors) { return []; @@ -580,21 +579,21 @@ function findSuitableDefaultModule(modules: NgAnalyzedModules): CompileNgModuleM return result; } -function spanOf(node: ts.Node): Span { +function spanOf(node: tss.Node): Span { return {start: node.getStart(), end: node.getEnd()}; } -function spanAt(sourceFile: ts.SourceFile, line: number, column: number): Span|undefined { +function spanAt(sourceFile: tss.SourceFile, line: number, column: number): Span|undefined { if (line != null && column != null) { - const position = ts.getPositionOfLineAndCharacter(sourceFile, line, column); - const findChild = function findChild(node: ts.Node): ts.Node | undefined { - if (node.kind > ts.SyntaxKind.LastToken && node.pos <= position && node.end > position) { - const betterNode = ts.forEachChild(node, findChild); + const position = tss.getPositionOfLineAndCharacter(sourceFile, line, column); + const findChild = function findChild(node: tss.Node): tss.Node | undefined { + if (node.kind > tss.SyntaxKind.LastToken && node.pos <= position && node.end > position) { + const betterNode = tss.forEachChild(node, findChild); return betterNode || node; } }; - const node = ts.forEachChild(sourceFile, findChild); + const node = tss.forEachChild(sourceFile, findChild); if (node) { return {start: node.getStart(), end: node.getEnd()}; }