From aae62753d4c6aabc459331df87c35778312ec8a4 Mon Sep 17 00:00:00 2001 From: Nicholas Cunningham Date: Wed, 8 May 2024 01:30:27 -0600 Subject: [PATCH] fix(module-federation): Throw an error if remote is invalid closes: #23024 --- e2e/utils/log-utils.ts | 2 +- .../angular/src/generators/host/host.spec.ts | 22 ++++++++++- packages/angular/src/generators/host/host.ts | 15 ++++++++ packages/js/src/index.ts | 1 + packages/js/src/utils/is-valid-variable.ts | 37 +++++++++++++++++++ .../react/src/generators/host/host.spec.ts | 19 ++++++++++ packages/react/src/generators/host/host.ts | 15 ++++++++ .../src/generators/remote/remote.spec.ts | 20 ++++++++++ .../react/src/generators/remote/remote.ts | 14 +++++++ 9 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 packages/js/src/utils/is-valid-variable.ts diff --git a/e2e/utils/log-utils.ts b/e2e/utils/log-utils.ts index 88632b9aea290a..1de5eb7abf0123 100644 --- a/e2e/utils/log-utils.ts +++ b/e2e/utils/log-utils.ts @@ -40,7 +40,7 @@ export function logSuccess(title: string, body?: string) { * @returns */ export function stripConsoleColors(log: string): string { - return log?.replace( + return log.replace( /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, '' ); diff --git a/packages/angular/src/generators/host/host.spec.ts b/packages/angular/src/generators/host/host.spec.ts index 18d8eb53f994cf..ab04beb3eaa2ed 100644 --- a/packages/angular/src/generators/host/host.spec.ts +++ b/packages/angular/src/generators/host/host.spec.ts @@ -6,11 +6,12 @@ import { getProjects, readProjectConfiguration, } from 'nx/src/generators/utils/project-configuration'; -import { E2eTestRunner } from '../../utils/test-runners'; +import { E2eTestRunner, UnitTestRunner } from '../../utils/test-runners'; import { generateTestHostApplication, generateTestRemoteApplication, } from '../utils/testing'; +import { Linter } from '@nx/eslint'; describe('Host App Generator', () => { it('should generate a host app with no remotes', async () => { @@ -632,4 +633,23 @@ describe('Host App Generator', () => { const packageJson = readJson(tree, 'package.json'); expect(packageJson).toEqual(initialPackageJson); }); + + it('should throw an error if invalid remotes names are provided and --dynamic is set to true', async () => { + const tree = createTreeWithEmptyWorkspace(); + const remote = 'invalid-remote-name'; + + await expect( + generateTestHostApplication(tree, { + name: 'myhostapp', + remotes: [remote], + dynamic: true, + projectNameAndRootFormat: 'as-provided', + e2eTestRunner: E2eTestRunner.None, + linter: Linter.None, + style: 'css', + unitTestRunner: UnitTestRunner.None, + typescriptConfiguration: false, + }) + ).rejects.toThrowError(`Invalid remote name provided: ${remote}.`); + }); }); diff --git a/packages/angular/src/generators/host/host.ts b/packages/angular/src/generators/host/host.ts index 6942429589f4c8..2202930066e784 100644 --- a/packages/angular/src/generators/host/host.ts +++ b/packages/angular/src/generators/host/host.ts @@ -4,6 +4,7 @@ import { joinPathFragments, runTasksInSerial, Tree, + logger, } from '@nx/devkit'; import { determineProjectNameAndRootOptions } from '@nx/devkit/src/generators/project-name-and-root-utils'; import { E2eTestRunner } from '../../utils/test-runners'; @@ -13,6 +14,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 +32,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.spec.ts b/packages/react/src/generators/host/host.spec.ts index c45c65cecf7298..4faae04b64e99e 100644 --- a/packages/react/src/generators/host/host.spec.ts +++ b/packages/react/src/generators/host/host.spec.ts @@ -340,4 +340,23 @@ describe('hostGenerator', () => { tree.read('foo/host-app/module-federation.config.ts', 'utf-8') ).toContain(`'remote1', 'remote2', 'remote3'`); }); + + it('should throw an error if invalid remotes names are provided and --dynamic is set to true', async () => { + const tree = createTreeWithEmptyWorkspace(); + const remote = 'invalid-remote-name'; + + await expect( + hostGenerator(tree, { + name: 'myhostapp', + remotes: [remote], + dynamic: true, + projectNameAndRootFormat: 'as-provided', + e2eTestRunner: 'none', + linter: Linter.None, + style: 'css', + unitTestRunner: 'none', + typescriptConfiguration: false, + }) + ).rejects.toThrowError(`Invalid remote name provided: ${remote}.`); + }); }); diff --git a/packages/react/src/generators/host/host.ts b/packages/react/src/generators/host/host.ts index a21e9c558a24be..6bcf21a81ea968 100644 --- a/packages/react/src/generators/host/host.ts +++ b/packages/react/src/generators/host/host.ts @@ -5,6 +5,7 @@ import { readProjectConfiguration, runTasksInSerial, Tree, + logger, updateProjectConfiguration, } from '@nx/devkit'; import { updateModuleFederationProject } from '../../rules/update-module-federation-project'; @@ -21,6 +22,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 +50,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.spec.ts b/packages/react/src/generators/remote/remote.spec.ts index 7352126fb77b35..bc755b9b832d48 100644 --- a/packages/react/src/generators/remote/remote.spec.ts +++ b/packages/react/src/generators/remote/remote.spec.ts @@ -292,4 +292,24 @@ describe('remote generator', () => { tree.read('test/module-federation.server.config.ts', 'utf-8') ).toMatchSnapshot(); }); + + it('should throw an error if invalid remotes names are provided and --dynamic is set to true', async () => { + const tree = createTreeWithEmptyWorkspace(); + const name = 'invalid-dynamic-remote-name'; + await expect( + remote(tree, { + name, + devServerPort: 4209, + dynamic: true, + e2eTestRunner: 'cypress', + linter: Linter.EsLint, + skipFormat: false, + style: 'css', + unitTestRunner: 'jest', + ssr: true, + projectNameAndRootFormat: 'as-provided', + typescriptConfiguration: true, + }) + ).rejects.toThrowError(`Invalid remote name provided: ${name}.`); + }); }); diff --git a/packages/react/src/generators/remote/remote.ts b/packages/react/src/generators/remote/remote.ts index 5ab224398ba2bc..aa528da79e02ab 100644 --- a/packages/react/src/generators/remote/remote.ts +++ b/packages/react/src/generators/remote/remote.ts @@ -8,6 +8,7 @@ import { readProjectConfiguration, runTasksInSerial, Tree, + logger, updateProjectConfiguration, } from '@nx/devkit'; @@ -23,6 +24,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 +92,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.