From 93170561389f0e96238aaaba04d2c394fbb3a541 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 12 Aug 2016 17:38:29 -0700 Subject: [PATCH] fix(ngc): Revert "fix(ngc): add an option to produce TS1.9-pathMapping imports (#10602)" (#10765) This reverts commit beadf6167a24e9ee0a567f2b4800a318785e00ec. --- .../integrationtest/tsconfig.json | 3 +- modules/@angular/compiler-cli/src/codegen.ts | 4 +- .../compiler-cli/src/reflector_host.ts | 97 +++++++++---------- modules/@angular/compiler-cli/test/mocks.ts | 2 - .../compiler-cli/test/reflector_host_spec.ts | 89 ++++++----------- tools/@angular/tsc-wrapped/src/options.ts | 12 --- 6 files changed, 80 insertions(+), 127 deletions(-) diff --git a/modules/@angular/compiler-cli/integrationtest/tsconfig.json b/modules/@angular/compiler-cli/integrationtest/tsconfig.json index a73a371c62..01090a2570 100644 --- a/modules/@angular/compiler-cli/integrationtest/tsconfig.json +++ b/modules/@angular/compiler-cli/integrationtest/tsconfig.json @@ -13,6 +13,7 @@ "moduleResolution": "node", "rootDir": "", "declaration": true, - "lib": ["es6", "dom"] + "lib": ["es6", "dom"], + "baseUrl": "." } } diff --git a/modules/@angular/compiler-cli/src/codegen.ts b/modules/@angular/compiler-cli/src/codegen.ts index 399d281b75..5f79f7ea1d 100644 --- a/modules/@angular/compiler-cli/src/codegen.ts +++ b/modules/@angular/compiler-cli/src/codegen.ts @@ -18,12 +18,14 @@ import * as ts from 'typescript'; import {CompileMetadataResolver, DirectiveNormalizer, DomElementSchemaRegistry, HtmlParser, Lexer, NgModuleCompiler, Parser, StyleCompiler, TemplateParser, TypeScriptEmitter, ViewCompiler} from './compiler_private'; import {Console} from './core_private'; -import {GENERATED_FILES, ReflectorHost, ReflectorHostContext} from './reflector_host'; +import {ReflectorHost, ReflectorHostContext} from './reflector_host'; import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities'; import {StaticReflector, StaticSymbol} from './static_reflector'; const nodeFs = require('fs'); +const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/; + const PREAMBLE = `/** * This file is generated by the Angular 2 template compiler. * Do not edit. diff --git a/modules/@angular/compiler-cli/src/reflector_host.ts b/modules/@angular/compiler-cli/src/reflector_host.ts index 9e86c37ef3..ce56d96473 100644 --- a/modules/@angular/compiler-cli/src/reflector_host.ts +++ b/modules/@angular/compiler-cli/src/reflector_host.ts @@ -16,7 +16,8 @@ import {StaticReflectorHost, StaticSymbol} from './static_reflector'; const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/; const DTS = /\.d\.ts$/; -export const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/; +const NODE_MODULES = path.sep + 'node_modules' + path.sep; +const IS_GENERATED = /\.(ngfactory|css(\.shim)?)$/; export interface ReflectorHostContext { fileExists(fileName: string): boolean; @@ -57,9 +58,6 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator { private resolve(m: string, containingFile: string) { const resolved = ts.resolveModuleName(m, containingFile, this.options, this.context).resolvedModule; - if (this.options.traceResolution) { - console.log('resolve', m, containingFile, '=>', resolved); - } return resolved ? resolved.resolvedFileName : null; }; @@ -81,8 +79,8 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator { * These need to be in a form that system.js can load, so absolute file paths don't work. * * The `containingFile` is always in the `genDir`, where as the `importedFile` can be in - * `genDir`, `node_module` or `rootDir`/`rootDirs`. - * The `importedFile` is either a generated file or an existing file. + * `genDir`, `node_module` or `basePath`. The `importedFile` is either a generated file or + * existing file. * * | genDir | node_module | rootDir * --------------+----------+-------------+---------- @@ -95,59 +93,45 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator { importedFile = this.resolveAssetUrl(importedFile, containingFile); containingFile = this.resolveAssetUrl(containingFile, ''); - if (this.options.traceResolution) { - console.log( - 'getImportPath from containingFile', containingFile, 'to importedFile', importedFile); - } - // If a file does not yet exist (because we compile it later), we still need to // assume it exists it so that the `resolve` method works! if (!this.compilerHost.fileExists(importedFile)) { this.context.assumeFileExists(importedFile); } - let importModuleName = importedFile.replace(EXT, ''); - const parts = importModuleName.split(path.sep).filter(p => !!p); - let foundRelativeImport: string; - for (let index = parts.length - 1; index >= 0; index--) { - let candidate = parts.slice(index, parts.length).join(path.sep); - if (this.resolve(candidate, containingFile) === importedFile) { - return candidate; + containingFile = this.rewriteGenDirPath(containingFile); + const containingDir = path.dirname(containingFile); + // drop extension + importedFile = importedFile.replace(EXT, ''); + + var nodeModulesIndex = importedFile.indexOf(NODE_MODULES); + const importModule = nodeModulesIndex === -1 ? + null : + importedFile.substring(nodeModulesIndex + NODE_MODULES.length); + const isGeneratedFile = IS_GENERATED.test(importedFile); + + if (isGeneratedFile) { + // rewrite to genDir path + if (importModule) { + // it is generated, therefore we do a relative path to the factory + return this.dotRelative(containingDir, this.genDir + NODE_MODULES + importModule); + } else { + // assume that import is also in `genDir` + importedFile = this.rewriteGenDirPath(importedFile); + return this.dotRelative(containingDir, importedFile); } - candidate = '.' + path.sep + candidate; - if (this.resolve(candidate, containingFile) === importedFile) { - if (this.options.writeImportsForRootDirs) { - foundRelativeImport = candidate; - } else { - foundRelativeImport = this.fixupGendirRelativePath(containingFile, importedFile); + } else { + // user code import + if (importModule) { + return importModule; + } else { + if (!this.isGenDirChildOfRootDir) { + // assume that they are on top of each other. + importedFile = importedFile.replace(this.basePath, this.genDir); } + return this.dotRelative(containingDir, importedFile); } } - - if (foundRelativeImport) return foundRelativeImport; - - // Try a relative import - let candidate = path.relative(path.dirname(containingFile), importModuleName); - if (this.resolve(candidate, containingFile) === importedFile) { - return this.fixupGendirRelativePath(containingFile, importedFile); - } - - throw new Error( - `Unable to find any resolvable import for ${importedFile} relative to ${containingFile}`); - } - - private fixupGendirRelativePath(containingFile: string, importedFile: string) { - let importModuleName = importedFile.replace(EXT, ''); - - if (!this.options.writeImportsForRootDirs && this.isGenDirChildOfRootDir) { - if (GENERATED_FILES.test(importedFile)) { - importModuleName = importModuleName.replace(this.basePath, this.genDir); - } - if (GENERATED_FILES.test(containingFile)) { - containingFile = containingFile.replace(this.basePath, this.genDir); - } - } - return this.dotRelative(path.dirname(containingFile), importModuleName); } private dotRelative(from: string, to: string): string { @@ -155,6 +139,21 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator { return rPath.startsWith('.') ? rPath : './' + rPath; } + /** + * Moves the path into `genDir` folder while preserving the `node_modules` directory. + */ + private rewriteGenDirPath(filepath: string) { + var nodeModulesIndex = filepath.indexOf(NODE_MODULES); + if (nodeModulesIndex !== -1) { + // If we are in node_modulse, transplant them into `genDir`. + return path.join(this.genDir, filepath.substring(nodeModulesIndex)); + } else { + // pretend that containing file is on top of the `genDir` to normalize the paths. + // we apply the `genDir` => `rootDir` delta through `rootDirPrefix` later. + return filepath.replace(this.basePath, this.genDir); + } + } + findDeclaration( module: string, symbolName: string, containingFile: string, containingModule?: string): StaticSymbol { diff --git a/modules/@angular/compiler-cli/test/mocks.ts b/modules/@angular/compiler-cli/test/mocks.ts index 34c519bcbc..49f9d87630 100644 --- a/modules/@angular/compiler-cli/test/mocks.ts +++ b/modules/@angular/compiler-cli/test/mocks.ts @@ -17,8 +17,6 @@ export interface Directory { [name: string]: Entry; } export class MockContext implements ReflectorHostContext { constructor(public currentDirectory: string, private files: Entry) {} - trace(s: string) { console.log(s); } - fileExists(fileName: string): boolean { return typeof this.getEntry(fileName) === 'string'; } directoryExists(path: string): boolean { return typeof this.getEntry(path) === 'object'; } diff --git a/modules/@angular/compiler-cli/test/reflector_host_spec.ts b/modules/@angular/compiler-cli/test/reflector_host_spec.ts index 2f9186e627..267870aceb 100644 --- a/modules/@angular/compiler-cli/test/reflector_host_spec.ts +++ b/modules/@angular/compiler-cli/test/reflector_host_spec.ts @@ -20,8 +20,6 @@ describe('reflector_host', () => { var reflectorNestedGenDir: ReflectorHost; var reflectorSiblingGenDir: ReflectorHost; - const DEBUG = false; - beforeEach(() => { context = new MockContext('/tmp/src', clone(FILES)); host = new MockCompilerHost(context); @@ -37,9 +35,8 @@ describe('reflector_host', () => { } reflectorNestedGenDir = new ReflectorHost( program, host, { - // Intentional trailing slash, check for regression of #10533 - genDir: '/tmp/src/gen/', - basePath: '/tmp/src', + genDir: '/tmp/project/src/gen/', + basePath: '/tmp/project/src', skipMetadataEmit: false, skipTemplateCodegen: false, trace: false @@ -47,97 +44,77 @@ describe('reflector_host', () => { context); reflectorSiblingGenDir = new ReflectorHost( program, host, { - genDir: '/tmp/gen', - // Intentional trailing slash, check for regression of #10533 - basePath: '/tmp/src/', + genDir: '/tmp/project/gen', + basePath: '/tmp/project/src/', skipMetadataEmit: false, skipTemplateCodegen: false, trace: false }, context); - }); - describe('path mapping', () => { - it('should use rootDirs for calculating relative imports', () => { - const reflectorHost = new ReflectorHost( - program, host, { - genDir: '/tmp/gen', - basePath: '/tmp/src/', - skipMetadataEmit: false, - skipTemplateCodegen: false, - trace: false, - traceResolution: DEBUG, - rootDirs: ['/tmp/src/', '/tmp/genfiles/'], - writeImportsForRootDirs: true, - }, - context); - expect(reflectorHost.getImportPath( - '/tmp/src/pathmapping/bootstrap.ts', '/tmp/genfiles/pathmapping/comp.d.ts')) - .toEqual('./comp'); - }); - }); - - describe('nested genDir', () => { + describe('nestedGenDir', () => { it('should import node_module from factory', () => { expect(reflectorNestedGenDir.getImportPath( - '/tmp/src/gen/my.ngfactory.ts', '/tmp/src/node_modules/@angular/core.d.ts')) + '/tmp/project/src/gen/my.ngfactory.ts', + '/tmp/project/node_modules/@angular/core.d.ts')) .toEqual('@angular/core'); }); it('should import factory from factory', () => { expect(reflectorNestedGenDir.getImportPath( - '/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ngfactory.ts')) + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ngfactory.ts')) .toEqual('./my.other.ngfactory'); expect(reflectorNestedGenDir.getImportPath( - '/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.css.ts')) + '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.css.ts')) .toEqual('../my.other.css'); expect(reflectorNestedGenDir.getImportPath( - '/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.css.shim.ts')) + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.css.shim.ts')) .toEqual('./a/my.other.css.shim'); }); it('should import application from factory', () => { - expect( - reflectorNestedGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ts')) + expect(reflectorNestedGenDir.getImportPath( + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ts')) .toEqual('../my.other'); - expect( - reflectorNestedGenDir.getImportPath('/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.ts')) + expect(reflectorNestedGenDir.getImportPath( + '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.ts')) .toEqual('../../my.other'); - expect( - reflectorNestedGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.ts')) + expect(reflectorNestedGenDir.getImportPath( + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.ts')) .toEqual('../a/my.other'); }); }); - describe('sibling genDir', () => { + describe('nestedGenDir', () => { it('should import node_module from factory', () => { expect(reflectorSiblingGenDir.getImportPath( - '/tmp/src/gen/my.ngfactory.ts', '/tmp/src/node_modules/@angular/core.d.ts')) + '/tmp/project/src/gen/my.ngfactory.ts', + '/tmp/project/node_modules/@angular/core.d.ts')) .toEqual('@angular/core'); }); it('should import factory from factory', () => { expect(reflectorSiblingGenDir.getImportPath( - '/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ngfactory.ts')) + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ngfactory.ts')) .toEqual('./my.other.ngfactory'); expect(reflectorSiblingGenDir.getImportPath( - '/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.css.ts')) + '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.css.ts')) .toEqual('../my.other.css'); expect(reflectorSiblingGenDir.getImportPath( - '/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.css.shim.ts')) + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.css.shim.ts')) .toEqual('./a/my.other.css.shim'); }); it('should import application from factory', () => { - expect( - reflectorSiblingGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ts')) + expect(reflectorSiblingGenDir.getImportPath( + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ts')) .toEqual('./my.other'); expect(reflectorSiblingGenDir.getImportPath( - '/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.ts')) + '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.ts')) .toEqual('../my.other'); expect(reflectorSiblingGenDir.getImportPath( - '/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.ts')) + '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.ts')) .toEqual('./a/my.other'); }); }); @@ -154,7 +131,7 @@ describe('reflector_host', () => { it('should be able to produce an import from main @angular/core', () => { expect(reflectorNestedGenDir.getImportPath( - '/tmp/src/main.ts', '/tmp/src/node_modules/@angular/core.d.ts')) + '/tmp/project/src/main.ts', '/tmp/project/node_modules/@angular/core.d.ts')) .toEqual('@angular/core'); }); @@ -322,11 +299,6 @@ const FILES: Entry = { }) } }, - 'pathmapping': {'bootstrap.ts': `import {a} from './comp.d.ts';`}, - 'a': { - 'my.other.css.shim.ts': dummyModule, - }, - 'my.other.ts': dummyModule, 'node_modules': { '@angular': { 'core.d.ts': dummyModule, @@ -338,13 +310,6 @@ const FILES: Entry = { 'empty.metadata.json': '[]', } } - }, - 'genfiles': { - 'pathmapping': { - 'comp.d.ts': ` - export declare let a: string; - ` - } } } }; diff --git a/tools/@angular/tsc-wrapped/src/options.ts b/tools/@angular/tsc-wrapped/src/options.ts index 86e1392ad0..9275b5a68d 100644 --- a/tools/@angular/tsc-wrapped/src/options.ts +++ b/tools/@angular/tsc-wrapped/src/options.ts @@ -18,18 +18,6 @@ interface Options extends ts.CompilerOptions { // Whether to embed debug information in the compiled templates debug?: boolean; - - // Starting with TypeScript 1.9, the 'rootDirs' option can be used - // to allow multiple source directories to have relative imports - // between them. - // This option causes generated code to use imports relative to the - // current directory, and requires you configure the 'rootDirs' to - // include both the genDir and rootDir. - // However, due to https://github.com/Microsoft/TypeScript/issues/8245 - // note that using this option does not lay out into a flat directory - // with application and generated sources side-by-side, so you must - // teach your module loader how to resolve such imports as well. - writeImportsForRootDirs?: boolean; } export default Options;