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
callstack · Nov 1, 2024 · 76738de · 76738de
1 parent 3732d4a
commit 76738de
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 27 deletions.
diff --git a/.changeset/brown-countries-tickle.md 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
@@ -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<string, Federated.URLResolver> = {};
 
@@ -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
@@ -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
@@ -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
@@ -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', () => {