Skip to content

Commit

Permalink
fix(ngcc): ensure that bundle rootDir is the package path (#34212)
Browse files Browse the repository at this point in the history
Previously the `rootDir` was set to the entry-point path but
this is incorrect if the source files are stored in a directory outside
the entry-point path. This is the case in the latest versions of the
Angular CDK.

Instead the `rootDir` should be the containing package path, which is
guaranteed to include all the source for the entry-point.

---

A symptom of this is an error when ngcc is trying to process the source of
an entry-point format after the entry-point's typings have already been
processed by a previous processing run.

During processing the `_toR3Reference()` function gets called which in turn
makes a call to `ReflectionHost.getDtsDeclaration()`. If the typings files
are also being processed this returns the node from the dts typings files.

But if we have already processed the typings files and are now processing
only an entry-point format without typings, the call to
`ReflectionHost.getDtsDeclaration()` returns `null`.

When this value is `null`, a JS `valueRef` is passed through as the DTS
`typeRef` to the `ReferenceEmitter`. In this case, the `ReferenceEmitter`
fails during `emit()` because no `ReferenceEmitStrategy` is able to provide
an emission:

1) The `LocalIdentifierStrategy` is not able help because in this case
`ImportMode` is `ForceNewImport`.
2) The `LogicalProjectStrategy` cannot find the JS file below the `rootDir`.

The second strategy failure is fixed by this PR.

Fixes angular/ngcc-validation#495

PR Close #34212
  • Loading branch information
petebacondarwin authored and AndrewKushnir committed Dec 5, 2019
1 parent aa2b578 commit 69dd516
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
12 changes: 7 additions & 5 deletions packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts
Expand Up @@ -47,15 +47,14 @@ export function makeEntryPointBundle(
mirrorDtsFromSrc: boolean = false,
enableI18nLegacyMessageIdFormat: boolean = true): EntryPointBundle {
// Create the TS program and necessary helpers.
const rootDir = entryPoint.package;
const options: ts.CompilerOptions = {
allowJs: true,
maxNodeModuleJsDepth: Infinity,
noLib: true,
rootDir: entryPoint.path, ...pathMappings
noLib: true, rootDir, ...pathMappings
};
const srcHost = new NgccSourcesCompilerHost(fs, options, entryPoint.path);
const dtsHost = new NgtscCompilerHost(fs, options);
const rootDirs = [absoluteFrom(entryPoint.path)];

// Create the bundle programs, as necessary.
const absFormatPath = fs.resolve(entryPoint.path, formatPath);
Expand All @@ -71,8 +70,11 @@ export function makeEntryPointBundle(
null;
const isFlatCore = isCore && src.r3SymbolsFile === null;

return {entryPoint, format, rootDirs, isCore,
isFlatCore, src, dts, enableI18nLegacyMessageIdFormat};
return {
entryPoint,
format,
rootDirs: [rootDir], isCore, isFlatCore, src, dts, enableI18nLegacyMessageIdFormat
};
}

function computePotentialDtsFilesFromJsFiles(
Expand Down
Expand Up @@ -148,6 +148,16 @@ runInEachFileSystem(() => {
name: _('/node_modules/internal/esm2015/src/internal.js'),
contents: 'export function internal();'
},

// A package with a secondary entry-point that has source files in a different tree
{
name: _('/node_modules/primary/secondary/index.d.ts'),
contents: 'export declare function secondary();'
},
{
name: _('/node_modules/primary/esm2015/secondary/index.js'),
contents: 'export function secondary();'
},
]);
}

Expand Down Expand Up @@ -268,5 +278,24 @@ runInEachFileSystem(() => {
expect(esm5bundle.dts !.program.getSourceFiles().map(sf => sf.fileName))
.not.toContain(absoluteFrom('/node_modules/test/internal.d.ts'));
});

it('should set the `rootDir` to the package path not the entry-point path', () => {
setupMockFileSystem();
const fs = getFileSystem();
const entryPoint: EntryPoint = {
name: 'secondary',
packageJson: {name: 'secondary'},
package: absoluteFrom('/node_modules/primary'),
path: absoluteFrom('/node_modules/primary/secondary'),
typings: absoluteFrom('/node_modules/primary/secondary/index.d.ts'),
compiledByAngular: true,
ignoreMissingDependencies: false,
generateDeepReexports: false,
};
const bundle = makeEntryPointBundle(
fs, entryPoint, './index.js', false, 'esm2015', /* transformDts */ true,
/* pathMappings */ undefined, /* mirrorDtsFromSrc */ true);
expect(bundle.rootDirs).toEqual([absoluteFrom('/node_modules/primary')]);
});
});
});

0 comments on commit 69dd516

Please sign in to comment.