From 5185ba737248860eb653ef09f103ff117371cca5 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Tue, 15 Mar 2022 15:25:21 +0000 Subject: [PATCH] cleanup(angular): address PR comments --- .../webpack-browser/webpack-browser.impl.ts | 15 +++-- .../angular/src/utils/mfe/mfe-webpack.spec.ts | 8 +-- packages/angular/src/utils/mfe/mfe-webpack.ts | 6 +- .../utils/mfe/with-module-federation.spec.ts | 14 +++-- .../src/utils/mfe/with-module-federation.ts | 57 +++++++++++++------ 5 files changed, 63 insertions(+), 37 deletions(-) diff --git a/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts b/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts index 4a822aff591755..d5471b18d0cb6f 100644 --- a/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts +++ b/packages/angular/src/builders/webpack-browser/webpack-browser.impl.ts @@ -66,22 +66,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); } }, }); diff --git a/packages/angular/src/utils/mfe/mfe-webpack.spec.ts b/packages/angular/src/utils/mfe/mfe-webpack.spec.ts index 9a51bf931d6d5b..63d32c7ed7d3fe 100644 --- a/packages/angular/src/utils/mfe/mfe-webpack.spec.ts +++ b/packages/angular/src/utils/mfe/mfe-webpack.spec.ts @@ -1,7 +1,7 @@ jest.mock('fs'); -jest.mock('@nrwl/workspace'); +jest.mock('@nrwl/workspace/src/utilities/typescript'); import * as fs from 'fs'; -import * as workspace from '@nrwl/workspace'; +import * as workspace from '@nrwl/workspace/src/utilities/typescript'; import { sharePackages, shareWorkspaceLibraries } from './mfe-webpack'; @@ -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!' ); } }); diff --git a/packages/angular/src/utils/mfe/mfe-webpack.ts b/packages/angular/src/utils/mfe/mfe-webpack.ts index b60447ef404468..314680e0a6538b 100644 --- a/packages/angular/src/utils/mfe/mfe-webpack.ts +++ b/packages/angular/src/utils/mfe/mfe-webpack.ts @@ -1,11 +1,13 @@ -import { readTsConfig } from '@nrwl/workspace'; 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; diff --git a/packages/angular/src/utils/mfe/with-module-federation.spec.ts b/packages/angular/src/utils/mfe/with-module-federation.spec.ts index 57a2d49a393213..d58c70bfce6384 100644 --- a/packages/angular/src/utils/mfe/with-module-federation.spec.ts +++ b/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'], diff --git a/packages/angular/src/utils/mfe/with-module-federation.ts b/packages/angular/src/utils/mfe/with-module-federation.ts index ae628c5306e7be..82df11f0cb3351 100644 --- a/packages/angular/src/utils/mfe/with-module-federation.ts +++ b/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, - target: string + target: string, + seenTargets: Set = 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(); 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) {