From 337b3b75477bc6f14e00537df9a7e5815266797b Mon Sep 17 00:00:00 2001
From: Nicholas Cunningham <ndcunningham@gmail.com>
Date: Tue, 30 Apr 2024 12:41:38 -0600
Subject: [PATCH] fix(module-federation): Throw an error if remote is invalid

closes: #23024
---
 e2e/angular/src/module-federation.test.ts     | 11 ++++++
 e2e/react/src/react-module-federation.test.ts | 11 ++++++
 e2e/utils/command-utils.ts                    |  4 +-
 packages/angular/src/generators/host/host.ts  | 14 +++++++
 packages/js/src/index.ts                      |  1 +
 packages/js/src/utils/is-valid-variable.ts    | 37 +++++++++++++++++++
 packages/react/src/generators/host/host.ts    | 14 +++++++
 .../react/src/generators/remote/remote.ts     | 13 +++++++
 8 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100644 packages/js/src/utils/is-valid-variable.ts

diff --git a/e2e/angular/src/module-federation.test.ts b/e2e/angular/src/module-federation.test.ts
index 7657bba1a79a98..9e7b8e38472381 100644
--- a/e2e/angular/src/module-federation.test.ts
+++ b/e2e/angular/src/module-federation.test.ts
@@ -310,6 +310,17 @@ describe('Angular Module Federation', () => {
     await killProcessAndPorts(processTsNode.pid, hostPort, remotePort);
   }, 20_000_000);
 
+  it('should throw an error if invalid remotes names are provided and --dynamic is set to true', async () => {
+    const shell = uniq('shell');
+    const remote = uniq('remote-1');
+    const result = runCLI(
+      `generate @nx/angular:host ${shell} --remotes=${remote} --e2eTestRunner=none --dynamic=true --project-name-and-root-format=as-provided --no-interactive --skipFormat`,
+      { silenceError: true }
+    );
+
+    expect(result).toContain(`Invalid remote name provided: ${remote}`);
+  });
+
   it('should federate a module from a library and update an existing remote', async () => {
     const lib = uniq('lib');
     const remote = uniq('remote');
diff --git a/e2e/react/src/react-module-federation.test.ts b/e2e/react/src/react-module-federation.test.ts
index d55ca519bb1a29..501e6c0232abf3 100644
--- a/e2e/react/src/react-module-federation.test.ts
+++ b/e2e/react/src/react-module-federation.test.ts
@@ -1001,6 +1001,17 @@ describe('React Module Federation', () => {
       }
     }, 500_000);
   });
+
+  it('should throw an error if invalid remotes names are provided and --dynamic is set to true', async () => {
+    const shell = uniq('shell');
+    const remote = uniq('remote-1');
+    const result = runCLI(
+      `generate @nx/react:host ${shell} --remotes=${remote} --e2eTestRunner=none --dynamic=true --project-name-and-root-format=as-provided --no-interactive --skipFormat`,
+      { silenceError: true }
+    );
+
+    expect(result).toContain(`Invalid remote name provided: ${remote}`);
+  }, 200_000);
 });
 
 function readPort(appName: string): number {
diff --git a/e2e/utils/command-utils.ts b/e2e/utils/command-utils.ts
index bd3a087f10db46..ecfba98714a551 100644
--- a/e2e/utils/command-utils.ts
+++ b/e2e/utils/command-utils.ts
@@ -371,12 +371,12 @@ export function runCLI(
       });
     }
 
-    const r = stripConsoleColors(logs);
+    const r = stripConsoleColors(`${logs}`);
 
     return r;
   } catch (e) {
     if (opts.silenceError) {
-      return stripConsoleColors(e.stdout + e.stderr);
+      return stripConsoleColors(`${e.stdout} ${e.stderr}`);
     } else {
       logError(`Original command: ${command}`, `${e.stdout}\n\n${e.stderr}`);
       throw e;
diff --git a/packages/angular/src/generators/host/host.ts b/packages/angular/src/generators/host/host.ts
index 6942429589f4c8..a9bb4b5b029d71 100644
--- a/packages/angular/src/generators/host/host.ts
+++ b/packages/angular/src/generators/host/host.ts
@@ -13,6 +13,7 @@ import { setupMf } from '../setup-mf/setup-mf';
 import { updateSsrSetup } from './lib';
 import type { Schema } from './schema';
 import { addMfEnvToTargetDefaultInputs } from '../utils/add-mf-env-to-inputs';
+import { isValidVariable } from '@nx/js';
 
 export async function host(tree: Tree, options: Schema) {
   return await hostInternal(tree, {
@@ -30,6 +31,19 @@ export async function hostInternal(tree: Tree, schema: Schema) {
   const remotesToGenerate: string[] = [];
   const remotesToIntegrate: string[] = [];
 
+  // Check to see if remotes are provided and also check if --dynamic is provided
+  // if both are check that the remotes are valid names else throw an error.
+  if (options.dynamic && options.remotes?.length > 0) {
+    options.remotes.forEach((remote) => {
+      const isValidRemote = isValidVariable(remote);
+      if (!isValidRemote.isValid) {
+        throw new Error(
+          `Invalid remote name provided: ${remote}. ${isValidRemote.message}`
+        );
+      }
+    });
+  }
+
   if (options.remotes && options.remotes.length > 0) {
     options.remotes.forEach((remote) => {
       if (!projects.has(remote)) {
diff --git a/packages/js/src/index.ts b/packages/js/src/index.ts
index 3664ecd10ae922..f5a83153ef3fa4 100644
--- a/packages/js/src/index.ts
+++ b/packages/js/src/index.ts
@@ -13,6 +13,7 @@ export * from './utils/package-json/update-package-json';
 export * from './utils/package-json/create-entry-points';
 export { libraryGenerator } from './generators/library/library';
 export { initGenerator } from './generators/init/init';
+export { isValidVariable } from './utils/is-valid-variable';
 
 // eslint-disable-next-line @typescript-eslint/no-restricted-imports
 export {
diff --git a/packages/js/src/utils/is-valid-variable.ts b/packages/js/src/utils/is-valid-variable.ts
new file mode 100644
index 00000000000000..9983eb052d9fef
--- /dev/null
+++ b/packages/js/src/utils/is-valid-variable.ts
@@ -0,0 +1,37 @@
+/**
+ * Determines if a given string is a valid JavaScript variable name.
+ * @param name name of the variable to be checked
+ * @returns result object with a boolean indicating if the name is valid and a message explaining why it is not valid
+ */
+export function isValidVariable(name: string): {
+  isValid: boolean;
+  message: string;
+} {
+  const validRegex = /^[a-zA-Z_$][0-9a-zA-Z_$]*$/;
+
+  if (validRegex.test(name)) {
+    return { isValid: true, message: 'The name is a valid identifier.' };
+  } else {
+    if (name === '') {
+      return { isValid: false, message: 'The name cannot be empty.' };
+    } else if (/^[0-9]/.test(name)) {
+      return { isValid: false, message: 'The name cannot start with a digit.' };
+    } else if (/[^a-zA-Z0-9_$]/.test(name)) {
+      return {
+        isValid: false,
+        message:
+          'The name can only contain letters, digits, underscores, and dollar signs.',
+      };
+    } else if (/^[^a-zA-Z_$]/.test(name)) {
+      return {
+        isValid: false,
+        message:
+          'The name must start with a letter, underscore, or dollar sign.',
+      };
+    }
+    return {
+      isValid: false,
+      message: 'The name is not a valid JavaScript identifier.',
+    };
+  }
+}
diff --git a/packages/react/src/generators/host/host.ts b/packages/react/src/generators/host/host.ts
index a21e9c558a24be..08f7709991b5ad 100644
--- a/packages/react/src/generators/host/host.ts
+++ b/packages/react/src/generators/host/host.ts
@@ -21,6 +21,7 @@ import { setupSsrForHost } from './lib/setup-ssr-for-host';
 import { updateModuleFederationE2eProject } from './lib/update-module-federation-e2e-project';
 import { NormalizedSchema, Schema } from './schema';
 import { addMfEnvToTargetDefaultInputs } from '../../utils/add-mf-env-to-inputs';
+import { isValidVariable } from '@nx/js';
 
 export async function hostGenerator(
   host: Tree,
@@ -48,6 +49,19 @@ export async function hostGeneratorInternal(
     addPlugin: false,
   };
 
+  // Check to see if remotes are provided and also check if --dynamic is provided
+  // if both are check that the remotes are valid names else throw an error.
+  if (options.dynamic && options.remotes?.length > 0) {
+    options.remotes.forEach((remote) => {
+      const isValidRemote = isValidVariable(remote);
+      if (!isValidRemote.isValid) {
+        throw new Error(
+          `Invalid remote name provided: ${remote}. ${isValidRemote.message}`
+        );
+      }
+    });
+  }
+
   const initTask = await applicationGenerator(host, {
     ...options,
     // The target use-case is loading remotes as child routes, thus always enable routing.
diff --git a/packages/react/src/generators/remote/remote.ts b/packages/react/src/generators/remote/remote.ts
index 5ab224398ba2bc..ec4aaf3d719dd9 100644
--- a/packages/react/src/generators/remote/remote.ts
+++ b/packages/react/src/generators/remote/remote.ts
@@ -23,6 +23,7 @@ import { setupTspathForRemote } from './lib/setup-tspath-for-remote';
 import { addRemoteToDynamicHost } from './lib/add-remote-to-dynamic-host';
 import { addMfEnvToTargetDefaultInputs } from '../../utils/add-mf-env-to-inputs';
 import { maybeJs } from '../../utils/maybe-js';
+import { isValidVariable } from '@nx/js';
 
 export function addModuleFederationFiles(
   host: Tree,
@@ -90,6 +91,18 @@ export async function remoteGeneratorInternal(host: Tree, schema: Schema) {
     // TODO(colum): remove when MF works with Crystal
     addPlugin: false,
   };
+
+  if (options.dynamic) {
+    // Dynamic remotes generate with library { type: 'var' } by default.
+    // We need to ensure that the remote name is a valid variable name.
+    const isValidRemote = isValidVariable(options.name);
+    if (!isValidRemote.isValid) {
+      throw new Error(
+        `Invalid remote name provided: ${options.name}. ${isValidRemote.message}`
+      );
+    }
+  }
+
   const initAppTask = await applicationGenerator(host, {
     ...options,
     // Only webpack works with module federation for now.