From 76738de12fdf29f5af78aaa23f03337c33c7926a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Roma=C5=84czyk?= Date: Fri, 1 Nov 2024 19:42:30 +0100 Subject: [PATCH] fix: allow for custom federated filename in MF1 Plugin (#775) * fix: allow for custom federated filename in MF1 Plugin * test: add cases for the fixed behaviour * refactor: use afterEach in MF tests * chore: add changeset --- .changeset/brown-countries-tickle.md | 5 +++ .../src/modules/ScriptManager/federated.ts | 6 ++- .../src/plugins/ModuleFederationPluginV1.ts | 7 ++-- .../ModuleFederationPluginV1.test.ts | 42 +++++++++++++------ .../ModuleFederationPluginV2.test.ts | 12 ++---- 5 files changed, 45 insertions(+), 27 deletions(-) create mode 100644 .changeset/brown-countries-tickle.md diff --git a/.changeset/brown-countries-tickle.md b/.changeset/brown-countries-tickle.md new file mode 100644 index 000000000..486b55197 --- /dev/null +++ b/.changeset/brown-countries-tickle.md @@ -0,0 +1,5 @@ +--- +"@callstack/repack": patch +--- + +Fix customization of MF1 federated entry filename diff --git a/packages/repack/src/modules/ScriptManager/federated.ts b/packages/repack/src/modules/ScriptManager/federated.ts index cf64c19ec..df392dd1d 100644 --- a/packages/repack/src/modules/ScriptManager/federated.ts +++ b/packages/repack/src/modules/ScriptManager/federated.ts @@ -181,10 +181,12 @@ export namespace Federated { * ``` * * @param config Configuration for the resolver. + * @param containerExt Extension of container bundles. Defaults to `.container.bundle`. * @returns A resolver function which will try to resolve URL based on given `scriptId` and `caller`. */ export function createURLResolver( - config: Federated.URLResolverConfig + config: Federated.URLResolverConfig, + containerExt = '.container.bundle' ): Federated.URLResolver { const resolvers: Record = {}; @@ -193,7 +195,7 @@ export namespace Federated { if (scriptId === key) { const url = config.containers[key] .replace(/\[name\]/g, scriptId) - .replace(/\[ext\]/g, '.container.bundle'); + .replace(/\[ext\]/g, containerExt); return url; } diff --git a/packages/repack/src/plugins/ModuleFederationPluginV1.ts b/packages/repack/src/plugins/ModuleFederationPluginV1.ts index 5631848f9..3f6198d98 100644 --- a/packages/repack/src/plugins/ModuleFederationPluginV1.ts +++ b/packages/repack/src/plugins/ModuleFederationPluginV1.ts @@ -259,12 +259,11 @@ export class ModuleFederationPluginV1 implements RspackPluginInstance { apply(compiler: Compiler) { const ModuleFederationPlugin = this.getModuleFederationPlugin(compiler); - // TODO fix in a separate PR (jbroma) - // biome-ignore format: fix in a separate PR const filenameConfig = - this.config.filename ?? this.config.exposes + this.config.filename ?? + (this.config.exposes ? `${this.config.name}.container.bundle` - : undefined; + : undefined); const libraryConfig = this.config.exposes ? { diff --git a/packages/repack/src/plugins/__tests__/ModuleFederationPluginV1.test.ts b/packages/repack/src/plugins/__tests__/ModuleFederationPluginV1.test.ts index 55eff4a8e..cc45c6c50 100644 --- a/packages/repack/src/plugins/__tests__/ModuleFederationPluginV1.test.ts +++ b/packages/repack/src/plugins/__tests__/ModuleFederationPluginV1.test.ts @@ -16,6 +16,10 @@ const mockCompiler = { } as unknown as Compiler; describe('ModuleFederationPlugin', () => { + afterEach(() => { + mockPlugin.mockClear(); + }); + it('should replace RemotesObject remotes', () => { new ModuleFederationPluginV1({ name: 'test', @@ -38,7 +42,6 @@ describe('ModuleFederationPlugin', () => { config = mockPlugin.mock.calls[0][0]; expect(config.remotes.external[0]).toMatch('promise new Promise'); expect(config.remotes.external[1]).toMatch('promise new Promise'); - mockPlugin.mockClear(); }); it('should replace string[] remotes', () => { @@ -50,7 +53,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.remotes[0]).toMatch('promise new Promise'); expect(config.remotes[1]).toMatch('promise new Promise'); - mockPlugin.mockClear(); }); it('should replace RemotesObject[] remotes', () => { @@ -66,7 +68,6 @@ describe('ModuleFederationPlugin', () => { expect(config.remotes[0].external).toMatch('promise new Promise'); expect(config.remotes[1].external[0]).toMatch('promise new Promise'); expect(config.remotes[1].external[1]).toMatch('promise new Promise'); - mockPlugin.mockClear(); }); it('should not add default resolver for remote', () => { @@ -80,7 +81,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.remotes.app1).toMatch('promise new Promise'); expect(config.remotes.app1).not.toMatch('scriptManager.addResolver'); - mockPlugin.mockClear(); }); it('should add default resolver for remote', () => { @@ -100,7 +100,6 @@ describe('ModuleFederationPlugin', () => { expect(config.remotes.app1).toMatch( 'http://localhost:6789/static/[name][ext]' ); - mockPlugin.mockClear(); }); it('should add default shared dependencies', () => { @@ -111,7 +110,6 @@ describe('ModuleFederationPlugin', () => { expect(config.shared).toHaveProperty('react-native'); expect(config.shared).toHaveProperty('react-native/'); expect(config.shared).toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not add deep imports to defaulted shared dependencies', () => { @@ -125,7 +123,6 @@ describe('ModuleFederationPlugin', () => { expect(config.shared).toHaveProperty('react-native'); expect(config.shared).not.toHaveProperty('react-native/'); expect(config.shared).not.toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should add deep imports to existing shared dependencies', () => { @@ -140,7 +137,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared).toHaveProperty('react-native/'); expect(config.shared).toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not add deep imports to existing shared dependencies', () => { @@ -156,7 +152,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared).not.toHaveProperty('react-native/'); expect(config.shared).not.toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not add deep imports to existing shared dependencies when react-native is not present', () => { @@ -170,7 +165,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared).not.toHaveProperty('react-native/'); expect(config.shared).not.toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should add deep imports to existing shared dependencies array', () => { @@ -182,7 +176,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared[2]).toHaveProperty('react-native/'); expect(config.shared[3]).toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not duplicate or override existing deep imports', () => { @@ -202,7 +195,6 @@ describe('ModuleFederationPlugin', () => { singleton: true, eager: true, }); - mockPlugin.mockClear(); }); it('should determine eager based on shared react-native config', () => { @@ -219,6 +211,30 @@ describe('ModuleFederationPlugin', () => { expect(config.shared).toHaveProperty('@react-native/'); expect(config.shared['react-native/'].eager).toBe(false); expect(config.shared['@react-native/'].eager).toBe(false); - mockPlugin.mockClear(); + }); + + it('should set default federated entry filename', () => { + new ModuleFederationPluginV1({ + name: 'test', + exposes: { + './App': './src/App', + }, + }).apply(mockCompiler); + + const config = mockPlugin.mock.calls[0][0]; + expect(config.filename).toBe('test.container.bundle'); + }); + + it('should allow for custom federated entry name through filename', () => { + new ModuleFederationPluginV1({ + name: 'test', + exposes: { + './App': './src/App', + }, + filename: 'remoteEntry.js', + }).apply(mockCompiler); + + const config = mockPlugin.mock.calls[0][0]; + expect(config.filename).toBe('remoteEntry.js'); }); }); diff --git a/packages/repack/src/plugins/__tests__/ModuleFederationPluginV2.test.ts b/packages/repack/src/plugins/__tests__/ModuleFederationPluginV2.test.ts index 173e920b9..815f2fa27 100644 --- a/packages/repack/src/plugins/__tests__/ModuleFederationPluginV2.test.ts +++ b/packages/repack/src/plugins/__tests__/ModuleFederationPluginV2.test.ts @@ -21,6 +21,10 @@ const runtimePluginPath = require.resolve( ); describe('ModuleFederationPlugin', () => { + afterEach(() => { + mockPlugin.mockClear(); + }); + it('should add default shared dependencies', () => { new ModuleFederationPluginV2({ name: 'test' }).apply(mockCompiler); @@ -29,7 +33,6 @@ describe('ModuleFederationPlugin', () => { expect(config.shared).toHaveProperty('react-native'); expect(config.shared).toHaveProperty('react-native/'); expect(config.shared).toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not add deep imports to defaulted shared dependencies', () => { @@ -43,7 +46,6 @@ describe('ModuleFederationPlugin', () => { expect(config.shared).toHaveProperty('react-native'); expect(config.shared).not.toHaveProperty('react-native/'); expect(config.shared).not.toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should add deep imports to existing shared dependencies', () => { @@ -58,7 +60,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared).toHaveProperty('react-native/'); expect(config.shared).toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not add deep imports to existing shared dependencies', () => { @@ -74,7 +75,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared).not.toHaveProperty('react-native/'); expect(config.shared).not.toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not add deep imports to existing shared dependencies when react-native is not present', () => { @@ -88,7 +88,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared).not.toHaveProperty('react-native/'); expect(config.shared).not.toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should add deep imports to existing shared dependencies array', () => { @@ -100,7 +99,6 @@ describe('ModuleFederationPlugin', () => { const config = mockPlugin.mock.calls[0][0]; expect(config.shared[2]).toHaveProperty('react-native/'); expect(config.shared[3]).toHaveProperty('@react-native/'); - mockPlugin.mockClear(); }); it('should not duplicate or override existing deep imports', () => { @@ -120,7 +118,6 @@ describe('ModuleFederationPlugin', () => { singleton: true, eager: true, }); - mockPlugin.mockClear(); }); it('should determine eager based on shared react-native config', () => { @@ -137,7 +134,6 @@ describe('ModuleFederationPlugin', () => { expect(config.shared).toHaveProperty('@react-native/'); expect(config.shared['react-native/'].eager).toBe(false); expect(config.shared['@react-native/'].eager).toBe(false); - mockPlugin.mockClear(); }); it('should add FederationRuntimePlugin to runtime plugins', () => {