From 83b19bf1a248dcfdc426c992fce282e2f5fe7adc Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 3 Jul 2019 19:31:33 +0100 Subject: [PATCH] fix(ivy): ngcc - compute potential d.ts files from .js files (#31411) If a package delcares a class internally on an NgModule, ngcc needs to be able to add a public export to this class's type. Previously, if the typing file for the declared is not imported from the typings entry-point file, then ngcc cannot find it. Now we try to guess the .d.ts files from the equivalent .js files. PR Close #31411 --- packages/compiler-cli/ngcc/src/main.ts | 3 +- .../ngcc/src/packages/bundle_program.ts | 6 ++- .../ngcc/src/packages/entry_point_bundle.ts | 46 +++++++++++++---- .../test/packages/entry_point_bundle_spec.ts | 50 +++++++++++++++++++ 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 7b1796aa38..336774dc7f 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -154,7 +154,8 @@ export function mainNgcc( // the property as processed even if its underlying format has been built already. if (!compiledFormats.has(formatPath) && (compileAllFormats || isFirstFormat)) { const bundle = makeEntryPointBundle( - fileSystem, entryPoint, formatPath, isCore, property, format, processDts, pathMappings); + fileSystem, entryPoint, formatPath, isCore, property, format, processDts, pathMappings, + true); if (bundle) { logger.info(`Compiling ${entryPoint.name} : ${property} as ${format}`); const transformedFiles = transformer.transform(bundle); diff --git a/packages/compiler-cli/ngcc/src/packages/bundle_program.ts b/packages/compiler-cli/ngcc/src/packages/bundle_program.ts index 74e058b113..99801adfad 100644 --- a/packages/compiler-cli/ngcc/src/packages/bundle_program.ts +++ b/packages/compiler-cli/ngcc/src/packages/bundle_program.ts @@ -32,9 +32,11 @@ export interface BundleProgram { */ export function makeBundleProgram( fs: FileSystem, isCore: boolean, path: AbsoluteFsPath, r3FileName: string, - options: ts.CompilerOptions, host: ts.CompilerHost): BundleProgram { + options: ts.CompilerOptions, host: ts.CompilerHost, + additionalFiles: AbsoluteFsPath[] = []): BundleProgram { const r3SymbolsPath = isCore ? findR3SymbolsPath(fs, dirname(path), r3FileName) : null; - const rootPaths = r3SymbolsPath ? [path, r3SymbolsPath] : [path]; + let rootPaths = + r3SymbolsPath ? [path, r3SymbolsPath, ...additionalFiles] : [path, ...additionalFiles]; const originalGetExpandoInitializer = patchTsGetExpandoInitializer(); const program = ts.createProgram(rootPaths, options, host); diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts b/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts index 37cc5c5464..18229512dd 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; -import {AbsoluteFsPath, FileSystem, absoluteFrom, resolve} from '../../../src/ngtsc/file_system'; +import {AbsoluteFsPath, FileSystem, absoluteFrom} from '../../../src/ngtsc/file_system'; import {NgtscCompilerHost} from '../../../src/ngtsc/file_system/src/compiler_host'; import {PathMappings} from '../utils'; import {BundleProgram, makeBundleProgram} from './bundle_program'; @@ -30,17 +30,21 @@ export interface EntryPointBundle { /** * Get an object that describes a formatted bundle for an entry-point. - * @param entryPointPath The path to the entry-point that contains the bundle. + * @param fs The current file-system being used. + * @param entryPoint The entry-point that contains the bundle. * @param formatPath The path to the source files for this bundle. - * @param typingsPath The path to the typings files if we should transform them with this bundle. * @param isCore This entry point is the Angular core package. + * @param formatProperty The property in the package.json that holds the formatPath. * @param format The underlying format of the bundle. * @param transformDts Whether to transform the typings along with this bundle. + * @param pathMappings An optional set of mappings to use when compiling files. + * @param mirrorDtsFromSrc If true then the `dts` program will contain additional files that + * were guessed by mapping the `src` files to `dts` files. */ export function makeEntryPointBundle( fs: FileSystem, entryPoint: EntryPoint, formatPath: string, isCore: boolean, formatProperty: EntryPointJsonProperty, format: EntryPointFormat, transformDts: boolean, - pathMappings?: PathMappings): EntryPointBundle|null { + pathMappings?: PathMappings, mirrorDtsFromSrc: boolean = false): EntryPointBundle|null { // Create the TS program and necessary helpers. const options: ts.CompilerOptions = { allowJs: true, @@ -53,13 +57,35 @@ export function makeEntryPointBundle( const rootDirs = [absoluteFrom(entryPoint.path)]; // Create the bundle programs, as necessary. - const src = makeBundleProgram( - fs, isCore, resolve(entryPoint.path, formatPath), 'r3_symbols.js', options, srcHost); - const dts = transformDts ? makeBundleProgram( - fs, isCore, resolve(entryPoint.path, entryPoint.typings), - 'r3_symbols.d.ts', options, dtsHost) : - null; + const absFormatPath = fs.resolve(entryPoint.path, formatPath); + const typingsPath = fs.resolve(entryPoint.path, entryPoint.typings); + const src = makeBundleProgram(fs, isCore, absFormatPath, 'r3_symbols.js', options, srcHost); + const additionalDtsFiles = transformDts && mirrorDtsFromSrc ? + computePotentialDtsFilesFromJsFiles(fs, src.program, absFormatPath, typingsPath) : + []; + const dts = transformDts ? + makeBundleProgram( + fs, isCore, typingsPath, 'r3_symbols.d.ts', options, dtsHost, additionalDtsFiles) : + null; const isFlatCore = isCore && src.r3SymbolsFile === null; return {entryPoint, format, formatProperty, rootDirs, isCore, isFlatCore, src, dts}; } + +function computePotentialDtsFilesFromJsFiles( + fs: FileSystem, srcProgram: ts.Program, formatPath: AbsoluteFsPath, + typingsPath: AbsoluteFsPath) { + const relativePath = fs.relative(fs.dirname(formatPath), fs.dirname(typingsPath)); + const additionalFiles: AbsoluteFsPath[] = []; + for (const sf of srcProgram.getSourceFiles()) { + if (!sf.fileName.endsWith('.js')) { + continue; + } + const dtsPath = fs.resolve( + fs.dirname(sf.fileName), relativePath, fs.basename(sf.fileName, '.js') + '.d.ts'); + if (fs.exists(dtsPath)) { + additionalFiles.push(dtsPath); + } + } + return additionalFiles; +} \ No newline at end of file diff --git a/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts b/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts index 9817784d7d..ee137b809f 100644 --- a/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts @@ -28,6 +28,7 @@ runInEachFileSystem(() => { { name: _('/node_modules/test/public_api.d.ts'), contents: ` + // Note: no import from './internal'; export * from "test/secondary"; export * from "./nested"; export declare class TestClass {}; @@ -36,6 +37,7 @@ runInEachFileSystem(() => { { name: _('/node_modules/test/public_api.js'), contents: ` + import {internal} from './internal'; export * from "test/secondary"; export * from "./nested"; export const TestClass = function() {}; @@ -55,6 +57,11 @@ runInEachFileSystem(() => { export const RootClass = function() {}; ` }, + { + name: _('/node_modules/test/internal.d.ts'), + contents: `export declare function internal(): void;` + }, + {name: _('/node_modules/test/internal.js'), contents: `export function internal() {}`}, {name: _('/node_modules/test/nested/index.d.ts'), contents: 'export * from "../root";'}, {name: _('/node_modules/test/nested/index.js'), contents: 'export * from "../root";'}, {name: _('/node_modules/test/es2015/index.js'), contents: 'export * from "./public_api";'}, @@ -147,6 +154,7 @@ runInEachFileSystem(() => { '/node_modules/test/public_api.js', '/node_modules/test/nested/index.js', '/node_modules/test/root.js', + '/node_modules/test/internal.js', // Modules from a secondary entry-point should be declaration files '/node_modules/test/secondary/public_api.d.ts', @@ -170,5 +178,47 @@ runInEachFileSystem(() => { '/node_modules/other/index.d.ts', ].map(p => absoluteFrom(p).toString()))); }); + + it('should include equivalently named, internally imported, src files in the typings program, if `mirrorDtsFromSrc` is true', + () => { + setupMockFileSystem(); + const fs = getFileSystem(); + const entryPoint: EntryPoint = { + name: 'test', + packageJson: {name: 'test'}, + package: absoluteFrom('/node_modules/test'), + path: absoluteFrom('/node_modules/test'), + typings: absoluteFrom('/node_modules/test/index.d.ts'), + compiledByAngular: true, + }; + const esm5bundle = makeEntryPointBundle( + fs, entryPoint, './index.js', false, 'esm5', 'esm5', /* transformDts */ true, + /* pathMappings */ undefined, /* mirrorDtsFromSrc */ true) !; + expect(esm5bundle.src.program.getSourceFiles().map(sf => sf.fileName)) + .toContain(absoluteFrom('/node_modules/test/internal.js')); + expect(esm5bundle.dts !.program.getSourceFiles().map(sf => sf.fileName)) + .toContain(absoluteFrom('/node_modules/test/internal.d.ts')); + }); + + it('should ignore, internally imported, src files in the typings program, if `mirrorDtsFromSrc` is false', + () => { + setupMockFileSystem(); + const fs = getFileSystem(); + const entryPoint: EntryPoint = { + name: 'test', + packageJson: {name: 'test'}, + package: absoluteFrom('/node_modules/test'), + path: absoluteFrom('/node_modules/test'), + typings: absoluteFrom('/node_modules/test/index.d.ts'), + compiledByAngular: true, + }; + const esm5bundle = makeEntryPointBundle( + fs, entryPoint, './index.js', false, 'esm5', 'esm5', /* transformDts */ true, + /* pathMappings */ undefined, /* mirrorDtsFromSrc */ false) !; + expect(esm5bundle.src.program.getSourceFiles().map(sf => sf.fileName)) + .toContain(absoluteFrom('/node_modules/test/internal.js')); + expect(esm5bundle.dts !.program.getSourceFiles().map(sf => sf.fileName)) + .not.toContain(absoluteFrom('/node_modules/test/internal.d.ts')); + }); }); });