Skip to content

Commit

Permalink
fix(module-federation): remote names should follow JS variable naming…
Browse files Browse the repository at this point in the history
… schema (#28401)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
We previously had a schema restriction on the characters allowed for
remote names. It was to prevent names that violated the JS spec for a
variable declaration.


## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
Ensure invalid project names fail error allowing the user to fix it at
generation


## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28354, #28408

---------

Co-authored-by: Jack Hsu <jack.hsu@gmail.com>
  • Loading branch information
Coly010 and jaysoo authored Oct 14, 2024
1 parent 1c466d0 commit 1badac8
Show file tree
Hide file tree
Showing 9 changed files with 331 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { E2eTestRunner, UnitTestRunner } from '../../utils/test-runners';
describe('federate-module', () => {
let schema: Schema = {
name: 'my-federated-module',
remote: 'my-remote',
path: 'apps/my-remote/src/my-federated-module.ts',
remoteDirectory: 'apps/my-remote',
remote: 'myremote',
path: 'apps/myremote/src/my-federated-module.ts',
remoteDirectory: 'apps/myremote',
};

describe('no remote', () => {
Expand All @@ -29,32 +29,32 @@ describe('federate-module', () => {

const projects = getProjects(tree);

expect(projects.get('my-remote').root).toEqual('apps/my-remote');
expect(projects.get('myremote').root).toEqual('apps/myremote');

expect(tree.exists('apps/my-remote/module-federation.config.ts')).toBe(
expect(tree.exists('apps/myremote/module-federation.config.ts')).toBe(
true
);

const content = tree.read(
'apps/my-remote/module-federation.config.ts',
'apps/myremote/module-federation.config.ts',
'utf-8'
);
expect(content).toContain(
`'./my-federated-module': 'apps/my-remote/src/my-federated-module.ts'`
`'./my-federated-module': 'apps/myremote/src/my-federated-module.ts'`
);

const tsconfig = JSON.parse(tree.read('tsconfig.base.json', 'utf-8'));
expect(
tsconfig.compilerOptions.paths['my-remote/my-federated-module']
).toEqual(['apps/my-remote/src/my-federated-module.ts']);
tsconfig.compilerOptions.paths['myremote/my-federated-module']
).toEqual(['apps/myremote/src/my-federated-module.ts']);
});
});

describe('with remote', () => {
const tree = createTreeWithEmptyWorkspace({ layout: 'apps-libs' });
let remoteSchema: remoteSchma = {
name: 'my-remote',
directory: 'my-remote',
name: 'myremote',
directory: 'myremote',
e2eTestRunner: E2eTestRunner.Cypress,
skipFormat: true,
linter: Linter.EsLint,
Expand All @@ -78,7 +78,7 @@ describe('federate-module', () => {
);

expect(content).not.toContain(
`'./my-federated-module': 'apps/my-remote/src/my-federated-module.ts'`
`'./my-federated-module': 'apps/myremote/src/my-federated-module.ts'`
);

await federateModuleGenerator(tree, {
Expand All @@ -92,15 +92,15 @@ describe('federate-module', () => {
'utf-8'
);
expect(content).toContain(
`'./my-federated-module': 'apps/my-remote/src/my-federated-module.ts'`
`'./my-federated-module': 'apps/myremote/src/my-federated-module.ts'`
);

const tsconfig = JSON.parse(tree.read('tsconfig.base.json', 'utf-8'));
expect(
tsconfig.compilerOptions.paths[
`${remoteSchema.name}/my-federated-module`
]
).toEqual(['apps/my-remote/src/my-federated-module.ts']);
).toEqual(['apps/myremote/src/my-federated-module.ts']);
});
});
});
Expand Down
15 changes: 15 additions & 0 deletions packages/angular/src/generators/remote/remote.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,4 +443,19 @@ describe('MF Remote App Generator', () => {
const packageJson = readJson(tree, 'package.json');
expect(packageJson).toEqual(initialPackageJson);
});

it('should error when an invalid remote name is passed to the remote generator', async () => {
const tree = createTreeWithEmptyWorkspace({ layout: 'apps-libs' });

await expect(
generateTestRemoteApplication(tree, {
directory: 'test/my-remote',
})
).rejects.toMatchInlineSnapshot(`
[Error: Invalid remote name: my-remote. Remote project names must:
- Start with a letter, dollar sign ($) or underscore (_)
- Followed by any valid character (letters, digits, underscores, or dollar signs)
The regular expression used is ^[a-zA-Z_$][a-zA-Z_$0-9]*$.]
`);
});
});
11 changes: 11 additions & 0 deletions packages/angular/src/generators/remote/remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
formatFiles,
getProjects,
runTasksInSerial,
stripIndents,
Tree,
} from '@nx/devkit';
import {
Expand Down Expand Up @@ -39,6 +40,16 @@ export async function remote(tree: Tree, schema: Schema) {
directory: options.directory,
});

const REMOTE_NAME_REGEX = '^[a-zA-Z_$][a-zA-Z_$0-9]*$';
const remoteNameRegex = new RegExp(REMOTE_NAME_REGEX);
if (!remoteNameRegex.test(remoteProjectName)) {
throw new Error(
stripIndents`Invalid remote name: ${remoteProjectName}. Remote project names must:
- Start with a letter, dollar sign ($) or underscore (_)
- Followed by any valid character (letters, digits, underscores, or dollar signs)
The regular expression used is ${REMOTE_NAME_REGEX}.`
);
}
const port = options.port ?? findNextAvailablePort(tree);

const appInstallTask = await applicationGenerator(tree, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ describe('federate-module', () => {
let tree: Tree;
let schema: Schema = {
name: 'my-federated-module',
remote: 'my-remote',
remoteDirectory: 'my-remote',
path: 'my-remote/src/my-federated-module.ts',
remote: 'myremote',
remoteDirectory: 'myremote',
path: 'myremote/src/my-federated-module.ts',
style: 'css',
skipFormat: true,
bundler: 'webpack',
Expand All @@ -33,35 +33,35 @@ describe('federate-module', () => {

beforeAll(() => {
tree = createTreeWithEmptyWorkspace();
tree.write('my-remote/src/my-federated-module.ts', ''); // Ensure that the file exists
tree.write('myremote/src/my-federated-module.ts', ''); // Ensure that the file exists
});
describe('no remote', () => {
it('should generate a remote and e2e', async () => {
await federateModuleGenerator(tree, schema);

const projects = getProjects(tree);

expect(projects.get('my-remote').root).toEqual('my-remote');
expect(projects.get('my-remote-e2e').root).toEqual('my-remote-e2e');
expect(projects.get('myremote').root).toEqual('myremote');
expect(projects.get('myremote-e2e').root).toEqual('myremote-e2e');
});

it('should contain an entry for the new path for module federation', async () => {
await federateModuleGenerator(tree, schema);

expect(tree.exists('my-remote/module-federation.config.ts')).toBe(true);
expect(tree.exists('myremote/module-federation.config.ts')).toBe(true);

const content = tree.read(
'my-remote/module-federation.config.ts',
'myremote/module-federation.config.ts',
'utf-8'
);
expect(content).toContain(
`'./my-federated-module': 'my-remote/src/my-federated-module.ts'`
`'./my-federated-module': 'myremote/src/my-federated-module.ts'`
);

const tsconfig = JSON.parse(tree.read('tsconfig.base.json', 'utf-8'));
expect(
tsconfig.compilerOptions.paths['my-remote/my-federated-module']
).toEqual(['my-remote/src/my-federated-module.ts']);
tsconfig.compilerOptions.paths['myremote/my-federated-module']
).toEqual(['myremote/src/my-federated-module.ts']);
});

it('should error when invalid path is provided', async () => {
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('federate-module', () => {
);

expect(content).not.toContain(
`'./my-federated-module': 'my-remote/src/my-federated-module.ts'`
`'./my-federated-module': 'myremote/src/my-federated-module.ts'`
);

await federateModuleGenerator(tree, {
Expand All @@ -112,13 +112,13 @@ describe('federate-module', () => {
'utf-8'
);
expect(content).toContain(
`'./my-federated-module': 'my-remote/src/my-federated-module.ts'`
`'./my-federated-module': 'myremote/src/my-federated-module.ts'`
);

const tsconfig = JSON.parse(tree.read('tsconfig.base.json', 'utf-8'));
expect(
tsconfig.compilerOptions.paths[`${schema.remote}/my-federated-module`]
).toEqual(['my-remote/src/my-federated-module.ts']);
).toEqual(['myremote/src/my-federated-module.ts']);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`remote generator bundler=rspack should create the remote with the correct config files 1`] = `
"const { composePlugins, withNx, withReact } = require('@nx/rspack');
const { withModuleFederation } = require('@nx/rspack/module-federation');
const baseConfig = require('./module-federation.config');
const config = {
...baseConfig,
};
// Nx plugins for rspack to build config object from Nx options and context.
/**
* DTS Plugin is disabled in Nx Workspaces as Nx already provides Typing support Module Federation
* The DTS Plugin can be enabled by setting dts: true
* Learn more about the DTS Plugin here: https://module-federation.io/configure/dts.html
*/
module.exports = composePlugins(withNx(), withReact(), withModuleFederation(config, { dts: false }));
"
`;

exports[`remote generator bundler=rspack should create the remote with the correct config files 2`] = `"module.exports = require('./rspack.config');"`;

exports[`remote generator bundler=rspack should create the remote with the correct config files 3`] = `
"module.exports = {
name: 'test',
exposes: {
'./Module': './src/remote-entry.ts',
},
};
"
`;

exports[`remote generator bundler=rspack should create the remote with the correct config files when --js=true 1`] = `
"const { composePlugins, withNx, withReact } = require('@nx/rspack');
const { withModuleFederation } = require('@nx/rspack/module-federation');
const baseConfig = require('./module-federation.config');
const config = {
...baseConfig,
};
// Nx plugins for rspack to build config object from Nx options and context.
/**
* DTS Plugin is disabled in Nx Workspaces as Nx already provides Typing support Module Federation
* The DTS Plugin can be enabled by setting dts: true
* Learn more about the DTS Plugin here: https://module-federation.io/configure/dts.html
*/
module.exports = composePlugins(withNx(), withReact(), withModuleFederation(config, { dts: false }));
"
`;

exports[`remote generator bundler=rspack should create the remote with the correct config files when --js=true 2`] = `"module.exports = require('./rspack.config');"`;

exports[`remote generator bundler=rspack should create the remote with the correct config files when --js=true 3`] = `
"module.exports = {
name: 'test',
exposes: {
'./Module': './src/remote-entry.js',
},
};
"
`;

exports[`remote generator bundler=rspack should create the remote with the correct config files when --typescriptConfiguration=true 1`] = `
"import { composePlugins, withNx, withReact } from '@nx/rspack';
import { withModuleFederation } from '@nx/rspack/module-federation';
import baseConfig from './module-federation.config';
const config = {
...baseConfig,
};
// Nx plugins for rspack to build config object from Nx options and context.
/**
* DTS Plugin is disabled in Nx Workspaces as Nx already provides Typing support Module Federation
* The DTS Plugin can be enabled by setting dts: true
* Learn more about the DTS Plugin here: https://module-federation.io/configure/dts.html
*/
export default composePlugins(
withNx(),
withReact(),
withModuleFederation(config, { dts: false })
);
"
`;

exports[`remote generator bundler=rspack should create the remote with the correct config files when --typescriptConfiguration=true 2`] = `
"export default require('./rspack.config');
"
`;

exports[`remote generator bundler=rspack should create the remote with the correct config files when --typescriptConfiguration=true 3`] = `
"import { ModuleFederationConfig } from '@nx/rspack/module-federation';
const config: ModuleFederationConfig = {
name: 'test',
exposes: {
'./Module': './src/remote-entry.ts',
},
};
export default config;
"
`;

exports[`remote generator bundler=rspack should generate correct remote with config files when using --ssr 1`] = `
"const {composePlugins, withNx, withReact} = require('@nx/rspack');
const {withModuleFederationForSSR} = require('@nx/rspack/module-federation');
const baseConfig = require("./module-federation.server.config");
const defaultConfig = {
...baseConfig,
};
// Nx plugins for rspack to build config object from Nx options and context.
/**
* DTS Plugin is disabled in Nx Workspaces as Nx already provides Typing support Module Federation
* The DTS Plugin can be enabled by setting dts: true
* Learn more about the DTS Plugin here: https://module-federation.io/configure/dts.html
*/
module.exports = composePlugins(withNx(), withReact({ssr: true}), withModuleFederationForSSR(defaultConfig, { dts: false }));
"
`;

exports[`remote generator bundler=rspack should generate correct remote with config files when using --ssr 2`] = `
"module.exports = {
name: 'test',
exposes: {
'./Module': './src/remote-entry.ts',
},
};
"
`;

exports[`remote generator bundler=rspack should generate correct remote with config files when using --ssr and --typescriptConfiguration=true 1`] = `
"import { composePlugins, withNx, withReact } from '@nx/rspack';
import { withModuleFederationForSSR } from '@nx/rspack/module-federation';
import baseConfig from './module-federation.server.config';
const defaultConfig = {
...baseConfig,
};
// Nx plugins for rspack to build config object from Nx options and context.
/**
* DTS Plugin is disabled in Nx Workspaces as Nx already provides Typing support Module Federation
* The DTS Plugin can be enabled by setting dts: true
* Learn more about the DTS Plugin here: https://module-federation.io/configure/dts.html
*/
export default composePlugins(
withNx(),
withReact({ ssr: true }),
withModuleFederationForSSR(defaultConfig, { dts: false })
);
"
`;

exports[`remote generator bundler=rspack should generate correct remote with config files when using --ssr and --typescriptConfiguration=true 2`] = `
"import { ModuleFederationConfig } from '@nx/rspack/module-federation';
const config: ModuleFederationConfig = {
name: 'test',
exposes: {
'./Module': './src/remote-entry.ts',
},
};
export default config;
"
`;
Loading

0 comments on commit 1badac8

Please sign in to comment.