Skip to content

Commit

Permalink
fix(ngcc): correctly report error when collecting dependencies of UMD…
Browse files Browse the repository at this point in the history
… module (angular#44245)

Previously, the ngcc `UmdReflectionHost` would throw a misleading error
when trying to collect dependencies of an invalidly formatted UMD
module. This happened because an error would be thrown while trying to
construct the error message for the actual error, by calling `getText()`
on certain TypeScript AST nodes. See
angular#44019 (comment)
for a more in-depth explanation.

This commit ensures `getText()` can be safely called on TypeScript AST
nodes when collecting dependencies of UMD modules.

PR Close angular#44245
  • Loading branch information
gkalpak authored and dimakuba committed Dec 28, 2021
1 parent 7b79071 commit fb89525
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class UmdDependencyHost extends DependencyHostBase {
protected override extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
const sf =
ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, true, ts.ScriptKind.JS);

if (sf.statements.length !== 1) {
return new Set();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ runInEachFileSystem(() => {
expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true);
expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true);
});

it('should correctly report errors for invalid source files', () => {
const {dependencies, missing, deepImports} = createDependencyInfo();

expect(
() => host.collectDependencies(
_('/invalid/format/index.js'), {dependencies, missing, deepImports}))
.toThrowError(
/^UMD wrapper body is not in a supported format \(expected a conditional expression or if statement\)/);
});
});

function setupMockFileSystem(): void {
Expand All @@ -172,6 +182,16 @@ runInEachFileSystem(() => {
contents: '{"esm2015": "./index.js"}'
},
{name: _('/no/imports/but/cannot/skip/index.metadata.json'), contents: 'MOCK METADATA'},
{
name: _('/invalid/format/index.js'),
contents: `
(function (global, factory) {
const invalidWrapperFunctionFormat = require('true');
}(this, function () {}));
`,
},
{name: _('/invalid/format/package.json'), contents: '{"esm2015": "./index.js"}'},
{name: _('/invalid/format/index.metadata.json'), contents: 'MOCK METADATA'},
{
name: _('/external/imports/index.js'),
contents: umd('imports_index', ['lib_1', 'lib_1/sub_1'])
Expand Down

0 comments on commit fb89525

Please sign in to comment.