cleanup(angular): address PR comments

packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts

@@ -69,22 +69,21 @@ function buildAppWithCustomWebpackConfiguration(
         pathToWebpackConfig,
         options.tsConfig
       );
+      // The extra Webpack configuration file can also export a Promise, for instance:
+      // `module.exports = new Promise(...)`. If it exports a single object, but not a Promise,
+      // then await will just resolve that object.
+      const config = await customWebpackConfiguration;
+
       // The extra Webpack configuration file can export a synchronous or asynchronous function,
       // for instance: `module.exports = async config => { ... }`.
-      if (typeof customWebpackConfiguration === 'function') {
+      if (typeof config === 'function') {
         return customWebpackConfiguration(
           baseWebpackConfig,
           options,
           context.target
         );
       } else {
-        return merge(
-          baseWebpackConfig,
-          // The extra Webpack configuration file can also export a Promise, for instance:
-          // `module.exports = new Promise(...)`. If it exports a single object, but not a Promise,
-          // then await will just resolve that object.
-          await customWebpackConfiguration
-        );
+        return merge(baseWebpackConfig, config);
       }
     },
   });

packages/angular/src/utils/mfe/mfe-webpack.spec.ts

@@ -18,8 +18,8 @@ describe('MFE Webpack Utils', () => {
         shareWorkspaceLibraries(['@myorg/shared']);
       } catch (error) {
         // ASSERT
-        expect(error.message).toEqual(
-          'NX MFE: TsConfig Path for workspace libraries does not exist! (null)'
+        expect(error.message).toContain(
+          'NX MFE: TsConfig Path for workspace libraries does not exist!'
         );
       }
     });

packages/angular/src/utils/mfe/mfe-webpack.ts

@@ -1,11 +1,13 @@
-import { readTsConfig } from '@nrwl/workspace/src/utilities/typescript';
 import { existsSync, readFileSync } from 'fs';
 import { NormalModuleReplacementPlugin } from 'webpack';
 import { appRootPath as rootPath } from 'nx/src/utils/app-root';
 import { normalizePath, joinPathFragments } from '@nrwl/devkit';
 import { dirname } from 'path';
 import { ParsedCommandLine } from 'typescript';
-import { getRootTsConfigPath } from '@nrwl/workspace/src/utilities/typescript';
+import {
+  getRootTsConfigPath,
+  readTsConfig,
+} from '@nrwl/workspace/src/utilities/typescript';
 
 export interface SharedLibraryConfig {
   singleton: boolean;

packages/angular/src/utils/mfe/with-module-federation.spec.ts

@@ -1,9 +1,11 @@
 jest.mock('fs');
 jest.mock('@nrwl/workspace/src/core/project-graph');
-jest.mock('@nrwl/workspace');
+jest.mock('@nrwl/workspace/src/utilities/typescript');
+jest.mock('@nrwl/workspace/src/core/file-utils');
 jest.mock('nx/src/shared/workspace');
 import * as graph from '@nrwl/workspace/src/core/project-graph';
-import * as workspace from '@nrwl/workspace';
+import * as typescriptUtils from '@nrwl/workspace/src/utilities/typescript';
+import * as workspace from '@nrwl/workspace/src/core/file-utils';
 import * as taoWorkspace from 'nx/src/shared/workspace';
 import * as fs from 'fs';
 
@@ -46,7 +48,7 @@ describe('withModuleFederation', () => {
       })
     );
 
-    (workspace.readTsConfig as jest.Mock).mockReturnValue({
+    (typescriptUtils.readTsConfig as jest.Mock).mockReturnValue({
       options: {
         paths: {
           shared: ['/libs/shared/src/index.ts'],
@@ -97,7 +99,7 @@ describe('withModuleFederation', () => {
       })
     );
 
-    (workspace.readTsConfig as jest.Mock).mockReturnValue({
+    (typescriptUtils.readTsConfig as jest.Mock).mockReturnValue({
       options: {
         paths: {
           shared: ['/libs/shared/src/index.ts'],
@@ -149,7 +151,7 @@ describe('withModuleFederation', () => {
       })
     );
 
-    (workspace.readTsConfig as jest.Mock).mockReturnValue({
+    (typescriptUtils.readTsConfig as jest.Mock).mockReturnValue({
       options: {
         paths: {
           shared: ['/libs/shared/src/index.ts'],
@@ -205,7 +207,7 @@ describe('withModuleFederation', () => {
       })
     );
 
-    (workspace.readTsConfig as jest.Mock).mockImplementation(() => ({
+    (typescriptUtils.readTsConfig as jest.Mock).mockImplementation(() => ({
       options: {
         paths: {
           shared: ['/libs/shared/src/index.ts'],

packages/angular/src/utils/mfe/with-module-federation.ts

@@ -7,13 +7,15 @@ import {
   createProjectGraphAsync,
   readCachedProjectGraph,
 } from '@nrwl/workspace/src/core/project-graph';
-import { readWorkspaceJson } from '@nrwl/workspace';
+import { readWorkspaceJson } from '@nrwl/workspace/src/core/file-utils';
 import { joinPathFragments, ProjectGraph } from '@nrwl/devkit';
 import ModuleFederationPlugin = require('webpack/lib/container/ModuleFederationPlugin');
 import { Workspaces } from 'nx/src/shared/workspace';
 import { appRootPath } from 'nx/src/utils/app-root';
-import { getRootTsConfigPath } from '@nrwl/workspace/src/utilities/typescript';
-import { readTsConfig } from '@nrwl/workspace';
+import {
+  getRootTsConfigPath,
+  readTsConfig,
+} from '@nrwl/workspace/src/utilities/typescript';
 import { ParsedCommandLine } from 'typescript';
 
 export type MFERemotes = string[] | [remoteName: string, remoteUrl: string][];
@@ -30,9 +32,14 @@ export interface MFEConfig {
 
 function recursivelyResolveWorkspaceDependents(
   projectGraph: ProjectGraph<any>,
-  target: string
+  target: string,
+  seenTargets: Set<string> = new Set()
 ) {
+  if (seenTargets.has(target)) {
+    return [];
+  }
   let dependencies = [target];
+  seenTargets.add(target);
 
   const workspaceDependencies = (
     projectGraph.dependencies[target] ?? []
@@ -41,10 +48,15 @@ function recursivelyResolveWorkspaceDependents(
     for (const dep of workspaceDependencies) {
       dependencies = [
         ...dependencies,
-        ...recursivelyResolveWorkspaceDependents(projectGraph, dep.target),
+        ...recursivelyResolveWorkspaceDependents(
+          projectGraph,
+          dep.target,
+          seenTargets
+        ),
       ];
     }
   }
+
   return dependencies;
 }
 
@@ -90,26 +102,31 @@ async function getDependentPackagesForProject(name: string) {
 
   const deps = projectGraph.dependencies[name].reduce(
     (dependencies, dependency) => {
-      const workspaceLibraries = dependencies.workspaceLibraries;
-      const npmPackages = dependencies.npmPackages;
+      const workspaceLibraries = new Set(dependencies.workspaceLibraries);
+      const npmPackages = new Set(dependencies.npmPackages);
 
-      if (dependency.target.startsWith('npm')) {
-        npmPackages.push(dependency.target.replace('npm:', ''));
+      if (dependency.target.startsWith('npm:')) {
+        npmPackages.add(dependency.target.replace('npm:', ''));
       } else {
-        workspaceLibraries.push(dependency.target);
+        workspaceLibraries.add(dependency.target);
       }
 
       return {
-        workspaceLibraries,
-        npmPackages,
+        workspaceLibraries: [...workspaceLibraries],
+        npmPackages: [...npmPackages],
       };
     },
     { workspaceLibraries: [], npmPackages: [] }
   );
+  const seenWorkspaceLibraries = new Set<string>();
   deps.workspaceLibraries = deps.workspaceLibraries.reduce(
     (workspaceLibraryDeps, workspaceLibrary) => [
       ...workspaceLibraryDeps,
-      ...recursivelyResolveWorkspaceDependents(projectGraph, workspaceLibrary),
+      ...recursivelyResolveWorkspaceDependents(
+        projectGraph,
+        workspaceLibrary,
+        seenWorkspaceLibraries
+      ),
     ],
     []
   );
@@ -122,10 +139,16 @@ async function getDependentPackagesForProject(name: string) {
 
 function determineRemoteUrl(remote: string) {
   const workspace = readWorkspaceJson();
-  return joinPathFragments(
-    workspace.projects[remote].targets.serve.options.publicHost,
-    'remoteEntry.mjs'
-  );
+  let publicHost = '';
+  try {
+    publicHost = workspace.projects[remote].targets.serve.options.publicHost;
+  } catch (error) {
+    throw new Error(
+      `Cannot automatically determine URL of remote (${remote}). Looked for property "publicHost" in the project's "serve" target.\n
+      You can also use the tuple syntax in your webpack config to configure your remotes. e.g. \`remotes: [['remote1', 'http://localhost:4201']]\``
+    );
+  }
+  return joinPathFragments(publicHost, 'remoteEntry.mjs');
 }
 
 function mapRemotes(remotes: MFERemotes) {