fix(ngcc): correctly report error when collecting dependencies of UMD…

… module 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.
gkalpak · Nov 24, 2021 · bc789a1 · bc789a1
1 parent 9a7b39c
commit bc789a1
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 1 deletion.
diff --git a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts
@@ -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();

diff --git a/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts
@@ -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 {
@@ -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'])