Skip to content

Commit

Permalink
fix(module-federation): Throw an error if remote is invalid (#23100)
Browse files Browse the repository at this point in the history
If you are generating a remote using `--dynamic` either by using the
`host` generator or the `remote` generator we now check to ensure that
the remote name is a valid JavaScript variable.

If this is not done the app with be invalid and unable to be ran or
bundled.


closes: #23024
  • Loading branch information
ndcunningham committed May 9, 2024
1 parent bdac1e2 commit 0322b98
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 1 deletion.
22 changes: 21 additions & 1 deletion packages/angular/src/generators/host/host.spec.ts
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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}.`);
});
});
14 changes: 14 additions & 0 deletions packages/angular/src/generators/host/host.ts
Expand Up @@ -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, {
Expand All @@ -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)) {
Expand Down
1 change: 1 addition & 0 deletions packages/js/src/index.ts
Expand Up @@ -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 {
Expand Down
37 changes: 37 additions & 0 deletions 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.',
};
}
}
19 changes: 19 additions & 0 deletions packages/react/src/generators/host/host.spec.ts
Expand Up @@ -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}.`);
});
});
14 changes: 14 additions & 0 deletions packages/react/src/generators/host/host.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions packages/react/src/generators/remote/remote.spec.ts
Expand Up @@ -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}.`);
});
});
13 changes: 13 additions & 0 deletions packages/react/src/generators/remote/remote.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 0322b98

Please sign in to comment.