Skip to content

Commit beadf61

Browse files
alexeaglealxhub
alexeagle
authored and
alxhub
committed
fix(ngc): add an option to produce TS1.9-pathMapping imports (angular#10602)
This fixes a regression in angular#10486
1 parent c8da7e9 commit beadf61

6 files changed

Lines changed: 128 additions & 81 deletions

File tree

‎modules/@angular/compiler-cli/integrationtest/tsconfig.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
"moduleResolution": "node",
1414
"rootDir": "",
1515
"declaration": true,
16-
"lib": ["es6", "dom"],
17-
"baseUrl": "."
16+
"lib": ["es6", "dom"]
1817
}
1918
}

‎modules/@angular/compiler-cli/src/codegen.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@ import * as ts from 'typescript';
1818

1919
import {CompileMetadataResolver, DirectiveNormalizer, DomElementSchemaRegistry, HtmlParser, Lexer, NgModuleCompiler, Parser, StyleCompiler, TemplateParser, TypeScriptEmitter, ViewCompiler} from './compiler_private';
2020
import {Console} from './core_private';
21-
import {ReflectorHost, ReflectorHostContext} from './reflector_host';
21+
import {GENERATED_FILES, ReflectorHost, ReflectorHostContext} from './reflector_host';
2222
import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities';
2323
import {StaticReflector, StaticSymbol} from './static_reflector';
2424

25-
const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
26-
2725
const PREAMBLE = `/**
2826
* This file is generated by the Angular 2 template compiler.
2927
* Do not edit.
@@ -69,7 +67,7 @@ export class CodeGenerator {
6967
}
7068

7169
// Write codegen in a directory structure matching the sources.
72-
private calculateEmitPath(filePath: string) {
70+
private calculateEmitPath(filePath: string): string {
7371
let root = this.options.basePath;
7472
for (let eachRootDir of this.options.rootDirs || []) {
7573
if (this.options.trace) {

‎modules/@angular/compiler-cli/src/reflector_host.ts‎

Lines changed: 49 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ import {StaticReflectorHost, StaticSymbol} from './static_reflector';
1616

1717
const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/;
1818
const DTS = /\.d\.ts$/;
19-
const NODE_MODULES = path.sep + 'node_modules' + path.sep;
20-
const IS_GENERATED = /\.(ngfactory|css(\.shim)?)$/;
19+
export const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
2120

2221
export interface ReflectorHostContext {
2322
fileExists(fileName: string): boolean;
@@ -58,6 +57,9 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
5857
private resolve(m: string, containingFile: string) {
5958
const resolved =
6059
ts.resolveModuleName(m, containingFile, this.options, this.context).resolvedModule;
60+
if (this.options.traceResolution) {
61+
console.log('resolve', m, containingFile, '=>', resolved);
62+
}
6163
return resolved ? resolved.resolvedFileName : null;
6264
};
6365

@@ -79,8 +81,8 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
7981
* These need to be in a form that system.js can load, so absolute file paths don't work.
8082
*
8183
* The `containingFile` is always in the `genDir`, where as the `importedFile` can be in
82-
* `genDir`, `node_module` or `basePath`. The `importedFile` is either a generated file or
83-
* existing file.
84+
* `genDir`, `node_module` or `rootDir`/`rootDirs`.
85+
* The `importedFile` is either a generated file or an existing file.
8486
*
8587
* | genDir | node_module | rootDir
8688
* --------------+----------+-------------+----------
@@ -93,67 +95,66 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
9395
importedFile = this.resolveAssetUrl(importedFile, containingFile);
9496
containingFile = this.resolveAssetUrl(containingFile, '');
9597

98+
if (this.options.traceResolution) {
99+
console.log(
100+
'getImportPath from containingFile', containingFile, 'to importedFile', importedFile);
101+
}
102+
96103
// If a file does not yet exist (because we compile it later), we still need to
97104
// assume it exists it so that the `resolve` method works!
98105
if (!this.compilerHost.fileExists(importedFile)) {
99106
this.context.assumeFileExists(importedFile);
100107
}
101108

102-
containingFile = this.rewriteGenDirPath(containingFile);
103-
const containingDir = path.dirname(containingFile);
104-
// drop extension
105-
importedFile = importedFile.replace(EXT, '');
106-
107-
var nodeModulesIndex = importedFile.indexOf(NODE_MODULES);
108-
const importModule = nodeModulesIndex === -1 ?
109-
null :
110-
importedFile.substring(nodeModulesIndex + NODE_MODULES.length);
111-
const isGeneratedFile = IS_GENERATED.test(importedFile);
112-
113-
if (isGeneratedFile) {
114-
// rewrite to genDir path
115-
if (importModule) {
116-
// it is generated, therefore we do a relative path to the factory
117-
return this.dotRelative(containingDir, this.genDir + NODE_MODULES + importModule);
118-
} else {
119-
// assume that import is also in `genDir`
120-
importedFile = this.rewriteGenDirPath(importedFile);
121-
return this.dotRelative(containingDir, importedFile);
109+
let importModuleName = importedFile.replace(EXT, '');
110+
const parts = importModuleName.split(path.sep).filter(p => !!p);
111+
let foundRelativeImport: string;
112+
for (let index = parts.length - 1; index >= 0; index--) {
113+
let candidate = parts.slice(index, parts.length).join(path.sep);
114+
if (this.resolve(candidate, containingFile) === importedFile) {
115+
return candidate;
122116
}
123-
} else {
124-
// user code import
125-
if (importModule) {
126-
return importModule;
127-
} else {
128-
if (!this.isGenDirChildOfRootDir) {
129-
// assume that they are on top of each other.
130-
importedFile = importedFile.replace(this.basePath, this.genDir);
117+
candidate = '.' + path.sep + candidate;
118+
if (this.resolve(candidate, containingFile) === importedFile) {
119+
if (this.options.writeImportsForRootDirs) {
120+
foundRelativeImport = candidate;
121+
} else {
122+
foundRelativeImport = this.fixupGendirRelativePath(containingFile, importedFile);
131123
}
132-
return this.dotRelative(containingDir, importedFile);
133124
}
134125
}
126+
127+
if (foundRelativeImport) return foundRelativeImport;
128+
129+
// Try a relative import
130+
let candidate = path.relative(path.dirname(containingFile), importModuleName);
131+
if (this.resolve(candidate, containingFile) === importedFile) {
132+
return this.fixupGendirRelativePath(containingFile, importedFile);
133+
}
134+
135+
throw new Error(
136+
`Unable to find any resolvable import for ${importedFile} relative to ${containingFile}`);
137+
}
138+
139+
private fixupGendirRelativePath(containingFile: string, importedFile: string) {
140+
let importModuleName = importedFile.replace(EXT, '');
141+
142+
if (!this.options.writeImportsForRootDirs && this.isGenDirChildOfRootDir) {
143+
if (GENERATED_FILES.test(importedFile)) {
144+
importModuleName = importModuleName.replace(this.basePath, this.genDir);
145+
}
146+
if (GENERATED_FILES.test(containingFile)) {
147+
containingFile = containingFile.replace(this.basePath, this.genDir);
148+
}
149+
}
150+
return this.dotRelative(path.dirname(containingFile), importModuleName);
135151
}
136152

137153
private dotRelative(from: string, to: string): string {
138154
var rPath: string = path.relative(from, to);
139155
return rPath.startsWith('.') ? rPath : './' + rPath;
140156
}
141157

142-
/**
143-
* Moves the path into `genDir` folder while preserving the `node_modules` directory.
144-
*/
145-
private rewriteGenDirPath(filepath: string) {
146-
var nodeModulesIndex = filepath.indexOf(NODE_MODULES);
147-
if (nodeModulesIndex !== -1) {
148-
// If we are in node_modulse, transplant them into `genDir`.
149-
return path.join(this.genDir, filepath.substring(nodeModulesIndex));
150-
} else {
151-
// pretend that containing file is on top of the `genDir` to normalize the paths.
152-
// we apply the `genDir` => `rootDir` delta through `rootDirPrefix` later.
153-
return filepath.replace(this.basePath, this.genDir);
154-
}
155-
}
156-
157158
findDeclaration(
158159
module: string, symbolName: string, containingFile: string,
159160
containingModule?: string): StaticSymbol {

‎modules/@angular/compiler-cli/test/mocks.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ export interface Directory { [name: string]: Entry; }
1717
export class MockContext implements ReflectorHostContext {
1818
constructor(public currentDirectory: string, private files: Entry) {}
1919

20+
trace(s: string) { console.log(s); }
21+
2022
fileExists(fileName: string): boolean { return typeof this.getEntry(fileName) === 'string'; }
2123

2224
directoryExists(path: string): boolean { return typeof this.getEntry(path) === 'object'; }

‎modules/@angular/compiler-cli/test/reflector_host_spec.ts‎

Lines changed: 62 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ describe('reflector_host', () => {
2020
var reflectorNestedGenDir: ReflectorHost;
2121
var reflectorSiblingGenDir: ReflectorHost;
2222

23+
const DEBUG = false;
24+
2325
beforeEach(() => {
2426
context = new MockContext('/tmp/src', clone(FILES));
2527
host = new MockCompilerHost(context);
@@ -35,86 +37,107 @@ describe('reflector_host', () => {
3537
}
3638
reflectorNestedGenDir = new ReflectorHost(
3739
program, host, {
38-
genDir: '/tmp/project/src/gen/',
39-
basePath: '/tmp/project/src',
40+
// Intentional trailing slash, check for regression of #10533
41+
genDir: '/tmp/src/gen/',
42+
basePath: '/tmp/src',
4043
skipMetadataEmit: false,
4144
skipTemplateCodegen: false,
4245
trace: false
4346
},
4447
context);
4548
reflectorSiblingGenDir = new ReflectorHost(
4649
program, host, {
47-
genDir: '/tmp/project/gen',
48-
basePath: '/tmp/project/src/',
50+
genDir: '/tmp/gen',
51+
// Intentional trailing slash, check for regression of #10533
52+
basePath: '/tmp/src/',
4953
skipMetadataEmit: false,
5054
skipTemplateCodegen: false,
5155
trace: false
5256
},
5357
context);
58+
59+
});
60+
61+
describe('path mapping', () => {
62+
it('should use rootDirs for calculating relative imports', () => {
63+
const reflectorHost = new ReflectorHost(
64+
program, host, {
65+
genDir: '/tmp/gen',
66+
basePath: '/tmp/src/',
67+
skipMetadataEmit: false,
68+
skipTemplateCodegen: false,
69+
trace: false,
70+
traceResolution: DEBUG,
71+
rootDirs: ['/tmp/src/', '/tmp/genfiles/'],
72+
writeImportsForRootDirs: true,
73+
},
74+
context);
75+
expect(reflectorHost.getImportPath(
76+
'/tmp/src/pathmapping/bootstrap.ts', '/tmp/genfiles/pathmapping/comp.d.ts'))
77+
.toEqual('./comp');
78+
});
5479
});
5580

56-
describe('nestedGenDir', () => {
81+
describe('nested genDir', () => {
5782
it('should import node_module from factory', () => {
5883
expect(reflectorNestedGenDir.getImportPath(
59-
'/tmp/project/src/gen/my.ngfactory.ts',
60-
'/tmp/project/node_modules/@angular/core.d.ts'))
84+
'/tmp/src/gen/my.ngfactory.ts', '/tmp/src/node_modules/@angular/core.d.ts'))
6185
.toEqual('@angular/core');
6286
});
6387

6488
it('should import factory from factory', () => {
6589
expect(reflectorNestedGenDir.getImportPath(
66-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ngfactory.ts'))
90+
'/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ngfactory.ts'))
6791
.toEqual('./my.other.ngfactory');
6892
expect(reflectorNestedGenDir.getImportPath(
69-
'/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.css.ts'))
93+
'/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.css.ts'))
7094
.toEqual('../my.other.css');
7195
expect(reflectorNestedGenDir.getImportPath(
72-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.css.shim.ts'))
96+
'/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.css.shim.ts'))
7397
.toEqual('./a/my.other.css.shim');
7498
});
7599

76100
it('should import application from factory', () => {
77-
expect(reflectorNestedGenDir.getImportPath(
78-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
101+
expect(
102+
reflectorNestedGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ts'))
79103
.toEqual('../my.other');
80-
expect(reflectorNestedGenDir.getImportPath(
81-
'/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
104+
expect(
105+
reflectorNestedGenDir.getImportPath('/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.ts'))
82106
.toEqual('../../my.other');
83-
expect(reflectorNestedGenDir.getImportPath(
84-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.ts'))
107+
expect(
108+
reflectorNestedGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.ts'))
85109
.toEqual('../a/my.other');
86110
});
87111
});
88112

89-
describe('nestedGenDir', () => {
113+
describe('sibling genDir', () => {
90114
it('should import node_module from factory', () => {
91115
expect(reflectorSiblingGenDir.getImportPath(
92-
'/tmp/project/src/gen/my.ngfactory.ts',
93-
'/tmp/project/node_modules/@angular/core.d.ts'))
116+
'/tmp/src/gen/my.ngfactory.ts', '/tmp/src/node_modules/@angular/core.d.ts'))
94117
.toEqual('@angular/core');
95118
});
96119

97120
it('should import factory from factory', () => {
98121
expect(reflectorSiblingGenDir.getImportPath(
99-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ngfactory.ts'))
122+
'/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ngfactory.ts'))
100123
.toEqual('./my.other.ngfactory');
101124
expect(reflectorSiblingGenDir.getImportPath(
102-
'/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.css.ts'))
125+
'/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.css.ts'))
103126
.toEqual('../my.other.css');
104127
expect(reflectorSiblingGenDir.getImportPath(
105-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.css.shim.ts'))
128+
'/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.css.shim.ts'))
106129
.toEqual('./a/my.other.css.shim');
107130
});
108131

109132
it('should import application from factory', () => {
110-
expect(reflectorSiblingGenDir.getImportPath(
111-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
133+
expect(
134+
reflectorSiblingGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ts'))
112135
.toEqual('./my.other');
113136
expect(reflectorSiblingGenDir.getImportPath(
114-
'/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
137+
'/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.ts'))
115138
.toEqual('../my.other');
116139
expect(reflectorSiblingGenDir.getImportPath(
117-
'/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.ts'))
140+
'/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.ts'))
118141
.toEqual('./a/my.other');
119142
});
120143
});
@@ -131,7 +154,7 @@ describe('reflector_host', () => {
131154

132155
it('should be able to produce an import from main @angular/core', () => {
133156
expect(reflectorNestedGenDir.getImportPath(
134-
'/tmp/project/src/main.ts', '/tmp/project/node_modules/@angular/core.d.ts'))
157+
'/tmp/src/main.ts', '/tmp/src/node_modules/@angular/core.d.ts'))
135158
.toEqual('@angular/core');
136159
});
137160

@@ -295,6 +318,11 @@ const FILES: Entry = {
295318
})
296319
}
297320
},
321+
'pathmapping': {'bootstrap.ts': `import {a} from './comp.d.ts';`},
322+
'a': {
323+
'my.other.css.shim.ts': dummyModule,
324+
},
325+
'my.other.ts': dummyModule,
298326
'node_modules': {
299327
'@angular': {
300328
'core.d.ts': dummyModule,
@@ -304,6 +332,13 @@ const FILES: Entry = {
304332
'unused.d.ts': dummyModule
305333
}
306334
}
335+
},
336+
'genfiles': {
337+
'pathmapping': {
338+
'comp.d.ts': `
339+
export declare let a: string;
340+
`
341+
}
307342
}
308343
}
309344
};

‎tools/@angular/tsc-wrapped/src/options.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ interface Options extends ts.CompilerOptions {
1818

1919
// Whether to embed debug information in the compiled templates
2020
debug?: boolean;
21+
22+
// Starting with TypeScript 1.9, the 'rootDirs' option can be used
23+
// to allow multiple source directories to have relative imports
24+
// between them.
25+
// This option causes generated code to use imports relative to the
26+
// current directory, and requires you configure the 'rootDirs' to
27+
// include both the genDir and rootDir.
28+
// However, due to https://github.com/Microsoft/TypeScript/issues/8245
29+
// note that using this option does not lay out into a flat directory
30+
// with application and generated sources side-by-side, so you must
31+
// teach your module loader how to resolve such imports as well.
32+
writeImportsForRootDirs?: boolean;
2133
}
2234

2335
export default Options;

0 commit comments

Comments
 (0)