fix(ivy): ngcc - prefer JavaScript source files when resolving module imports (#30017)
Packages that do not follow APF may have the declaration files in the same directory as one source format, typically ES5. This is problematic for ngcc, as it needs to create a TypeScript program with all JavaScript sources of an entry-point, whereas TypeScript's module resolution mechanism would have resolved an internal module import to the external facing .d.ts declaration file, instead of the JavaScript source file. This behavior results in the program to be analysed being incomplete. This commit introduces a custom compiler host that recognizes the above scenario and rewires the resolution of a .d.ts declaration file to its JavaScript counterpart, if applicable. Fixes #29939 PR Close #30017
This commit is contained in:
JoostK
Kara Erickson
parent
7ec8806dc5
commit
638ba4a2cf
@ -12,7 +12,7 @@ import {FileSystem} from '../file_system/file_system';
|
||||
import {PathMappings} from '../utils';
|
||||
import {BundleProgram, makeBundleProgram} from './bundle_program';
|
||||
import {EntryPointFormat, EntryPointJsonProperty} from './entry_point';
|
||||
import {NgccCompilerHost} from './ngcc_compiler_host';
|
||||
import {NgccCompilerHost, NgccSourcesCompilerHost} from './ngcc_compiler_host';
|
||||
|
||||
/**
|
||||
* A bundle of files and paths (and TS programs) that correspond to a particular
|
||||
@ -48,16 +48,17 @@ export function makeEntryPointBundle(
|
||||
noLib: true,
|
||||
rootDir: entryPointPath, ...pathMappings
|
||||
};
|
||||
const host = new NgccCompilerHost(fs, options);
|
||||
const srcHost = new NgccSourcesCompilerHost(fs, options, entryPointPath);
|
||||
const dtsHost = new NgccCompilerHost(fs, options);
|
||||
const rootDirs = [AbsoluteFsPath.from(entryPointPath)];
|
||||
|
||||
// Create the bundle programs, as necessary.
|
||||
const src = makeBundleProgram(
|
||||
fs, isCore, AbsoluteFsPath.resolve(entryPointPath, formatPath), 'r3_symbols.js', options,
|
||||
host);
|
||||
srcHost);
|
||||
const dts = transformDts ? makeBundleProgram(
|
||||
fs, isCore, AbsoluteFsPath.resolve(entryPointPath, typingsPath),
|
||||
'r3_symbols.d.ts', options, host) :
|
||||
'r3_symbols.d.ts', options, dtsHost) :
|
||||
null;
|
||||
const isFlatCore = isCore && src.r3SymbolsFile === null;
|
||||
|
||||
|
||||
@ -10,11 +10,12 @@ import * as ts from 'typescript';
|
||||
|
||||
import {AbsoluteFsPath} from '../../../src/ngtsc/path';
|
||||
import {FileSystem} from '../file_system/file_system';
|
||||
import {isRelativePath} from '../utils';
|
||||
|
||||
export class NgccCompilerHost implements ts.CompilerHost {
|
||||
private _caseSensitive = this.fs.exists(AbsoluteFsPath.fromUnchecked(__filename.toUpperCase()));
|
||||
|
||||
constructor(private fs: FileSystem, private options: ts.CompilerOptions) {}
|
||||
constructor(protected fs: FileSystem, protected options: ts.CompilerOptions) {}
|
||||
|
||||
getSourceFile(fileName: string, languageVersion: ts.ScriptTarget): ts.SourceFile|undefined {
|
||||
const text = this.readFile(fileName);
|
||||
@ -64,3 +65,41 @@ export class NgccCompilerHost implements ts.CompilerHost {
|
||||
return this.fs.readFile(AbsoluteFsPath.fromUnchecked(fileName));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Represents a compiler host that resolves a module import as a JavaScript source file if
|
||||
* available, instead of the .d.ts typings file that would have been resolved by TypeScript. This
|
||||
* is necessary for packages that have their typings in the same directory as the sources, which
|
||||
* would otherwise let TypeScript prefer the .d.ts file instead of the JavaScript source file.
|
||||
*/
|
||||
export class NgccSourcesCompilerHost extends NgccCompilerHost {
|
||||
private cache = ts.createModuleResolutionCache(
|
||||
this.getCurrentDirectory(), file => this.getCanonicalFileName(file));
|
||||
|
||||
constructor(fs: FileSystem, options: ts.CompilerOptions, protected entryPointPath: string) {
|
||||
super(fs, options);
|
||||
}
|
||||
|
||||
resolveModuleNames(
|
||||
moduleNames: string[], containingFile: string, reusedNames?: string[],
|
||||
redirectedReference?: ts.ResolvedProjectReference): Array<ts.ResolvedModule|undefined> {
|
||||
return moduleNames.map(moduleName => {
|
||||
const {resolvedModule} = ts.resolveModuleName(
|
||||
moduleName, containingFile, this.options, this, this.cache, redirectedReference);
|
||||
|
||||
// If the module request originated from a relative import in a JavaScript source file,
|
||||
// TypeScript may have resolved the module to its .d.ts declaration file if the .js source
|
||||
// file was in the same directory. This is undesirable, as we need to have the actual
|
||||
// JavaScript being present in the program. This logic recognizes this scenario and rewrites
|
||||
// the resolved .d.ts declaration file to its .js counterpart, if it exists.
|
||||
if (resolvedModule !== undefined && resolvedModule.extension === ts.Extension.Dts &&
|
||||
containingFile.endsWith('.js') && isRelativePath(moduleName)) {
|
||||
const jsFile = resolvedModule.resolvedFileName.replace(/\.d\.ts$/, '.js');
|
||||
if (this.fileExists(jsFile)) {
|
||||
return {...resolvedModule, resolvedFileName: jsFile, extension: ts.Extension.Js};
|
||||
}
|
||||
}
|
||||
return resolvedModule;
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,120 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
import {makeEntryPointBundle} from '../../src/packages/entry_point_bundle';
|
||||
import {MockFileSystem} from '../helpers/mock_file_system';
|
||||
|
||||
function createMockFileSystem() {
|
||||
return new MockFileSystem({
|
||||
'/node_modules/test': {
|
||||
'package.json':
|
||||
'{"module": "./index.js", "es2015": "./es2015/index.js", "typings": "./index.d.ts"}',
|
||||
'index.d.ts': 'export * from "./public_api";',
|
||||
'index.js': 'export * from "./public_api";',
|
||||
'index.metadata.json': '...',
|
||||
'public_api.d.ts': `
|
||||
export * from "test/secondary";
|
||||
export * from "./nested";
|
||||
export declare class TestClass {};
|
||||
`,
|
||||
'public_api.js': `
|
||||
export * from "test/secondary";
|
||||
export * from "./nested";
|
||||
export const TestClass = function() {};
|
||||
`,
|
||||
'root.d.ts': `
|
||||
import * from 'other';
|
||||
export declare class RootClass {};
|
||||
`,
|
||||
'root.js': `
|
||||
import * from 'other';
|
||||
export const RootClass = function() {};
|
||||
`,
|
||||
'nested': {
|
||||
'index.d.ts': 'export * from "../root";',
|
||||
'index.js': 'export * from "../root";',
|
||||
},
|
||||
'es2015': {
|
||||
'index.js': 'export * from "./public_api";',
|
||||
'public_api.js': 'export class TestClass {};',
|
||||
'root.js': `
|
||||
import * from 'other';
|
||||
export class RootClass {};
|
||||
`,
|
||||
'nested': {
|
||||
'index.js': 'export * from "../root";',
|
||||
},
|
||||
},
|
||||
'secondary': {
|
||||
'package.json':
|
||||
'{"module": "./index.js", "es2015": "./es2015/index.js", "typings": "./index.d.ts"}',
|
||||
'index.d.ts': 'export * from "./public_api";',
|
||||
'index.js': 'export * from "./public_api";',
|
||||
'index.metadata.json': '...',
|
||||
'public_api.d.ts': 'export declare class SecondaryClass {};',
|
||||
'public_api.js': 'export class SecondaryClass {};',
|
||||
'es2015': {
|
||||
'index.js': 'export * from "./public_api";',
|
||||
'public_api.js': 'export class SecondaryClass {};',
|
||||
},
|
||||
},
|
||||
},
|
||||
'/node_modules/other': {
|
||||
'package.json':
|
||||
'{"module": "./index.js", "es2015": "./es2015/index.js", "typings": "./index.d.ts"}',
|
||||
'index.d.ts': 'export * from "./public_api";',
|
||||
'index.js': 'export * from "./public_api";',
|
||||
'index.metadata.json': '...',
|
||||
'public_api.d.ts': 'export declare class OtherClass {};',
|
||||
'public_api.js': 'export class OtherClass {};',
|
||||
'es2015': {
|
||||
'index.js': 'export * from "./public_api";',
|
||||
'public_api.js': 'export class OtherClass {};',
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
describe('entry point bundle', () => {
|
||||
// https://github.com/angular/angular/issues/29939
|
||||
it('should resolve JavaScript sources instead of declaration files if they are adjacent', () => {
|
||||
const fs = createMockFileSystem();
|
||||
const esm5bundle = makeEntryPointBundle(
|
||||
fs, '/node_modules/test', './index.js', './index.d.ts', false, 'esm5', 'esm5', true) !;
|
||||
|
||||
expect(esm5bundle.src.program.getSourceFiles().map(sf => sf.fileName))
|
||||
.toEqual(jasmine.arrayWithExactContents([
|
||||
// Modules from the entry-point itself should be source files
|
||||
'/node_modules/test/index.js',
|
||||
'/node_modules/test/public_api.js',
|
||||
'/node_modules/test/nested/index.js',
|
||||
'/node_modules/test/root.js',
|
||||
|
||||
// Modules from a secondary entry-point should be declaration files
|
||||
'/node_modules/test/secondary/public_api.d.ts',
|
||||
'/node_modules/test/secondary/index.d.ts',
|
||||
|
||||
// Modules resolved from "other" should be declaration files
|
||||
'/node_modules/other/public_api.d.ts',
|
||||
'/node_modules/other/index.d.ts',
|
||||
]));
|
||||
|
||||
expect(esm5bundle.dts !.program.getSourceFiles().map(sf => sf.fileName))
|
||||
.toEqual(jasmine.arrayWithExactContents([
|
||||
// All modules in the dts program should be declaration files
|
||||
'/node_modules/test/index.d.ts',
|
||||
'/node_modules/test/public_api.d.ts',
|
||||
'/node_modules/test/nested/index.d.ts',
|
||||
'/node_modules/test/root.d.ts',
|
||||
'/node_modules/test/secondary/public_api.d.ts',
|
||||
'/node_modules/test/secondary/index.d.ts',
|
||||
'/node_modules/other/public_api.d.ts',
|
||||
'/node_modules/other/index.d.ts',
|
||||
]));
|
||||
});
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user