Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow for custom federated filename in MF1 Plugin #775

Merged
merged 4 commits into from
Nov 1, 2024
+45 −27
Merged

fix: allow for custom federated filename in MF1 Plugin #775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57ee352
fix: allow for custom federated filename in MF1 Plugin
jbroma Nov 1, 2024
b52f354
test: add cases for the fixed behaviour
jbroma Nov 1, 2024
f509029
refactor: use afterEach in MF tests
jbroma Nov 1, 2024
f206242
chore: add changeset
jbroma Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brown-countries-tickle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@callstack/repack": patch
---

Fix customization of MF1 federated entry filename
6 changes: 4 additions & 2 deletions packages/repack/src/modules/ScriptManager/federated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> = {};

Expand All @@ -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;
}

Expand Down
7 changes: 3 additions & 4 deletions packages/repack/src/plugins/ModuleFederationPluginV1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
? {
Expand Down
42 changes: 29 additions & 13 deletions packages/repack/src/plugins/__tests__/ModuleFederationPluginV1.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const mockCompiler = {
} as unknown as Compiler;

describe('ModuleFederationPlugin', () => {
afterEach(() => {
mockPlugin.mockClear();
});

it('should replace RemotesObject remotes', () => {
new ModuleFederationPluginV1({
name: 'test',
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -202,7 +195,6 @@ describe('ModuleFederationPlugin', () => {
singleton: true,
eager: true,
});
mockPlugin.mockClear();
});

it('should determine eager based on shared react-native config', () => {
Expand All @@ -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');
});
});
12 changes: 4 additions & 8 deletions packages/repack/src/plugins/__tests__/ModuleFederationPluginV2.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -120,7 +118,6 @@ describe('ModuleFederationPlugin', () => {
singleton: true,
eager: true,
});
mockPlugin.mockClear();
});

it('should determine eager based on shared react-native config', () => {
Expand All @@ -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', () => {
Expand Down