fix(ngcc): render UMD imports even if no prior imports (#34353)

Previously the UMD rendering formatter assumed that
there would already be import (and an export) arguments
to the UMD factory function.

This commit adds support for this corner case.

Fixes #34138

PR Close #34353

packages/compiler-cli/ngcc/src/rendering/umd_rendering_formatter.ts

@@ -121,9 +121,10 @@ function renderCommonJsDependencies(
     return;
   }
   const factoryCall = conditional.whenTrue;
-  const injectionPoint = factoryCall.getEnd() -
-      1;  // Backup one char to account for the closing parenthesis on the call
-  imports.forEach(i => output.appendLeft(injectionPoint, `,require('${i.specifier}')`));
+  // Backup one char to account for the closing parenthesis on the call
+  const injectionPoint = factoryCall.getEnd() - 1;
+  const importString = imports.map(i => `require('${i.specifier}')`).join(',');
+  output.appendLeft(injectionPoint, (factoryCall.arguments.length > 0 ? ',' : '') + importString);
 }
 
 /**
@@ -135,13 +136,24 @@ function renderAmdDependencies(
   if (!conditional) {
     return;
   }
-  const dependencyArray = conditional.whenTrue.arguments[1];
-  if (!dependencyArray || !ts.isArrayLiteralExpression(dependencyArray)) {
-    return;
+  const amdDefineCall = conditional.whenTrue;
+  const importString = imports.map(i => `'${i.specifier}'`).join(',');
+  // The dependency array (if it exists) is the second to last argument
+  // `define(id?, dependencies?, factory);`
+  const factoryIndex = amdDefineCall.arguments.length - 1;
+  const dependencyArray = amdDefineCall.arguments[factoryIndex - 1];
+  if (dependencyArray === undefined || !ts.isArrayLiteralExpression(dependencyArray)) {
+    // No array provided: `define(factory)` or `define(id, factory)`.
+    // Insert a new array in front the `factory` call.
+    const injectionPoint = amdDefineCall.arguments[factoryIndex].getFullStart();
+    output.appendLeft(injectionPoint, `[${importString}],`);
+  } else {
+    // Already an array, add imports to the end of the array.
+    // Backup one char to account for the closing square bracket on the array
+    const injectionPoint = dependencyArray.getEnd() - 1;
+    output.appendLeft(
+        injectionPoint, (dependencyArray.elements.length > 0 ? ',' : '') + importString);
   }
-  const injectionPoint = dependencyArray.getEnd() -
-      1;  // Backup one char to account for the closing square bracket on the array
-  imports.forEach(i => output.appendLeft(injectionPoint, `,'${i.specifier}'`));
 }
 
 /**
@@ -155,7 +167,9 @@ function renderGlobalDependencies(
   }
   // Backup one char to account for the closing parenthesis after the argument list of the call.
   const injectionPoint = globalFactoryCall.getEnd() - 1;
-  imports.forEach(i => output.appendLeft(injectionPoint, `,global.${getGlobalIdentifier(i)}`));
+  const importString = imports.map(i => `global.${getGlobalIdentifier(i)}`).join(',');
+  output.appendLeft(
+      injectionPoint, (globalFactoryCall.arguments.length > 0 ? ',' : '') + importString);
 }
 
 /**
@@ -175,9 +189,20 @@ function renderFactoryParameters(
   if (!ts.isFunctionExpression(factoryFunction)) {
     return;
   }
+
   const parameters = factoryFunction.parameters;
-  const injectionPoint = parameters[parameters.length - 1].getEnd();
-  imports.forEach(i => output.appendLeft(injectionPoint, `,${i.qualifier}`));
+  const parameterString = imports.map(i => i.qualifier).join(',');
+  if (parameters.length > 0) {
+    const injectionPoint = parameters[parameters.length - 1].getEnd();
+    output.appendLeft(injectionPoint, ',' + parameterString);
+  } else {
+    // If there are no parameters then the factory function will look like:
+    // function () { ... }
+    // The AST does not give us a way to find the insertion point - between the two parentheses.
+    // So we must use a regular expression on the text of the function.
+    const injectionPoint = factoryFunction.getStart() + factoryFunction.getText().indexOf('()') + 1;
+    output.appendLeft(injectionPoint, parameterString);
+  }
 }
 
 /**

packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts

@@ -291,6 +291,40 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
            expect(output.toString())
                .toContain(`(function (exports,someSideEffect,localDep,core,i0,i1) {'use strict';`);
          });
+
+      it('should handle the case where there were no prior imports nor exports', () => {
+        const PROGRAM: TestFile = {
+          name: _('/node_modules/test-package/some/file.js'),
+          contents: `
+          /* A copyright notice */
+          (function (global, factory) {
+          typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
+          typeof define === 'function' && define.amd ? define('file', factory) :
+          (factory());
+          }(this, (function () {'use strict';
+            var index = '';
+            return index;
+          })));`,
+        };
+        const {renderer, program} = setup(PROGRAM);
+        const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js'));
+        const output = new MagicString(PROGRAM.contents);
+        renderer.addImports(
+            output,
+            [
+              {specifier: '@angular/core', qualifier: 'i0'},
+              {specifier: '@angular/common', qualifier: 'i1'}
+            ],
+            file);
+        const outputSrc = output.toString();
+
+        expect(outputSrc).toContain(
+            `typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('@angular/core'),require('@angular/common')) :`);
+        expect(outputSrc).toContain(
+            `typeof define === 'function' && define.amd ? define('file',['@angular/core','@angular/common'], factory) :`);
+        expect(outputSrc).toContain(`(factory(global.ng.core,global.ng.common));`);
+        expect(outputSrc).toContain(`(function (i0,i1) {'use strict';`);
+      });
     });
 
     describe('addExports', () => {