Skip to content
Permalink
Browse files

fix(ngcc): correctly include internal .d.ts files (#33875)

Some declaration files may not be referenced from an entry-point's
main typings file, as it may declare types that are only used internally.
ngcc has logic to include declaration files based on all source files,
to ensure internal declaration files are available.

For packages following APF layout, however, this logic was insufficient.
Consider an entry-point with base path of `/esm2015/testing` and typings
residing in `/testing`, the file
`/esm2015/testing/src/nested/internal.js` has its typings file at
`/testing/src/nested/internal.d.ts`. Previously, the declaration was
assumed to be located at `/esm2015/testing/testing/internal.d.ts` (by
means of `/esm2015/testing/src/nested/../../testing/internal.d.ts`)
which is not where the declaration file can be found. This commit
resolves the issue by looking in the correct directory.

PR Close #33875
  • Loading branch information
JoostK authored and alxhub committed Nov 16, 2019
1 parent 5926b28 commit 0854dc86883d34570d759d93bbcce354c65d1bac
@@ -73,16 +73,20 @@ export function makeEntryPointBundle(
function computePotentialDtsFilesFromJsFiles(
fs: FileSystem, srcProgram: ts.Program, formatPath: AbsoluteFsPath,
typingsPath: AbsoluteFsPath) {
const relativePath = fs.relative(fs.dirname(formatPath), fs.dirname(typingsPath));
const formatRoot = fs.dirname(formatPath);
const typingsRoot = 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);

// Given a source file at e.g. `esm2015/src/some/nested/index.js`, try to resolve the
// declaration file under the typings root in `src/some/nested/index.d.ts`.
const mirroredDtsPath =
fs.resolve(typingsRoot, fs.relative(formatRoot, sf.fileName.replace(/\.js$/, '.d.ts')));

This comment has been minimized.

Copy link
@konradekk

konradekk Dec 7, 2019

In other places (e.g. d40ee6a) regexes are avoided; it seems that here’s a perfect place to incorporate mentioned findings: sf.fileName could be safely sliced/substr’d/substring’d, or whatever, and concatenated afterwards…?

if (fs.exists(mirroredDtsPath)) {
additionalFiles.push(mirroredDtsPath);
}
}
return additionalFiles;
@@ -128,6 +128,26 @@ runInEachFileSystem(() => {
name: _('/node_modules/other/es2015/public_api.js'),
contents: 'export class OtherClass {};'
},

// Mimic an AFP package with declaration files in a different tree than the sources
{name: _('/node_modules/internal/index.d.ts'), contents: 'export * from "./src/index";'},
{name: _('/node_modules/internal/src/index.d.ts'), contents: ''},
{
name: _('/node_modules/internal/src/internal.d.ts'),
contents: 'export declare function internal();'
},
{
name: _('/node_modules/internal/esm2015/index.js'),
contents: 'export * from "./src/index";'
},
{
name: _('/node_modules/internal/esm2015/src/index.js'),
contents: 'import {Internal} from "./internal";'
},
{
name: _('/node_modules/internal/esm2015/src/internal.js'),
contents: 'export function internal();'
},
]);
}

@@ -203,6 +223,29 @@ runInEachFileSystem(() => {
.toContain(absoluteFrom('/node_modules/test/internal.d.ts'));
});

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: 'internal',
packageJson: {name: 'internal'},
package: absoluteFrom('/node_modules/internal'),
path: absoluteFrom('/node_modules/internal'),
typings: absoluteFrom('/node_modules/internal/index.d.ts'),
compiledByAngular: true,
ignoreMissingDependencies: false,
generateDeepReexports: false,
};
const esm5bundle = makeEntryPointBundle(
fs, entryPoint, './esm2015/index.js', false, 'esm2015', /* transformDts */ true,
/* pathMappings */ undefined, /* mirrorDtsFromSrc */ true);
expect(esm5bundle.src.program.getSourceFiles().map(sf => sf.fileName))
.toContain(absoluteFrom('/node_modules/internal/esm2015/src/internal.js'));
expect(esm5bundle.dts !.program.getSourceFiles().map(sf => sf.fileName))
.toContain(absoluteFrom('/node_modules/internal/src/internal.d.ts'));
});

it('should ignore, internally imported, src files in the typings program, if `mirrorDtsFromSrc` is false',
() => {
setupMockFileSystem();

0 comments on commit 0854dc8

Please sign in to comment.
You can’t perform that action at this time.