From 159788685a2fb3f003a17a3a437407baaa0261c3 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 30 Nov 2018 10:37:06 -0800 Subject: [PATCH] fix(ivy): resolve resources using TS module resolution semantics (#27357) Previously ngtsc assumed resource files (templateUrl, styleUrls) would be physically present in the file system relative to the .ts file which referenced them. However, ngc previously resolved such references in the context of ts.CompilerOptions.rootDirs. Material depends on this functionality in its build. This commit introduces resolution of resources by leveraging the TypeScript module resolver, ts.resolveModuleName(). This resolver is used in a way which will never succeed, but on failure will return a list of locations checked. This list is then filtered to obtain the correct potential locations of the resource. PR Close #27357 --- .../ngcc/src/analysis/decoration_analyzer.ts | 6 +- .../src/ngtsc/annotations/src/api.ts | 4 +- .../src/ngtsc/annotations/src/component.ts | 13 ++- packages/compiler-cli/src/ngtsc/program.ts | 8 +- .../compiler-cli/src/ngtsc/resource_loader.ts | 98 +++++++++++++++---- packages/compiler-cli/test/ngtsc/env.ts | 16 ++- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 19 ++++ 7 files changed, 128 insertions(+), 36 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts index 4be8a9db6c..d9749b1cb1 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {ConstantPool} from '@angular/compiler'; +import * as path from 'canonical-path'; import * as fs from 'fs'; import * as ts from 'typescript'; @@ -46,7 +47,10 @@ export interface MatchingHandler { * `ResourceLoader` which directly uses the filesystem to resolve resources synchronously. */ export class FileResourceLoader implements ResourceLoader { - load(url: string): string { return fs.readFileSync(url, 'utf8'); } + load(url: string, containingFile: string): string { + url = path.resolve(path.dirname(containingFile), url); + return fs.readFileSync(url, 'utf8'); + } } /** diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/api.ts b/packages/compiler-cli/src/ngtsc/annotations/src/api.ts index 32b4c3ff00..10b156d031 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/api.ts @@ -7,6 +7,6 @@ */ export interface ResourceLoader { - preload?(url: string): Promise|undefined; - load(url: string): string; + preload?(url: string, containingFile: string): Promise|undefined; + load(url: string, containingFile: string): string; } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index b32786d8bc..9010b3c757 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -58,6 +58,7 @@ export class ComponentDecoratorHandler implements const meta = this._resolveLiteral(decorator); const component = reflectObjectLiteral(meta); const promises: Promise[] = []; + const containingFile = node.getSourceFile().fileName; if (this.resourceLoader.preload !== undefined && component.has('templateUrl')) { const templateUrlExpr = component.get('templateUrl') !; @@ -66,8 +67,7 @@ export class ComponentDecoratorHandler implements throw new FatalDiagnosticError( ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); } - const url = path.posix.resolve(path.dirname(node.getSourceFile().fileName), templateUrl); - const promise = this.resourceLoader.preload(url); + const promise = this.resourceLoader.preload(templateUrl, containingFile); if (promise !== undefined) { promises.push(promise); } @@ -76,8 +76,7 @@ export class ComponentDecoratorHandler implements const styleUrls = this._extractStyleUrls(component); if (this.resourceLoader.preload !== undefined && styleUrls !== null) { for (const styleUrl of styleUrls) { - const url = path.posix.resolve(path.dirname(node.getSourceFile().fileName), styleUrl); - const promise = this.resourceLoader.preload(url); + const promise = this.resourceLoader.preload(styleUrl, containingFile); if (promise !== undefined) { promises.push(promise); } @@ -91,6 +90,7 @@ export class ComponentDecoratorHandler implements } analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput { + const containingFile = node.getSourceFile().fileName; const meta = this._resolveLiteral(decorator); this.literalCache.delete(decorator); @@ -117,8 +117,7 @@ export class ComponentDecoratorHandler implements throw new FatalDiagnosticError( ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); } - const url = path.posix.resolve(path.dirname(node.getSourceFile().fileName), templateUrl); - templateStr = this.resourceLoader.load(url); + templateStr = this.resourceLoader.load(templateUrl, containingFile); } else if (component.has('template')) { const templateExpr = component.get('template') !; const resolvedTemplate = staticallyResolve(templateExpr, this.reflector, this.checker); @@ -210,7 +209,7 @@ export class ComponentDecoratorHandler implements if (styles === null) { styles = []; } - styles.push(...styleUrls.map(styleUrl => this.resourceLoader.load(styleUrl))); + styles.push(...styleUrls.map(styleUrl => this.resourceLoader.load(styleUrl, containingFile))); } let encapsulation: number = 0; diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 82f884b009..62c4e0309d 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -48,9 +48,11 @@ export class NgtscProgram implements api.Program { this.rootDirs.push(host.getCurrentDirectory()); } this.closureCompilerEnabled = !!options.annotateForClosureCompiler; - this.resourceLoader = host.readResource !== undefined ? - new HostResourceLoader(host.readResource.bind(host)) : - new FileResourceLoader(); + this.resourceLoader = + host.readResource !== undefined && host.resourceNameToFileName !== undefined ? + new HostResourceLoader( + host.resourceNameToFileName.bind(host), host.readResource.bind(host)) : + new FileResourceLoader(host, this.options); const shouldGenerateShims = options.allowEmptyCodegenFiles || false; this.host = host; let rootFiles = [...rootNames]; diff --git a/packages/compiler-cli/src/ngtsc/resource_loader.ts b/packages/compiler-cli/src/ngtsc/resource_loader.ts index 3ab9118f7a..5986795d97 100644 --- a/packages/compiler-cli/src/ngtsc/resource_loader.ts +++ b/packages/compiler-cli/src/ngtsc/resource_loader.ts @@ -7,6 +7,7 @@ */ import * as fs from 'fs'; +import * as ts from 'typescript'; import {ResourceLoader} from './annotations'; @@ -17,46 +18,107 @@ export class HostResourceLoader implements ResourceLoader { private cache = new Map(); private fetching = new Map>(); - constructor(private host: (url: string) => string | Promise) {} + constructor( + private resolver: (file: string, basePath: string) => string | null, + private loader: (url: string) => string | Promise) {} - preload(url: string): Promise|undefined { - if (this.cache.has(url)) { + preload(file: string, containingFile: string): Promise|undefined { + const resolved = this.resolver(file, containingFile); + if (resolved === null) { return undefined; - } else if (this.fetching.has(url)) { - return this.fetching.get(url); } - const result = this.host(url); + if (this.cache.has(resolved)) { + return undefined; + } else if (this.fetching.has(resolved)) { + return this.fetching.get(resolved); + } + + const result = this.loader(resolved); if (typeof result === 'string') { - this.cache.set(url, result); + this.cache.set(resolved, result); return undefined; } else { const fetchCompletion = result.then(str => { - this.fetching.delete(url); - this.cache.set(url, str); + this.fetching.delete(resolved); + this.cache.set(resolved, str); }); - this.fetching.set(url, fetchCompletion); + this.fetching.set(resolved, fetchCompletion); return fetchCompletion; } } - load(url: string): string { - if (this.cache.has(url)) { - return this.cache.get(url) !; + load(file: string, containingFile: string): string { + const resolved = this.resolver(file, containingFile); + if (resolved === null) { + throw new Error( + `HostResourceLoader: could not resolve ${file} in context of ${containingFile})`); } - const result = this.host(url); - if (typeof result !== 'string') { - throw new Error(`HostResourceLoader: host(${url}) returned a Promise`); + if (this.cache.has(resolved)) { + return this.cache.get(resolved) !; } - this.cache.set(url, result); + + const result = this.loader(resolved); + if (typeof result !== 'string') { + throw new Error(`HostResourceLoader: loader(${resolved}) returned a Promise`); + } + this.cache.set(resolved, result); return result; } } + + +// `failedLookupLocations` is in the name of the type ts.ResolvedModuleWithFailedLookupLocations +// but is marked @internal in TypeScript. See https://github.com/Microsoft/TypeScript/issues/28770. +type ResolvedModuleWithFailedLookupLocations = + ts.ResolvedModuleWithFailedLookupLocations & {failedLookupLocations: ReadonlyArray}; + /** * `ResourceLoader` which directly uses the filesystem to resolve resources synchronously. */ export class FileResourceLoader implements ResourceLoader { - load(url: string): string { return fs.readFileSync(url, 'utf8'); } + constructor(private host: ts.CompilerHost, private options: ts.CompilerOptions) {} + + load(file: string, containingFile: string): string { + // Attempt to resolve `file` in the context of `containingFile`, while respecting the rootDirs + // option from the tsconfig. First, normalize the file name. + + // Strip a leading '/' if one is present. + if (file.startsWith('/')) { + file = file.substr(1); + } + // Turn absolute paths into relative paths. + if (!file.startsWith('.')) { + file = `./${file}`; + } + + // TypeScript provides utilities to resolve module names, but not resource files (which aren't + // a part of the ts.Program). However, TypeScript's module resolution can be used creatively + // to locate where resource files should be expected to exist. Since module resolution returns + // a list of file names that were considered, the loader can enumerate the possible locations + // for the file by setting up a module resolution for it that will fail. + file += '.$ngresource$'; + + // clang-format off + const failedLookup = ts.resolveModuleName(file, containingFile, this.options, this.host) as ResolvedModuleWithFailedLookupLocations; + // clang-format on + if (failedLookup.failedLookupLocations === undefined) { + throw new Error( + `Internal error: expected to find failedLookupLocations during resolution of resource '${file}' in context of ${containingFile}`); + } + + const candidateLocations = + failedLookup.failedLookupLocations + .filter(candidate => candidate.endsWith('.$ngresource$.ts')) + .map(candidate => candidate.replace(/\.\$ngresource\$\.ts$/, '')); + + for (const candidate of candidateLocations) { + if (fs.existsSync(candidate)) { + return fs.readFileSync(candidate, 'utf8'); + } + } + throw new Error(`Could not find resource ${file} in context of ${containingFile}`); + } } diff --git a/packages/compiler-cli/test/ngtsc/env.ts b/packages/compiler-cli/test/ngtsc/env.ts index 5106de7999..54ecd951f8 100644 --- a/packages/compiler-cli/test/ngtsc/env.ts +++ b/packages/compiler-cli/test/ngtsc/env.ts @@ -96,11 +96,17 @@ export class NgtscTestEnvironment { write(fileName: string, content: string) { this.support.write(fileName, content); } - tsconfig(extraOpts: {[key: string]: string | boolean} = {}): void { - const opts = JSON.stringify({...extraOpts, 'enableIvy': 'ngtsc'}); - const tsconfig: string = - `{"extends": "./tsconfig-base.json", "angularCompilerOptions": ${opts}}`; - this.write('tsconfig.json', tsconfig); + tsconfig(extraOpts: {[key: string]: string | boolean} = {}, extraRootDirs?: string[]): void { + const tsconfig: {[key: string]: any} = { + extends: './tsconfig-base.json', + angularCompilerOptions: {...extraOpts, enableIvy: 'ngtsc'}, + }; + if (extraRootDirs !== undefined) { + tsconfig.compilerOptions = { + rootDirs: ['.', ...extraRootDirs], + }; + } + this.write('tsconfig.json', JSON.stringify(tsconfig, null, 2)); } /** diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 29fc307877..d2601ee479 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -89,6 +89,25 @@ describe('ngtsc behavioral tests', () => { expect(jsContents).toContain('Hello World'); }); + it('should compile Components with a templateUrl in a different rootDir', () => { + env.tsconfig({}, ['./extraRootDir']); + env.write('extraRootDir/test.html', '

Hello World

'); + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'test-cmp', + templateUrl: 'test.html', + }) + export class TestCmp {} + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('Hello World'); + }); + it('should compile components with styleUrls', () => { env.tsconfig(); env.write('test.ts', `