Skip to content

feat(performance): report unused Module Federation shared keys and remotes - #4

Open
anurag6569201 wants to merge 1 commit into
qa/agent-webpack-webpack/pr-04-21990/basefrom
qa/agent-webpack-webpack/pr-04-21990/head
Open

anurag6569201 wants to merge 1 commit into
qa/agent-webpack-webpack/pr-04-21990/basefrom
qa/agent-webpack-webpack/pr-04-21990/head

Conversation

@anurag6569201

Copy link
Copy Markdown

Summary

Extends performance.unusedConfig — which already names the aliases, defines, externals and rules nothing used — to Module Federation. The shared half catches a silent failure: an exact key matches only the exact request, so shared: { lodash: { singleton: true } } next to import "lodash/debounce" shares nothing at all, bundles the module in, and still reports compiled successfully; for a package holding state that is a runtime bug, not bytes. Changing the key to "lodash/" turns the same build into provide shared module + consume shared module (singleton). The remotes half is weaker and the warning says so: a misspelled name already errors as Module not found, so it mostly names dead configuration, next to that error when there is one. Refs webpack#17122.

What kind of change does this PR introduce?

feat

Did you add tests for your changes?

Yes — nine cases under test/configCases/performance/unused-federation-* (shared, clean, share-key alias, prefix, remotes, ordered, no-hints, error, stats), green under both ConfigTestCases and ConfigCacheTestCases.

Does this PR introduce a breaking change?

No — no new option; performance.unusedConfig gains checks, and its schema description is updated to match.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

The performance.unusedConfig entry on webpack.js.org should mention the two federation checks.

Use of AI

AI (Claude Code) was used to implement the plugin and its tests, and to check the claims behind them against real builds. Two findings came out of that and are reflected in the diff: matching a remote name by cutting at the last / is wrong for names holding slashes (remotes: { "scope/def": … } with import "scope/def/hello/other/world" is falsely reported — now covered by unused-federation-remotes), and the original wording of the remotes warning claimed a typo would resolve elsewhere when in practice it already fails as Module not found. All output quoted above is from builds run locally, not recalled.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FxbjLoCKW57MoXMHEzmruX


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added diagnostics for unused Module Federation shared modules and remotes.
    • Reports unused entries as warnings, errors, or compilation hints according to performance settings.
    • Identifies affected entries, distinguishes shared modules from remotes, and links to relevant documentation.
    • Supports shared-module prefixes, duplicate configurations, remote matching, and consistent alphabetical reporting.
    • Added an option to report ESM references that prevent literal import() or new URL() output.
  • Documentation

    • Updated performance configuration descriptions to cover unused Federation entries and analyzable bailouts.

Source merge-base: 4448f8bbec0a4d48d40e0f9a859c9cd2ef69d2ee
Source head: 96e275dac0a138f356c9a496b50100ce6b0333de

@shipwright-agent

Copy link
Copy Markdown

⛔ Shipwright · Blocked

Recommendation: do not merge PR #4 · Tier T3
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (7)

  • CRITICAL The diff adds 'require("./performance/UnusedFederationPlugin")' in WebpackOptionsApply.js, but the diff does not include 'lib/performance/UnusedFederationPlugin.js'. · lib/WebpackOptionsApply.js:1089
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL ContainerReferencePlugin.js imports 'addDeclaredRemotes' from './declaredRemotes', but the diff does not include 'lib/container/declaredRemotes.js'. · lib/container/ContainerReferencePlugin.js:17
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The new UnusedFederationPlugin is required and applied in WebpackOptionsApply.js, but the file lib/performance/UnusedFederationPlugin.js is not present in the diff. · lib/WebpackOptionsApply.js:1089
    • Fix: Fix the review finding before release.
  • CRITICAL ContainerReferencePlugin.js imports addDeclaredRemotes from './declaredRemotes', but the diff does not include a new file lib/container/declaredRemotes.js. · lib/container/ContainerReferencePlugin.js:17
    • Fix: Fix the review finding before release.
  • HIGH The new file lib/sharing/declaredShared.js exports addDeclaredShared and getDeclaredShared, but the diff does not show any consumer of getDeclaredShared. · lib/sharing/declaredShared.js:47
    • Fix: Fix the review finding before release.
  • HIGH The new 'declaredShared.js' exports 'getDeclaredShared', but no consumer is present in the diff. · lib/sharing/declaredShared.js:47
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The warning message embeds declared federation names directly into a WebpackError string. · lib/errors/UnusedFederationWarning.js:20
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 45,320 input · 717 output · 46,037 total tokens · $0.0104 · 12s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

const UnusedAliasesPlugin = require("./performance/UnusedAliasesPlugin");
const UnusedDefinesPlugin = require("./performance/UnusedDefinesPlugin");
const UnusedExternalsPlugin = require("./performance/UnusedExternalsPlugin");
const UnusedFederationPlugin = require("./performance/UnusedFederationPlugin");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

The diff adds 'require("./performance/UnusedFederationPlugin")' in WebpackOptionsApply.js, but the diff does not include 'lib/performance/UnusedFederationPlugin.js'.

Impact: The diff adds 'require("./performance/UnusedFederationPlugin")' in WebpackOptionsApply.js, but the diff does not include 'lib/performance/UnusedFederationPlugin.js'. Enabling 'performance.unusedConfig' will throw MODULE_NOT_FOUND at startup.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

const RemoteModule = require("./RemoteModule");
const RemoteRuntimeModule = require("./RemoteRuntimeModule");
const RemoteToExternalDependency = require("./RemoteToExternalDependency");
const { addDeclaredRemotes } = require("./declaredRemotes");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

ContainerReferencePlugin.js imports 'addDeclaredRemotes' from './declaredRemotes', but the diff does not include 'lib/container/declaredRemotes.js'.

Impact: ContainerReferencePlugin.js imports 'addDeclaredRemotes' from './declaredRemotes', but the diff does not include 'lib/container/declaredRemotes.js'. Every build using ContainerReferencePlugin will fail with MODULE_NOT_FOUND.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

const UnusedAliasesPlugin = require("./performance/UnusedAliasesPlugin");
const UnusedDefinesPlugin = require("./performance/UnusedDefinesPlugin");
const UnusedExternalsPlugin = require("./performance/UnusedExternalsPlugin");
const UnusedFederationPlugin = require("./performance/UnusedFederationPlugin");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

The new UnusedFederationPlugin is required and applied in WebpackOptionsApply.js, but the file lib/performance/UnusedFederationPlugin.js is not present in the diff.

Impact: The new UnusedFederationPlugin is required and applied in WebpackOptionsApply.js, but the file lib/performance/UnusedFederationPlugin.js is not present in the diff. This will cause a module resolution error at runtime whenever performance.unusedConfig is enabled.

Suggested fix: Fix the review finding before release.

const RemoteModule = require("./RemoteModule");
const RemoteRuntimeModule = require("./RemoteRuntimeModule");
const RemoteToExternalDependency = require("./RemoteToExternalDependency");
const { addDeclaredRemotes } = require("./declaredRemotes");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

ContainerReferencePlugin.js imports addDeclaredRemotes from './declaredRemotes', but the diff does not include a new file lib/container/declaredRemotes.js.

Impact: ContainerReferencePlugin.js imports addDeclaredRemotes from './declaredRemotes', but the diff does not include a new file lib/container/declaredRemotes.js. This import will fail at runtime with MODULE_NOT_FOUND, breaking any build using ContainerReferencePlugin.

Suggested fix: Fix the review finding before release.

};

module.exports.addDeclaredShared = addDeclaredShared;
module.exports.getDeclaredShared = getDeclaredShared;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The new file lib/sharing/declaredShared.js exports addDeclaredShared and getDeclaredShared, but the diff does not show any consumer of getDeclaredShared.

Impact: The new file lib/sharing/declaredShared.js exports addDeclaredShared and getDeclaredShared, but the diff does not show any consumer of getDeclaredShared. The performance hint reporting logic that should use these declarations is not present in the changed files, so the feature cannot actually report unused shared keys or remotes. This is a concrete incomplete implementation introduced by the PR.

Suggested fix: Fix the review finding before release.

};

module.exports.addDeclaredShared = addDeclaredShared;
module.exports.getDeclaredShared = getDeclaredShared;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The new 'declaredShared.js' exports 'getDeclaredShared', but no consumer is present in the diff.

Impact: The new 'declaredShared.js' exports 'getDeclaredShared', but no consumer is present in the diff. The feature is incomplete: declarations are recorded but never read, so unused shared keys cannot be reported.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

/**
* Creates an instance of UnusedFederationWarning.
* @param {UnusedFederationDetails[]} entries the entries nothing used
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · HIGH

The warning message embeds declared federation names directly into a WebpackError string.

Impact: The warning message embeds declared federation names directly into a WebpackError string. If a remote or shared key contains control characters or terminal escape sequences, they are emitted unescaped into build output, creating a log/terminal injection surface.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant