Skip to content

Conversation

@ahabhgk
Copy link
Contributor

@ahabhgk ahabhgk commented Oct 27, 2025

Summary

test coverage for #11861, which should be fixed by #11993

close #11861

Related links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings October 27, 2025 08:07
@ahabhgk ahabhgk requested a review from stormslowly as a code owner October 27, 2025 08:07
@netlify
Copy link

netlify bot commented Oct 27, 2025

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit 0079e82
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68ff5a9d7a5df50008726fe7
😎 Deploy Preview https://deploy-preview-12003--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions bot added team The issue/pr is created by the member of Rspack. release: bug fix release: bug related release(mr only) labels Oct 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a panic that occurs when Module Federation shares asset modules. The fix refactors the logic in AssetParserAndGenerator::get_types to properly handle source type determination when assets are shared between federated modules.

Key changes:

  • Refactored conditional logic in get_types method to avoid panic scenarios
  • Added test case to verify asset modules work correctly with Module Federation sharing
  • Fixed debug configuration path to use generic "node" instead of absolute path

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/rspack_plugin_asset/src/lib.rs Refactored get_types method logic to eliminate panic conditions when handling shared asset modules
tests/rspack-test/configCases/assets/asset-shared-by-mf/test.js Added test to verify asset imports work when shared via Module Federation
tests/rspack-test/configCases/assets/asset-shared-by-mf/rspack.config.js Test configuration for Module Federation with shared assets
tests/rspack-test/configCases/assets/asset-shared-by-mf/index.js Test entry point that dynamically imports the test module
scripts/debug/launch.mjs Changed node program path from absolute to relative for portability

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ahabhgk ahabhgk enabled auto-merge (squash) October 27, 2025 08:10
@ahabhgk ahabhgk requested review from LingyuCoder and removed request for stormslowly October 27, 2025 08:10
@github-actions
Copy link
Contributor

github-actions bot commented Oct 27, 2025

📦 Binary Size-limit

Comparing 0079e82 to feat(browser): support CORS worker and custom wasm url (#11996) by CPunisher

🙈 Size remains the same at 47.85MB

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 27, 2025

CodSpeed Performance Report

Merging #12003 will not alter performance

Comparing fix-share-asset-mf (0079e82) with main (5385264)

Summary

✅ 17 untouched

SyMind
SyMind previously approved these changes Oct 27, 2025
@ahabhgk ahabhgk force-pushed the fix-share-asset-mf branch from ddb93f1 to c39ad72 Compare October 27, 2025 11:41
@ahabhgk ahabhgk changed the title fix: should not panic for mf sharing asset modules test: should not panic for mf sharing asset modules Oct 27, 2025
@github-actions github-actions bot removed the release: bug fix release: bug related release(mr only) label Oct 27, 2025
@ahabhgk ahabhgk requested a review from SyMind October 27, 2025 12:08
@ahabhgk ahabhgk merged commit 859baaa into main Oct 28, 2025
72 of 74 checks passed
@ahabhgk ahabhgk deleted the fix-share-asset-mf branch October 28, 2025 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Module Federation crash when trying to share an asset

3 participants