Improve infrastructure logging across federation plugins #4115
Conversation
|
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@codex review pull request |
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors logging across federation plugins to standardize on infrastructure-aware loggers, improve log filtering in tests, and bind loggers to Webpack compilers for better integration and observability.
- Introduces
createInfrastructureLoggerandbindLoggerToCompilerutilities for consistent infrastructure logging - Replaces direct console calls with proper logger instances across all plugins
- Enhances test utilities to filter out expected infrastructure logs, reducing test noise
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/src/logger.ts | Core logger implementation with infrastructure logger support and compiler binding |
| packages/sdk/src/index.ts | Exports new logger utilities and types |
| packages/rspack/src/logger.ts | Updates to use infrastructure logger with fallback |
| packages/rspack/src/ModuleFederationPlugin.ts | Binds logger to compiler and replaces console.warn |
| packages/node/src/plugins/NodeFederationPlugin.ts | Adds logger binding and replaces console.error |
| packages/node/src/plugins/DynamicFilesystemChunkLoadingRuntimeModule.ts | Adds logger support and infrastructure logger binding |
| packages/nextjs-mf/src/plugins/container/RemoveEagerModulesFromRuntimePlugin.ts | Binds logger and replaces console calls |
| packages/nextjs-mf/src/plugins/NextFederationPlugin/remove-unnecessary-shared-keys.ts | Replaces console.warn with logger |
| packages/nextjs-mf/src/plugins/NextFederationPlugin/remove-unnecessary-shared-keys.test.ts | Updates test to use logger instead of console |
| packages/nextjs-mf/src/plugins/NextFederationPlugin/index.ts | Binds logger and replaces console.error |
| packages/nextjs-mf/src/plugins/NextFederationPlugin/apply-client-plugins.ts | Replaces console logging with logger |
| packages/nextjs-mf/src/plugins/CopyFederationPlugin.ts | Binds logger and replaces console.error |
| packages/nextjs-mf/src/logger.ts | Creates nextjs-mf specific logger |
| packages/manifest/src/logger.ts | Updates to use infrastructure logger with fallback |
| packages/manifest/src/StatsPlugin.ts | Binds logger to compiler |
| packages/enhanced/test/helpers/infrastructureLogErrors.js | Adds log filtering utilities for allowed infrastructure logs |
| packages/enhanced/test/configCases/sharing/consume-module/index.js | Updates warning capture to ignore infrastructure logs |
| packages/enhanced/test/configCases/sharing/consume-module-ignore-warnings/index.js | Updates warning capture to ignore infrastructure logs |
| packages/enhanced/test/ConfigTestCases.template.js | Adds infrastructure log stripping functionality |
| packages/enhanced/src/lib/container/runtime/ChildCompilationRuntimePlugin.ts | Uses infrastructure logger |
| packages/enhanced/src/lib/container/ModuleFederationPlugin.ts | Binds logger and uses infrastructure logger |
| packages/enhanced/src/lib/container/ContainerEntryModule.ts | Uses infrastructure logger |
| packages/dts-plugin/src/plugins/ConsumeTypesPlugin.ts | Uses infrastructure logger |
| packages/data-prefetch/src/cli/index.ts | Creates infrastructure logger and binds to compiler |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| this.prefix = prefix; | ||
| } | ||
|
|
||
| setDelegate(delegate?: LoggerDelegate | null) { |
There was a problem hiding this comment.
[nitpick] The setDelegate method accepts both undefined and null but treats them identically. Consider using a single type (undefined or null) for consistency.
| setDelegate(delegate?: LoggerDelegate | null) { | |
| setDelegate(delegate?: LoggerDelegate) { |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| private emit(method: LogMethod, args: any[]) { | ||
| const delegate = this.delegate ?? DEFAULT_DELEGATE; |
There was a problem hiding this comment.
The null-coalescing check this.delegate ?? DEFAULT_DELEGATE is redundant since setDelegate already ensures this.delegate is never null/undefined by setting it to DEFAULT_DELEGATE.
| const delegate = this.delegate ?? DEFAULT_DELEGATE; | |
| const delegate = this.delegate; |
Copilot uses AI. Check for mistakes.
| const createBundlerLogger: typeof createLogger = | ||
| typeof createInfrastructureLogger === 'function' | ||
| ? (createInfrastructureLogger as unknown as typeof createLogger) |
There was a problem hiding this comment.
The type assertion as unknown as typeof createLogger is unsafe and could mask type incompatibilities. Consider using a more specific type or union type instead.
| const createBundlerLogger: typeof createLogger = | |
| typeof createInfrastructureLogger === 'function' | |
| ? (createInfrastructureLogger as unknown as typeof createLogger) | |
| type LoggerFactory = typeof createLogger; | |
| const createBundlerLogger: LoggerFactory = | |
| typeof createInfrastructureLogger === 'function' | |
| // Type guard: check if the function signature matches | |
| // If not, wrap or fallback to createLogger | |
| ? (createInfrastructureLogger as LoggerFactory) |
Copilot uses AI. Check for mistakes.
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
|
@codex give harsh review or pr |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
This pull request refactors logging across several packages to standardize on infrastructure-aware loggers, improve log filtering in tests, and bind loggers to Webpack compilers for better integration and observability. It also enhances test utilities to ignore expected infrastructure logs, reducing noise in test output. The changes touch core plugin implementations, test helpers, and logger utility files.
Logging refactor and standardization:
loggerwithinfrastructureLoggeror a dynamic logger created viacreateInfrastructureLoggerandcreateLoggerin multiple packages (@module-federation/sdk,@module-federation/dts-plugin,@module-federation/enhanced,@module-federation/manifest,@module-federation/nextjs-mf), ensuring consistent infrastructure logging. [1] [2] Faf5bf1cL13, Fcb0b23eL8, Fcb0b23eL175, F193371cL1, [3] F80534b3L27, F80534b3L120, F6388ea5L1, F6388ea5L78, F1a3a53eL3, F1a3a53eL37)bindLoggerToCompilerin all major plugins (PrefetchPlugin,ModuleFederationPlugin,StatsPlugin,CopyBuildOutputPlugin,NextFederationPlugin) for improved log association and context. (Fd24c5a7L35, Fcb0b23eL102, Fbefb284L40, F6388ea5L23, F80534b3L54)Test and log filtering improvements:
stripAllowedInfrastructureLogsutility and enhancesfilterInfraStructureErrorsto ignore logs with specific infrastructure prefixes, reducing false positives in test output. This includes recognizing new allowed log prefixes and normalizing log entries. (Ffca3dcaL18, Ffca3dcaL216, Ffca3dcaL261, Ffca3dcaL363, F259d10eL10, F259d10eL20)Logger creation improvements:
createInfrastructureLoggerandcreateLoggerdepending on environment, ensuring compatibility and flexibility. (Fd24c5a7L18, F193371cL1, packages/nextjs-mf/src/logger.tsR1-R13)Plugin-specific logging updates:
console.warnandconsole.errorcalls in plugin logic with the appropriate logger methods for consistency and better log routing. (Fd24c5a7L54, Fcb0b23eL175, F6388ea5L78, F1a3a53eL37, F80534b3L120)General code and import hygiene:
These changes together improve logging infrastructure, test reliability, and code maintainability across the module federation ecosystem.