Skip to content

refactor: rename container hooks for clarity and consistency #3902

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.

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

Open
wants to merge 3 commits into
base: fix/runtime-safety-checks
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions .changeset/hook-renaming-cleanup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@module-federation/enhanced": patch
"@module-federation/nextjs-mf": patch
---

refactor: rename container hooks for clarity and consistency

- Renamed `addContainerEntryModule` to `addContainerEntryDependency`
- Renamed `addFederationRuntimeModule` to `addFederationRuntimeDependency`
- Added new `addRemoteDependency` hook for remote module tracking
- Updated all hook usages across the codebase to use new names
- This is an internal refactoring with no breaking changes to external APIs
15 changes: 10 additions & 5 deletions INCREMENTAL_PR_PLAN_REVISED.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Revised Incremental PR Plan for packages/enhanced Changes
f# Revised Incremental PR Plan for packages/enhanced Changes

## Overview
Based on a detailed diff analysis, this document provides a more accurate breakdown of changes into focused, incremental PRs. Each PR represents a distinct feature, fix, or refactor that can be merged independently.
Expand All @@ -21,7 +21,7 @@ Based on a detailed diff analysis, this document provides a more accurate breakd

### PR 2: Hook Renaming and Cleanup
**Size**: Small (~6 files)
**Risk**: Medium (potential breaking change)
**Risk**: Low (internal refactoring only - all usages updated)
**Type**: Refactor
**Feature**: Rename container hooks for clarity and consistency

Expand All @@ -31,18 +31,23 @@ Based on a detailed diff analysis, this document provides a more accurate breakd
- `src/lib/container/runtime/FederationModulesPlugin.ts`
- `src/lib/container/runtime/EmbedFederationRuntimePlugin.ts`
- `src/lib/container/RemoteModule.ts` (use new hook)
- Any other files that reference these hooks

**Changes**:
- `addContainerEntryModule` → `addContainerEntryDependency`
- `addFederationRuntimeModule` → `addFederationRuntimeDependency`
- Add new `addRemoteDependency` hook

**Implementation with backward compatibility**:
**Implementation**:
```javascript
// Direct rename - all usages updated in same PR
compiler.hooks.addContainerEntryDependency = new SyncHook([...]);
compiler.hooks.addContainerEntryModule = compiler.hooks.addContainerEntryDependency; // deprecated
compiler.hooks.addFederationRuntimeDependency = new SyncHook([...]);
compiler.hooks.addRemoteDependency = new SyncHook([...]);
```

**Note**: This is NOT a breaking change because all hook usages within the codebase are updated in the same PR.

---

### PR 3: Enhanced HoistContainerReferencesPlugin
Expand Down Expand Up @@ -282,4 +287,4 @@ All Feature PRs ─────────────────────
2. **Reduced Risk**: Smaller, focused changes are easier to review and test
3. **Flexibility**: Some PRs can be developed in parallel
4. **Progressive Enhancement**: Each filtering feature builds on the previous
5. **Early Wins**: Runtime fixes and hook renaming can be merged quickly
5. **Early Wins**: Runtime fixes and hook renaming can be merged quickly
6 changes: 3 additions & 3 deletions packages/enhanced/src/lib/container/ContainerPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ class ContainerPlugin {
},
(error: WebpackError | null | undefined) => {
if (error) return reject(error);
hooks.addContainerEntryModule.call(dep);
hooks.addContainerEntryDependency.call(dep);
resolve(undefined);
},
);
Expand All @@ -233,7 +233,7 @@ class ContainerPlugin {
if (err) {
return reject(err);
}
hooks.addFederationRuntimeModule.call(
hooks.addFederationRuntimeDependency.call(
federationRuntimeDependency,
);
resolve(undefined);
Expand Down Expand Up @@ -291,7 +291,7 @@ class ContainerPlugin {
{ name: undefined },
(error: WebpackError | null | undefined) => {
if (error) return callback(error);
hooks.addContainerEntryModule.call(dep);
hooks.addContainerEntryDependency.call(dep);
callback();
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import RemoteToExternalDependency from './RemoteToExternalDependency';
import { parseOptions } from './options';
import { containerReferencePlugin } from '@module-federation/sdk';
import FederationRuntimePlugin from './runtime/FederationRuntimePlugin';
import FederationModulesPlugin from './runtime/FederationModulesPlugin';
import schema from '../../schemas/container/ContainerReferencePlugin';
import checkOptions from '../../schemas/container/ContainerReferencePlugin.check';

Expand Down Expand Up @@ -107,6 +108,8 @@ class ContainerReferencePlugin {
new FallbackModuleFactory(),
);

const hooks = FederationModulesPlugin.getCompilationHooks(compilation);

normalModuleFactory.hooks.factorize.tap(
'ContainerReferencePlugin',
//@ts-ignore
Expand All @@ -118,7 +121,7 @@ class ContainerReferencePlugin {
(data.request.length === key.length ||
data.request.charCodeAt(key.length) === slashCode)
) {
return new RemoteModule(
const remoteModule = new RemoteModule(
data.request,
//@ts-ignore
config.external.map((external: any, i: any) =>
Expand All @@ -132,6 +135,8 @@ class ContainerReferencePlugin {
//@ts-ignore
config.shareScope,
);
hooks.addRemoteDependency.call(remoteModule);
return remoteModule;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ export class HoistContainerReferences implements WebpackPluginInstance {
const logger = compilation.getLogger(PLUGIN_NAME);
const hooks = FederationModulesPlugin.getCompilationHooks(compilation);
const containerEntryDependencies = new Set<Dependency>();
hooks.addContainerEntryModule.tap(
hooks.addContainerEntryDependency.tap(
'HoistContainerReferences',
(dep: ContainerEntryDependency) => {
containerEntryDependencies.add(dep);
},
);
hooks.addFederationRuntimeModule.tap(
hooks.addFederationRuntimeDependency.tap(
'HoistContainerReferences',
(dep: FederationRuntimeDependency) => {
containerEntryDependencies.add(dep);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class EmbedFederationRuntimePlugin {
);

// Collect federation runtime dependencies.
federationHooks.addFederationRuntimeModule.tap(
federationHooks.addFederationRuntimeDependency.tap(
PLUGIN_NAME,
(dependency: FederationRuntimeDependency) => {
containerEntrySet.add(dependency);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ const PLUGIN_NAME = 'FederationModulesPlugin';
/** @typedef {{ header: string[], beforeStartup: string[], startup: string[], afterStartup: string[], allowInlineStartup: boolean }} Bootstrap */

type CompilationHooks = {
addContainerEntryModule: SyncHook<[ContainerEntryDependency], void>;
addFederationRuntimeModule: SyncHook<[FederationRuntimeDependency], void>;
addContainerEntryDependency: SyncHook<[ContainerEntryDependency], void>;
addFederationRuntimeDependency: SyncHook<[FederationRuntimeDependency], void>;
addRemoteDependency: SyncHook<[any], void>;
};

class FederationModulesPlugin {
Expand All @@ -36,8 +37,9 @@ class FederationModulesPlugin {
let hooks = compilationHooksMap.get(compilation);
if (hooks === undefined) {
hooks = {
addContainerEntryModule: new SyncHook(['dependency']),
addFederationRuntimeModule: new SyncHook(['module']),
addContainerEntryDependency: new SyncHook(['dependency']),
addFederationRuntimeDependency: new SyncHook(['dependency']),
addRemoteDependency: new SyncHook(['dependency']),
};
compilationHooksMap.set(compilation, hooks);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,9 @@ class FederationRuntimePlugin {
if (err) {
return callback(err);
}
hooks.addFederationRuntimeModule.call(federationRuntimeDependency);
hooks.addFederationRuntimeDependency.call(
federationRuntimeDependency,
);
callback();
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ jest.mock('../../../src/lib/container/runtime/FederationRuntimePlugin', () => {
return mockFederationRuntimePlugin;
});

// Mock FederationModulesPlugin
jest.mock('../../../src/lib/container/runtime/FederationModulesPlugin', () => {
return {
__esModule: true,
default: {
getCompilationHooks: jest.fn(() => ({
addContainerEntryDependency: { tap: jest.fn() },
addFederationRuntimeDependency: { tap: jest.fn() },
addRemoteDependency: { tap: jest.fn() },
})),
},
};
});

// Mock FallbackModuleFactory
jest.mock(
'../../../src/lib/container/FallbackModuleFactory',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class InvertedContainerPlugin {
(compilation: Compilation) => {
const hooks = FederationModulesPlugin.getCompilationHooks(compilation);
const containers = new Set();
hooks.addContainerEntryModule.tap(
hooks.addContainerEntryDependency.tap(
'EmbeddedContainerPlugin',
(dependency) => {
if (dependency instanceof dependencies.ContainerEntryDependency) {
Expand Down