Skip to content

Conversation

@LingyuCoder
Copy link
Contributor

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 CssUrl and NewUrl dependency types, ignored asset modules now export module.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

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

Copilot AI review requested due to automatic review settings December 16, 2025 07:38
@netlify
Copy link

netlify bot commented Dec 16, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 22091b4
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/69412e30fdbe6900082539ad

@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 Dec 16, 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 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 CssUrl and NewUrl dependency types when modules are ignored, exporting module.exports = "data:,"; instead of an empty comment
  • Removes the test filter file that was skipping tests for this functionality (issue #8531)
  • Imports DependencyType and RuntimeGlobals to 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.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2025

Rsdoctor Bundle Diff Analysis

Found 5 project(s) in monorepo.

📁 react-10k

Path: ../build-tools-performance/cases/react-10k/dist/rsdoctor-data.json

📌 Baseline Commit: 5fade2fad0 | PR: #12445

Metric Current Baseline Change
📊 Total Size 5.7 MB 5.7 MB 153.0 B (0.0%)
📄 JavaScript 5.7 MB 5.7 MB 153.0 B (0.0%)
🎨 CSS 21.0 B 21.0 B 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-10k Bundle Diff

📁 react-1k

Path: ../build-tools-performance/cases/react-1k/dist/rsdoctor-data.json

📌 Baseline Commit: 5fade2fad0 | PR: #12445

Metric Current Baseline Change
📊 Total Size 823.6 KB 823.4 KB 153.0 B (0.0%)
📄 JavaScript 823.6 KB 823.4 KB 153.0 B (0.0%)
🎨 CSS 0 B 0 B N/A
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-1k Bundle Diff

📁 react-5k

Path: ../build-tools-performance/cases/react-5k/dist/rsdoctor-data.json

📌 Baseline Commit: 5fade2fad0 | PR: #12445

Metric Current Baseline Change
📊 Total Size 2.7 MB 2.7 MB 153.0 B (0.0%)
📄 JavaScript 2.7 MB 2.7 MB 153.0 B (0.0%)
🎨 CSS 21.0 B 21.0 B 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-5k Bundle Diff

📁 rome

Path: ../build-tools-performance/cases/rome/dist/rsdoctor-data.json

📌 Baseline Commit: 5fade2fad0 | PR: #12445

Metric Current Baseline Change
📊 Total Size 984.3 KB 984.3 KB 0 B (0.0%)
📄 JavaScript 984.3 KB 984.3 KB 0 B (0.0%)
🎨 CSS 0 B 0 B N/A
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: rome Bundle Diff

📁 ui-components

Path: ../build-tools-performance/cases/ui-components/dist/rsdoctor-data.json

📌 Baseline Commit: 5fade2fad0 | PR: #12445

Metric Current Baseline Change
📊 Total Size 2.1 MB 2.1 MB 1.6 KB (0.1%)
📄 JavaScript 2.0 MB 2.0 MB 1.6 KB (0.1%)
🎨 CSS 83.0 KB 83.0 KB 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: ui-components Bundle Diff

Generated by Rsdoctor GitHub Action

@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2025

📦 Binary Size-limit

Comparing 22091b4 to feat(deps)!: bump swc_core from 46.0.3 to 50.2.3 and swc_experimental (#12445) by CPunisher

❌ Size increased by 512bytes from 48.28MB to 48.28MB (⬆️0.00%)

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 16, 2025

CodSpeed Performance Report

Merging #12468 will not alter performance

Comparing fix/ignored-asset-module-handling (22091b4) with main (5fade2f)

Summary

✅ 17 untouched

@LingyuCoder LingyuCoder requested a review from hardfist December 16, 2025 11:29
@LingyuCoder LingyuCoder enabled auto-merge (squash) December 16, 2025 11:40
@LingyuCoder LingyuCoder merged commit 886003f into main Dec 17, 2025
52 checks passed
@LingyuCoder LingyuCoder deleted the fix/ignored-asset-module-handling branch December 17, 2025 02:36
@CPunisher CPunisher mentioned this pull request Dec 17, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants