-
-
Notifications
You must be signed in to change notification settings - Fork 754
fix: handle ignored asset modules with proper data URL export #12468
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
Conversation
✅ Deploy Preview for rspack canceled.
|
There was a problem hiding this 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 bug in the handling of ignored asset modules for dependencies that expect URL exports (CSS url() and new URL()). When an asset module is resolved to false via alias configuration, it needs to provide a valid data URL export rather than empty content. This ensures compatibility with CSS URL dependencies and JavaScript URL constructors that reference ignored assets.
Key Changes:
- Adds conditional handling for
CssUrlandNewUrldependency types when modules are ignored, exportingmodule.exports = "data:,";instead of an empty comment - Removes the test filter file that was skipping tests for this functionality (issue #8531)
- Imports
DependencyTypeandRuntimeGlobalsto support the new conditional logic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/rspack-test/configCases/asset-modules/ignore/test.filter.js |
Removes test filter since the underlying bug is now fixed |
crates/rspack_core/src/normal_module_factory.rs |
Adds conditional logic to export data URL for ignored CssUrl and NewUrl dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rsdoctor Bundle Diff AnalysisFound 5 project(s) in monorepo. 📁 react-10kPath:
📦 Download Diff Report: react-10k Bundle Diff 📁 react-1kPath:
📦 Download Diff Report: react-1k Bundle Diff 📁 react-5kPath:
📦 Download Diff Report: react-5k Bundle Diff 📁 romePath:
📦 Download Diff Report: rome Bundle Diff 📁 ui-componentsPath:
📦 Download Diff Report: ui-components Bundle Diff Generated by Rsdoctor GitHub Action |
📦 Binary Size-limit
❌ Size increased by 512bytes from 48.28MB to 48.28MB (⬆️0.00%) |
CodSpeed Performance ReportMerging #12468 will not alter performanceComparing Summary
|
Summary
This PR fixes the handling of ignored asset modules. Previously, when an asset module was ignored, it would return an empty comment
/* (ignored) */for all dependency types. This caused issues for CSS URL and New URL dependencies, which expect a valid module export.The fix ensures that for
CssUrlandNewUrldependency types, ignored asset modules now exportmodule.exports = "data:,";instead of empty content, allowing these dependencies to work correctly even when the asset is ignored.This change also removes the test filter file that was previously skipping tests for this issue, as the bug is now fixed.
Checklist