Skip to content

fix: cache concatenated module imports is not safe#11385

Merged
LingyuCoder merged 2 commits intomainfrom
fix/concatenated-imports-runtime
Aug 15, 2025
Merged

fix: cache concatenated module imports is not safe#11385
LingyuCoder merged 2 commits intomainfrom
fix/concatenated-imports-runtime

Conversation

@LingyuCoder
Copy link
Copy Markdown
Contributor

@LingyuCoder LingyuCoder commented Aug 14, 2025

Summary

concatenated module imports are related to runtime, should add it to cache key

Related links

Checklist

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

Copilot AI review requested due to automatic review settings August 14, 2025 07:47
@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Aug 14, 2025

This comment was marked as outdated.

@netlify
Copy link
Copy Markdown

netlify bot commented Aug 14, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 2b7a383
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/689da006449f6a00088672e3

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 14, 2025

📦 Binary Size-limit

Comparing 2b7a383 to fix: should use specific runtime to get target url (#11372) by Fy

❌ Size increased by 1.75KB from 47.58MB to 47.59MB (⬆️0.00%)

@LingyuCoder LingyuCoder force-pushed the fix/concatenated-imports-runtime branch from 095a016 to bb8fdd8 Compare August 14, 2025 08:00
@LingyuCoder LingyuCoder requested a review from Copilot August 14, 2025 08:01

This comment was marked as outdated.

@LingyuCoder LingyuCoder requested a review from Copilot August 14, 2025 08:37
Copy link
Copy Markdown
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 caching issue for concatenated module imports by ensuring runtime context is included in the cache key. Previously, concatenated module imports were cached without considering the runtime, which could lead to incorrect cache hits when the same module is used in different runtime contexts.

  • Updates the cache key for concatenated module entries to include runtime information
  • Refactors the caching mechanism from module imports to module entries
  • Adds Clone trait to concatenation entry types to support the new caching approach

Reviewed Changes

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

File Description
crates/rspack_core/src/concatenated_module.rs Updates cache key to include runtime, refactors concatenation entry types, and removes nested caching
crates/rspack_core/src/artifacts/module_graph_cache_artifact.rs Renames and restructures cache from imports to entries with runtime-aware keys

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@LingyuCoder LingyuCoder enabled auto-merge (squash) August 14, 2025 08:52
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Aug 14, 2025

CodSpeed Performance Report

Merging #11385 will not alter performance

Comparing fix/concatenated-imports-runtime (2b7a383) with main (2462e17)1

🎉 Hooray! codspeed-rust just leveled up to 2.7.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 17 untouched benchmarks

Footnotes

  1. No successful run was found on main (c28a0a2) during the generation of this report, so 2462e17 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@LingyuCoder LingyuCoder requested a review from JSerFeng August 15, 2025 04:50
@LingyuCoder LingyuCoder merged commit cd1466f into main Aug 15, 2025
44 checks passed
@LingyuCoder LingyuCoder deleted the fix/concatenated-imports-runtime branch August 15, 2025 05:21
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